Skip to contentSkip to conversation

usbvision-video: two use after frees

PATCH In review Dan Carpenter 3 participants 4 messages Oct 16, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 usbvision-video: two use after frees

The lock has been freed in usbvision_release() so there is no need to call mutex_unlock() here.

drivers/media/usb/usbvision/usbvision-video.c +2 -0
index 68bc961..9bfa041 100644
@@ -446,6 +446,7 @@ static int usbvision_v4l2_close(struct file *file)
 	if (usbvision->remove_pending) {
 		printk(KERN_INFO "%s: Final disconnect\n", __func__);
 		usbvision_release(usbvision);
+		return 0;
 	}
 	mutex_unlock(&usbvision->v4l2_lock);
 
@@ -1221,6 +1222,7 @@ static int usbvision_radio_close(struct file *file)
 	if (usbvision->remove_pending) {
 		printk(KERN_INFO "%s: Final disconnect\n", __func__);
 		usbvision_release(usbvision);
+		return err_code;
 	}
 
 	mutex_unlock(&usbvision->v4l2_lock);
#1 lore raw
Quoted 2 lines · On Thursday 16 October 2014 10:57:21 Dan Carpenter wrote:

The lock has been freed in usbvision_release() so there is no need to call mutex_unlock() here.

Yuck :-/

The driver should really be converted to use video_device::release. That might be out of scope for this fix though. Is usbvision maintained ?

Quoted 23 lines
(quoted diff) +0 -0

b/drivers/media/usb/usbvision/usbvision-video.c index 68bc961..9bfa041 100644

drivers/media/usb/usbvision/usbvision-video.c +2 -0
@@ -446,6 +446,7 @@ static int usbvision_v4l2_close(struct file *file)
 	if (usbvision->remove_pending) {
 		printk(KERN_INFO "%s: Final disconnect\n", __func__);
 		usbvision_release(usbvision);
+		return 0;
 	}
 	mutex_unlock(&usbvision->v4l2_lock);
 
@@ -1221,6 +1222,7 @@ static int usbvision_radio_close(struct file *file)
 	if (usbvision->remove_pending) {
 		printk(KERN_INFO "%s: Final disconnect\n", __func__);
 		usbvision_release(usbvision);
+		return err_code;
 	}
 
 	mutex_unlock(&usbvision->v4l2_lock);
Signature · 3 lines
Regards,

Laurent Pinchart
#2 lore raw
Quoted 8 lines · Laurent Pinchart
Quoted 2 lines · On Thursday 16 October 2014 10:57:21 Dan Carpenter wrote:

The lock has been freed in usbvision_release() so there is no need to call mutex_unlock() here.

Yuck :-/

The driver should really be converted to use video_device::release. That might be out of scope for this fix though. Is usbvision maintained ?

I have hardware, and at some point I plan to convert it to modern frameworks. But ENOTIME for now. So I guess I might be the closest to a being a maintainer.

Regards,

Hans

Quoted 25 lines
Quoted 24 lines
(quoted diff) +0 -0

b/drivers/media/usb/usbvision/usbvision-video.c index 68bc961..9bfa041 100644

drivers/media/usb/usbvision/usbvision-video.c +2 -0
@@ -446,6 +446,7 @@ static int usbvision_v4l2_close(struct file *file)
 	if (usbvision->remove_pending) {
 		printk(KERN_INFO "%s: Final disconnect\n", __func__);
 		usbvision_release(usbvision);
+		return 0;
 	}
 	mutex_unlock(&usbvision->v4l2_lock);
 
@@ -1221,6 +1222,7 @@ static int usbvision_radio_close(struct file *file)
 	if (usbvision->remove_pending) {
 		printk(KERN_INFO "%s: Final disconnect\n", __func__);
 		usbvision_release(usbvision);
+		return err_code;
 	}
 
 	mutex_unlock(&usbvision->v4l2_lock);
#3 lore raw
Quoted 5 lines · On Thursday 16 October 2014 10:23:05 Hans Verkuil wrote:
Quoted 4 lines · Laurent Pinchart
Quoted 2 lines · On Thursday 16 October 2014 10:57:21 Dan Carpenter wrote:

The lock has been freed in usbvision_release() so there is no need to call mutex_unlock() here.

Quoted 8 lines
Quoted 4 lines

Yuck :-/

The driver should really be converted to use video_device::release. That might be out of scope for this fix though. Is usbvision maintained ?

I have hardware, and at some point I plan to convert it to modern frameworks. But ENOTIME for now. So I guess I might be the closest to a being a maintainer.

Can you ack the patch then ? :-)

Quoted 23 lines
Quoted 23 lines
Quoted 23 lines
(quoted diff) +0 -0

b/drivers/media/usb/usbvision/usbvision-video.c index 68bc961..9bfa041 100644

drivers/media/usb/usbvision/usbvision-video.c +2 -0
@@ -446,6 +446,7 @@ static int usbvision_v4l2_close(struct file *file)
 	if (usbvision->remove_pending) {
 		printk(KERN_INFO "%s: Final disconnect\n", __func__);
 		usbvision_release(usbvision);
+		return 0;
 	}
 	mutex_unlock(&usbvision->v4l2_lock);
 
@@ -1221,6 +1222,7 @@ static int usbvision_radio_close(struct file *file)
 	if (usbvision->remove_pending) {
 		printk(KERN_INFO "%s: Final disconnect\n", __func__);
 		usbvision_release(usbvision);
+		return err_code;
 	}
 
 	mutex_unlock(&usbvision->v4l2_lock);
Signature · 3 lines
Regards,

Laurent Pinchart