Hi Sakari,

Thank you for the patch.

On Tue, Jan 29, 2019 at 11:49:44PM +0200, Sakari Ailus wrote:
> 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.
> 
> Reported-by: Chiranjeevi Rapolu <chiranjeevi.rap...@intel.com>
> Tested-by: Chiranjeevi Rapolu <chiranjeevi.rap...@intel.com>
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>

The analysis looks good to me.

Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

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

>  drivers/media/usb/uvc/uvc_video.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c 
> b/drivers/media/usb/uvc/uvc_video.c
> index 84525ff047450..a30c5e1893e72 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -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)

-- 
Regards,

Laurent Pinchart

Reply via email to