Re: BUG: unable to handle kernel paging request at 6b6b6bcb (v4l2_device_disconnect+0x11/0x30)

[Sitsofe Wheeler <sitsofe@yahoo.com>]

On Mon, Sep 05, 2011 at 12:16:42PM +0200, Hans Verkuil wrote:
> On Monday, September 05, 2011 12:13:26 Hans Verkuil wrote:
> > 
> > The original order is correct, but what I missed is that for drivers
> > that release (free) everything in the videodev release callback the
> > v4l2_device struct is also freed and v4l2_device_put will fail.
> > 
> > To fix this, add this code just before the vdev->release call:
> > 
> > 	/* Do not call v4l2_device_put if there is no release callback set. */
> > 	if (v4l2_dev->release == NULL)
> > 		v4l2_dev = NULL;
> > 
> > If there is no release callback, then the refcounting is pointless
> > anyway.
> > 
> > This should work.
> 
> Note that in the long run using the v4l2_device release callback
> instead of the videodev release is better. But it's a lot of work to
> convert everything so that's long term. I'm quite surprised BTW that
> this bug wasn't found much earlier.

This inline patch fixes the second "poison overwritten" problem so:
Tested-by: Sitsofe Wheeler <sitsofe@yahoo.com>

However, it does not prevent the original oops that was reported in the
original message. Yang Ruirui's patch in
https://lkml.org/lkml/2011/9/1/74 seems to be required to resolve
that initial problem - can it be ACK'd? Yang's patch is reproduced
inline below:

For uvc device, dev->vdev.dev is the &intf->dev,
uvc_delete code is as below:
	usb_put_intf(dev->intf);
	usb_put_dev(dev->udev);

	uvc_status_cleanup(dev);
	uvc_ctrl_cleanup_device(dev);

## the intf dev is released above, so below code will oops.

	if (dev->vdev.dev)
		v4l2_device_unregister(&dev->vdev);
Fix it by get_device in v4l2_device_register and put_device in v4l2_device_disconnect
---
 drivers/media/video/v4l2-device.c |    2 ++
 1 file changed, 2 insertions(+)
diff --git a/drivers/media/video/v4l2-device.c b/drivers/media/video/v4l2-device.c
index c72856c..e6a2c3b 100644
--- a/drivers/media/video/v4l2-device.c
+++ b/drivers/media/video/v4l2-device.c
@@ -38,6 +38,7 @@ int v4l2_device_register(struct device *dev, struct v4l2_device *v4l2_dev)
 	mutex_init(&v4l2_dev->ioctl_lock);
 	v4l2_prio_init(&v4l2_dev->prio);
 	kref_init(&v4l2_dev->ref);
+	get_device(dev);
 	v4l2_dev->dev = dev;
 	if (dev == NULL) {
 		/* If dev == NULL, then name must be filled in by the caller */
@@ -93,6 +94,7 @@ void v4l2_device_disconnect(struct v4l2_device *v4l2_dev)
 
 	if (dev_get_drvdata(v4l2_dev->dev) == v4l2_dev)
 		dev_set_drvdata(v4l2_dev->dev, NULL);
+	put_device(v4l2_dev->dev);
 	v4l2_dev->dev = NULL;
 }
 EXPORT_SYMBOL_GPL(v4l2_device_disconnect);

-- 
Sitsofe | http://sucs.org/~sits/