uvcvideo: Prevent new URBs being processed at stream stop
With asynchronous handling of the URBs from the USB Complete handler, we
get a continual stream of packets being received while we are attempting
to shutdown the stream.
Packets that have already been received and processed are on a
work-queue, but during stream shutdown the URBs that those packets
belong to are killed and free'd.
To prevent this race from causing invalid memory accesses, prevent new
URBs from being processed when uvc_stop_streaming() is called by
introducing a new flag "UVC_QUEUE_STOPPING" into the queue, and checking
this when processing the URB to be queued.
With this, we can flush the work queue, and commence a normal pipe
shutdown. Work tasks that are already queued are processed, but the URBs
are prevented from being re-submitted to the USB stack.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
v2:
- Re work critical sections for potential race
v3:
- Previous rework was incorrect - and caused a deadlock.
- Ensure that URB's are not resubmitted during work processing
(This causes USB core error reports)
- clear stopping flag bit after the stream is disabled.
diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index 869ae06..cfc34f1 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -171,10 +171,23 @@
struct uvc_streaming *stream = uvc_queue_to_stream(queue);
unsigned long flags;
+ /* Prevent new buffers coming in */
+ spin_lock_irqsave(&queue->irqlock, flags);
+ queue->flags |= UVC_QUEUE_STOPPING;
+ spin_unlock_irqrestore(&queue->irqlock, flags);
+
+ /*
+ * All pending work should be completed before disabling the stream, as
+ * all URB's will be both killed and free'd during
+ * uvc_video_enable(s, 0)
+ */
+ flush_workqueue(stream->async_wq);
+
uvc_video_enable(stream, 0);
spin_lock_irqsave(&queue->irqlock, flags);
uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
+ queue->flags &= ~(UVC_QUEUE_STOPPING);
spin_unlock_irqrestore(&queue->irqlock, flags);
}
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index b8f3221..659dea2 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1069,6 +1069,8 @@
static void uvc_video_decode_data_work(struct work_struct *work)
{
struct uvc_urb *uvc_urb = container_of(work, struct uvc_urb, work);
+ struct uvc_streaming *stream = uvc_urb->stream;
+ struct uvc_video_queue *queue = &stream->queue;
unsigned int i;
int ret;
@@ -1081,10 +1083,13 @@
uvc_queue_buffer_release(op->buf);
}
- ret = usb_submit_urb(uvc_urb->urb, GFP_ATOMIC);
- if (ret < 0) {
- uvc_printk(KERN_ERR, "Failed to resubmit video URB (%d).\n",
- ret);
+ if (!(queue->flags & UVC_QUEUE_STOPPING)) {
+ /* Prevent resubmitting URBs when shutting down */
+ ret = usb_submit_urb(uvc_urb->urb, GFP_ATOMIC);
+ if (ret < 0) {
+ uvc_printk(KERN_ERR, "Failed to resubmit video URB (%d).\n",
+ ret);
+ }
}
}
@@ -1364,6 +1369,20 @@
struct uvc_streaming *stream = uvc_urb->stream;
struct uvc_video_queue *queue = &stream->queue;
struct uvc_buffer *buf = NULL;
+ unsigned long flags;
+ bool stopping;
+
+ spin_lock_irqsave(&queue->irqlock, flags);
+ stopping = queue->flags & UVC_QUEUE_STOPPING;
+ spin_unlock_irqrestore(&queue->irqlock, flags);
+
+ /*
+ * Simply accept and discard completed URB's without processing when the
+ * stream is being shutdown. URB's will be killed and freed as part of
+ * the uvc_video_enable(s, 0) action.
+ */
+ if (stopping)
+ return;
switch (urb->status) {
case 0:
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 74a36ba..f18f88a 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -384,6 +384,7 @@
#define UVC_QUEUE_DISCONNECTED (1 << 0)
#define UVC_QUEUE_DROP_CORRUPTED (1 << 1)
+#define UVC_QUEUE_STOPPING (1 << 2)
struct uvc_video_queue {
struct vb2_queue queue;
