linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/1] uvc: Avoid NULL pointer dereference at the end of streaming
@ 2019-01-30 10:09 Sakari Ailus
  2019-01-31  2:04 ` Sasha Levin
  0 siblings, 1 reply; 4+ messages in thread
From: Sakari Ailus @ 2019-01-30 10:09 UTC (permalink / raw)
  To: laurent.pinchart; +Cc: linux-media, chiranjeevi.rapolu

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.

Fixes: 9c0863b1cc48 ("[media] vb2: call buf_finish from __queue_cancel")
Reported-by: Chiranjeevi Rapolu <chiranjeevi.rapolu@intel.com>
Tested-by: Chiranjeevi Rapolu <chiranjeevi.rapolu@intel.com>
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: stable@vger.kernel.org
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
since v1:

- Improve the comment in uvc_video_clock_update().

- Add appropriate Cc: stable, Reviewed-by: and Fixes: tags.

 drivers/media/usb/uvc/uvc_video.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 84525ff047450..e314657a1843a 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -676,6 +676,14 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
 	if (!uvc_hw_timestamps_param)
 		return;
 
+	/*
+	 * We will get called from __vb2_queue_cancel() if there are buffers
+	 * done but not dequeued by the user, but the sample array has already
+	 * been released at that time. Just bail out in that case.
+	 */
+	if (!clock->samples)
+		return;
+
 	spin_lock_irqsave(&clock->lock, flags);
 
 	if (clock->count < clock->size)
-- 
2.11.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/1] uvc: Avoid NULL pointer dereference at the end of streaming
  2019-01-30 10:09 [PATCH v2 1/1] uvc: Avoid NULL pointer dereference at the end of streaming Sakari Ailus
@ 2019-01-31  2:04 ` Sasha Levin
  2019-01-31  7:31   ` Sakari Ailus
  0 siblings, 1 reply; 4+ messages in thread
From: Sasha Levin @ 2019-01-31  2:04 UTC (permalink / raw)
  To: Sasha Levin, Sakari Ailus, laurent.pinchart
  Cc: linux-media, chiranjeevi.rapolu, stable, stable

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 9c0863b1cc48 [media] vb2: call buf_finish from __queue_cancel.

The bot has tested the following trees: v4.20.5, v4.19.18, v4.14.96, v4.9.153, v4.4.172, v3.18.133.

v4.20.5: Build OK!
v4.19.18: Build OK!
v4.14.96: Build OK!
v4.9.153: Build OK!
v4.4.172: Build OK!
v3.18.133: Failed to apply! Possible dependencies:
    5d0fd3c806b9 ("[media] uvcvideo: Disable hardware timestamps by default")


How should we proceed with this patch?

--
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/1] uvc: Avoid NULL pointer dereference at the end of streaming
  2019-01-31  2:04 ` Sasha Levin
@ 2019-01-31  7:31   ` Sakari Ailus
  2019-02-01 10:26     ` Laurent Pinchart
  0 siblings, 1 reply; 4+ messages in thread
From: Sakari Ailus @ 2019-01-31  7:31 UTC (permalink / raw)
  To: Sasha Levin; +Cc: laurent.pinchart, linux-media, chiranjeevi.rapolu, stable

Hi Sasha,

On Thu, Jan 31, 2019 at 02:04:48AM +0000, Sasha Levin wrote:
> Hi,
> 
> [This is an automated email]
> 
> This commit has been processed because it contains a "Fixes:" tag,
> fixing commit: 9c0863b1cc48 [media] vb2: call buf_finish from __queue_cancel.
> 
> The bot has tested the following trees: v4.20.5, v4.19.18, v4.14.96, v4.9.153, v4.4.172, v3.18.133.
> 
> v4.20.5: Build OK!
> v4.19.18: Build OK!
> v4.14.96: Build OK!
> v4.9.153: Build OK!
> v4.4.172: Build OK!
> v3.18.133: Failed to apply! Possible dependencies:
>     5d0fd3c806b9 ("[media] uvcvideo: Disable hardware timestamps by default")
> 
> 
> How should we proceed with this patch?

IMO 5d0fd3c806b9 should be applied as well. It's effectively a bugfix as
well (but which also, for most users, covered the problem fixed by
9c0863b1cc48).

Laurent, could you confirm?

-- 
Regards,

Sakari Ailus
sakari.ailus@linux.intel.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 1/1] uvc: Avoid NULL pointer dereference at the end of streaming
  2019-01-31  7:31   ` Sakari Ailus
@ 2019-02-01 10:26     ` Laurent Pinchart
  0 siblings, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2019-02-01 10:26 UTC (permalink / raw)
  To: Sakari Ailus; +Cc: Sasha Levin, linux-media, chiranjeevi.rapolu, stable

Hello,

On Thu, Jan 31, 2019 at 09:31:07AM +0200, Sakari Ailus wrote:
> On Thu, Jan 31, 2019 at 02:04:48AM +0000, Sasha Levin wrote:
> > Hi,
> > 
> > [This is an automated email]
> > 
> > This commit has been processed because it contains a "Fixes:" tag,
> > fixing commit: 9c0863b1cc48 [media] vb2: call buf_finish from __queue_cancel.
> > 
> > The bot has tested the following trees: v4.20.5, v4.19.18, v4.14.96, v4.9.153, v4.4.172, v3.18.133.
> > 
> > v4.20.5: Build OK!
> > v4.19.18: Build OK!
> > v4.14.96: Build OK!
> > v4.9.153: Build OK!
> > v4.4.172: Build OK!
> > v3.18.133: Failed to apply! Possible dependencies:
> >     5d0fd3c806b9 ("[media] uvcvideo: Disable hardware timestamps by default")
> > 
> > 
> > How should we proceed with this patch?
> 
> IMO 5d0fd3c806b9 should be applied as well. It's effectively a bugfix as
> well (but which also, for most users, covered the problem fixed by
> 9c0863b1cc48).
> 
> Laurent, could you confirm?

That seems good to me.

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-02-01 10:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-30 10:09 [PATCH v2 1/1] uvc: Avoid NULL pointer dereference at the end of streaming Sakari Ailus
2019-01-31  2:04 ` Sasha Levin
2019-01-31  7:31   ` Sakari Ailus
2019-02-01 10:26     ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).