linux-kernel - Re: Linux 2.6.26-rc9 circular lock with uvcvideo on resume from hibernation

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: <200807092210.56678.laurent.pinchart@skynet.be>
Date:	Wed, 9 Jul 2008 22:10:56 +0200
From:	Laurent Pinchart <laurent.pinchart@...net.be>
To:	Roland Dreier <roland@...italvampire.org>
Cc:	Romano Giannetti <romano@....icai.upcomillas.es>,
	linux-kernel@...r.kernel.org, linux-uvc-devel@...lios.de
Subject: Re: Linux 2.6.26-rc9 circular lock with uvcvideo on resume from hibernation

On Wednesday 09 July 2008, Roland Dreier wrote:
>  > uvc_disconnect()                      | uvc_v4l2_open()
>  > ...                                   |
>  > mutex_lock(&uvc_driver.open_mutex);   |
>  > dev->state |= UVC_DEV_DISCONNECTED;   |
>  > mutex_unlock(&uvc_driver.open_mutex); |
>  >
>  >                                       | mutex_lock(&uvc_driver.open_mute
>  >                                       |x); vdev = video_devdata(file);
>  >                                       | video = video_get_drvdata(vdev);
>  >
>  > kref_put(&dev->kref, uvc_delete);     |
>  >
>  >                                       | if (video->dev->state...)
>  >
>  > kref_put() in uvc_disconnect() will call uvc_delete(), which will in
>  > turn free the video structure. uvc_v4l2_open() will then dereference
>  > freed memory when testing the device state.
>
> I don't believe this is correct.  I tried to explain it in my
> changelog by saying "uvc_delete() does uvc_unregister_video() (and
> hence video_unregister_device(), which is synchronized with
> videodev_lock) as its first thing, so there is no risk of
> use-after-free in uvc_v4l2_open()."
>
> In other words, the first thing uvc_delete() does is call
> uvc_unregister_video(), which will video_unregister_device().  Since
> this needs to take videodev_lock, it will wait until uvc_v4l2_open()
> returns (which it will do, since state is now UVC_DEV_DISCONNECTED).
> So the video struct will not be freed until after uvc_v4l2_open()
> returns.
>
> As far as I can see there is no use-after-free.

My bad, you seem to be right. I'll apply your patch.

Have you checked David's videodev patch ? Any opinion ?

Best regards,

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