Commit 9e08117c authored by Alan Stern's avatar Alan Stern Committed by Mauro Carvalho Chehab

media: usbvision: Fix races among open, close, and disconnect

Visual inspection of the usbvision driver shows that it suffers from
three races between its open, close, and disconnect handlers.  In
particular, the driver is careful to update its usbvision->user and
usbvision->remove_pending flags while holding the private mutex, but:

	usbvision_v4l2_close() and usbvision_radio_close() don't hold
	the mutex while they check the value of
	usbvision->remove_pending;

	usbvision_disconnect() doesn't hold the mutex while checking
	the value of usbvision->user; and

	also, usbvision_v4l2_open() and usbvision_radio_open() don't
	check whether the device has been unplugged before allowing
	the user to open the device files.

Each of these can potentially lead to usbvision_release() being called
twice and use-after-free errors.

This patch fixes the races by reading the flags while the mutex is
still held and checking for pending removes before allowing an open to
succeed.
Signed-off-by: default avatarAlan Stern <stern@rowland.harvard.edu>
CC: <stable@vger.kernel.org>
Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab+samsung@kernel.org>
parent c7a19146
...@@ -314,6 +314,10 @@ static int usbvision_v4l2_open(struct file *file) ...@@ -314,6 +314,10 @@ static int usbvision_v4l2_open(struct file *file)
if (mutex_lock_interruptible(&usbvision->v4l2_lock)) if (mutex_lock_interruptible(&usbvision->v4l2_lock))
return -ERESTARTSYS; return -ERESTARTSYS;
if (usbvision->remove_pending) {
err_code = -ENODEV;
goto unlock;
}
if (usbvision->user) { if (usbvision->user) {
err_code = -EBUSY; err_code = -EBUSY;
} else { } else {
...@@ -377,6 +381,7 @@ static int usbvision_v4l2_open(struct file *file) ...@@ -377,6 +381,7 @@ static int usbvision_v4l2_open(struct file *file)
static int usbvision_v4l2_close(struct file *file) static int usbvision_v4l2_close(struct file *file)
{ {
struct usb_usbvision *usbvision = video_drvdata(file); struct usb_usbvision *usbvision = video_drvdata(file);
int r;
PDEBUG(DBG_IO, "close"); PDEBUG(DBG_IO, "close");
...@@ -391,9 +396,10 @@ static int usbvision_v4l2_close(struct file *file) ...@@ -391,9 +396,10 @@ static int usbvision_v4l2_close(struct file *file)
usbvision_scratch_free(usbvision); usbvision_scratch_free(usbvision);
usbvision->user--; usbvision->user--;
r = usbvision->remove_pending;
mutex_unlock(&usbvision->v4l2_lock); mutex_unlock(&usbvision->v4l2_lock);
if (usbvision->remove_pending) { if (r) {
printk(KERN_INFO "%s: Final disconnect\n", __func__); printk(KERN_INFO "%s: Final disconnect\n", __func__);
usbvision_release(usbvision); usbvision_release(usbvision);
return 0; return 0;
...@@ -1064,6 +1070,11 @@ static int usbvision_radio_open(struct file *file) ...@@ -1064,6 +1070,11 @@ static int usbvision_radio_open(struct file *file)
if (mutex_lock_interruptible(&usbvision->v4l2_lock)) if (mutex_lock_interruptible(&usbvision->v4l2_lock))
return -ERESTARTSYS; return -ERESTARTSYS;
if (usbvision->remove_pending) {
err_code = -ENODEV;
goto out;
}
err_code = v4l2_fh_open(file); err_code = v4l2_fh_open(file);
if (err_code) if (err_code)
goto out; goto out;
...@@ -1096,6 +1107,7 @@ static int usbvision_radio_open(struct file *file) ...@@ -1096,6 +1107,7 @@ static int usbvision_radio_open(struct file *file)
static int usbvision_radio_close(struct file *file) static int usbvision_radio_close(struct file *file)
{ {
struct usb_usbvision *usbvision = video_drvdata(file); struct usb_usbvision *usbvision = video_drvdata(file);
int r;
PDEBUG(DBG_IO, ""); PDEBUG(DBG_IO, "");
...@@ -1109,9 +1121,10 @@ static int usbvision_radio_close(struct file *file) ...@@ -1109,9 +1121,10 @@ static int usbvision_radio_close(struct file *file)
usbvision_audio_off(usbvision); usbvision_audio_off(usbvision);
usbvision->radio = 0; usbvision->radio = 0;
usbvision->user--; usbvision->user--;
r = usbvision->remove_pending;
mutex_unlock(&usbvision->v4l2_lock); mutex_unlock(&usbvision->v4l2_lock);
if (usbvision->remove_pending) { if (r) {
printk(KERN_INFO "%s: Final disconnect\n", __func__); printk(KERN_INFO "%s: Final disconnect\n", __func__);
v4l2_fh_release(file); v4l2_fh_release(file);
usbvision_release(usbvision); usbvision_release(usbvision);
...@@ -1543,6 +1556,7 @@ static int usbvision_probe(struct usb_interface *intf, ...@@ -1543,6 +1556,7 @@ static int usbvision_probe(struct usb_interface *intf,
static void usbvision_disconnect(struct usb_interface *intf) static void usbvision_disconnect(struct usb_interface *intf)
{ {
struct usb_usbvision *usbvision = to_usbvision(usb_get_intfdata(intf)); struct usb_usbvision *usbvision = to_usbvision(usb_get_intfdata(intf));
int u;
PDEBUG(DBG_PROBE, ""); PDEBUG(DBG_PROBE, "");
...@@ -1559,13 +1573,14 @@ static void usbvision_disconnect(struct usb_interface *intf) ...@@ -1559,13 +1573,14 @@ static void usbvision_disconnect(struct usb_interface *intf)
v4l2_device_disconnect(&usbvision->v4l2_dev); v4l2_device_disconnect(&usbvision->v4l2_dev);
usbvision_i2c_unregister(usbvision); usbvision_i2c_unregister(usbvision);
usbvision->remove_pending = 1; /* Now all ISO data will be ignored */ usbvision->remove_pending = 1; /* Now all ISO data will be ignored */
u = usbvision->user;
usb_put_dev(usbvision->dev); usb_put_dev(usbvision->dev);
usbvision->dev = NULL; /* USB device is no more */ usbvision->dev = NULL; /* USB device is no more */
mutex_unlock(&usbvision->v4l2_lock); mutex_unlock(&usbvision->v4l2_lock);
if (usbvision->user) { if (u) {
printk(KERN_INFO "%s: In use, disconnect pending\n", printk(KERN_INFO "%s: In use, disconnect pending\n",
__func__); __func__);
wake_up_interruptible(&usbvision->wait_frame); wake_up_interruptible(&usbvision->wait_frame);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment