linux-kernel - Re: BUG: unable to handle kernel paging request at 6b6b6bcb (v4l2_device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CABqxG0dwKgCVNDJhitn3M5t099-eAV1MPVRRji+U_u6ooABY9A@mail.gmail.com>
Date:	Tue, 6 Sep 2011 16:07:48 +0800
From:	Dave Young <hidave.darkstar@...il.com>
To:	Sitsofe Wheeler <sitsofe@...oo.com>
Cc:	Hans Verkuil <hverkuil@...all.nl>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	Guennadi Liakhovetski <g.liakhovetski@....de>,
	Mauro Carvalho Chehab <mchehab@...radead.org>
Subject: Re: BUG: unable to handle kernel paging request at 6b6b6bcb (v4l2_device_disconnect+0x11/0x30)

On Tue, Sep 6, 2011 at 6:31 AM, Sitsofe Wheeler <sitsofe@...oo.com> wrote:
> 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@...oo.com>

Confirmed

>
> 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:

Thanks, If it's ok, I can resend it as inline.

>
> 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/
>



-- 
Regards
Yang RuiRui
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/