uvc: Avoid NULL pointer dereference at the end of streaming
Subsystems (2)
PATCH 1/1 uvc: Avoid NULL pointer dereference at the end of streaming
The UVC video driver converts the timestamp from hardware specific unit to one known by the kernel at the time when the buffer is dequeued. This is fine in general, but the streamoff operation consists of the following steps (among other things):
1. uvc_video_clock_cleanup --- the hardware clock sample array is released and the pointer to the array is set to NULL,
2. buffers in active state are returned to the user and
3. buf_finish callback is called on buffers that are prepared. buf_finish includes calling uvc_video_clock_update that accesses the hardware clock sample array.
The above is serialised by a queue specific mutex. Address the problem by skipping the clock conversion if the hardware clock sample array is already released.
---
Hi Laurent,
This seems like something that's been out there for a while... I'll figure out soon which stable kernels should receive it, if any.
drivers/media/usb/uvc/uvc_video.c | 7 +++++++ 1 file changed, 7 insertions(+)
drivers/media/usb/uvc/uvc_video.c +7 -0
@@ -676,6 +676,13 @@ void uvc_video_clock_update(struct uvc_streaming *stream,if (!uvc_hw_timestamps_param)return ; +/* +* We may get called if there are buffers done but not dequeued by the +* user. Just bail out in that case. +*/ +if (!clock->samples) +return ; +spin_lock_irqsave (&clock->lock, flags);if (clock->count < clock->size)
Signature · 1 line
2.11.0
Hi Sakari,
Thank you for the patch.
Quoted 14 lines · Sakari Ailus
The UVC video driver converts the timestamp from hardware specific unit to one known by the kernel at the time when the buffer is dequeued. This is fine in general, but the streamoff operation consists of the following steps (among other things):
1.
uvc_video_clock_cleanup--- the hardware clock sample array is released and the pointer to the array is set to NULL,2. buffers in active state are returned to the user and
3.
buf_finishcallback is called on buffers that are prepared.buf_finishincludes callinguvc_video_clock_updatethat accesses the hardware clock sample array.
Quoted 7 lines
The above is serialised by a queue specific mutex. Address the problem by skipping the clock conversion if the hardware clock sample array is already released.
Reported-by Chiranjeevi Rapolu <[email protected]>Tested-by Chiranjeevi Rapolu <[email protected]>Signed-off-by Sakari Ailus <[email protected]>
The analysis looks good to me.
Quoted 5 lines
---
Hi Laurent,This seems like something that's been out there for a while... I'll figure out soon which stable kernels should receive it, if any.
Should I wait for the proper Fixes: and Cc:stable tags before queuing this patch then ?
Quoted 21 lines
drivers/media/usb/uvc/uvc_video.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/media/usb/uvc/uvc_video.c +7
-0@@ -676,6 +676,13 @@ void uvc_video_clock_update(struct uvc_streaming *stream,if (!uvc_hw_timestamps_param)return ; +/* +* We may get called if there are buffers done but not dequeued by the +* user. Just bail out in that case. +*/ +if (!clock->samples) +return ; +spin_lock_irqsave (&clock->lock, flags);if (clock->count < clock->size)
Signature · 3 lines
Regards, Laurent Pinchart
Hi Laurent,
Quoted 31 lines · Laurent Pinchart
Hi Sakari,
Thank you for the patch.
Quoted 14 lines · Sakari Ailus
The UVC video driver converts the timestamp from hardware specific unit to one known by the kernel at the time when the buffer is dequeued. This is fine in general, but the streamoff operation consists of the following steps (among other things):
1.
uvc_video_clock_cleanup--- the hardware clock sample array is released and the pointer to the array is set to NULL,2. buffers in active state are returned to the user and
3.
buf_finishcallback is called on buffers that are prepared.buf_finishincludes callinguvc_video_clock_updatethat accesses the hardware clock sample array.Quoted 7 lines
The above is serialised by a queue specific mutex. Address the problem by skipping the clock conversion if the hardware clock sample array is already released.
Reported-by Chiranjeevi Rapolu <[email protected]>Tested-by Chiranjeevi Rapolu <[email protected]>Signed-off-by Sakari Ailus <[email protected]>The analysis looks good to me.
Reviewed-by Laurent Pinchart <[email protected]>
Quoted 8 lines
Quoted 5 lines
---
Hi Laurent,This seems like something that's been out there for a while... I'll figure out soon which stable kernels should receive it, if any.
Should I wait for the proper Fixes: and Cc:stable tags before queuing this patch then ?
Please do. I'll figure these out in a moment...
Quoted 22 lines
Quoted 22 lines
drivers/media/usb/uvc/uvc_video.c | 7 +++++++ 1 file changed, 7 insertions(+)drivers/media/usb/uvc/uvc_video.c +7
-0@@ -676,6 +676,13 @@ void uvc_video_clock_update(struct uvc_streaming *stream,if (!uvc_hw_timestamps_param)return ; +/* +* We may get called if there are buffers done but not dequeued by the +* user. Just bail out in that case. +*/ +if (!clock->samples) +return ; +spin_lock_irqsave (&clock->lock, flags);if (clock->count < clock->size)
Signature · 4 lines
Regards, Sakari Ailus [email protected]