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

[Laurent Pinchart]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/