V4L/DVB (13831): uvcvideo: Fix oops caused by a race condition in buffer dequeuing

Buffers were marked as done before being removed from the IRQ queue. If
a userspace application dequeued and requeued the buffer fast enough
during that time window, the buffer could end up being deleted twice,
generating an oops in interrupt context.

Add a new state, UVC_BUF_STATE_READY, to mark buffers as ready for reuse
but not yet removed from the queue, and transition to UVC_BUF_STATE_DONE
only when the buffer is removed from the queue.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

diff --git a/drivers/media/video/uvc/uvc_queue.c b/drivers/media/video/uvc/uvc_queue.c
index f854698c40618a4a5225209f1060933f1aafb430..ea11839cba4a439f92461d899910171d1c9f41bb 100644 (file)
--- a/drivers/media/video/uvc/uvc_queue.c
+++ b/drivers/media/video/uvc/uvc_queue.c
@@ -59,9 +59,9 @@
  *    returns immediately.
  *
  *    When the buffer is full, the completion handler removes it from the irq
- *    queue, marks it as ready (UVC_BUF_STATE_DONE) and wakes its wait queue.
+ *    queue, marks it as done (UVC_BUF_STATE_DONE) and wakes its wait queue.
  *    At that point, any process waiting on the buffer will be woken up. If a
- *    process tries to dequeue a buffer after it has been marked ready, the
+ *    process tries to dequeue a buffer after it has been marked done, the
  *    dequeing will succeed immediately.
  *
  * 2. Buffers are queued, user is waiting on a buffer and the device gets
@@ -201,6 +201,7 @@ static void __uvc_query_buffer(struct uvc_buffer *buf,
                break;
        case UVC_BUF_STATE_QUEUED:
        case UVC_BUF_STATE_ACTIVE:
+       case UVC_BUF_STATE_READY:
                v4l2_buf->flags |= V4L2_BUF_FLAG_QUEUED;
                break;
        case UVC_BUF_STATE_IDLE:
@@ -295,13 +296,15 @@ static int uvc_queue_waiton(struct uvc_buffer *buf, int nonblocking)
 {
        if (nonblocking) {
                return (buf->state != UVC_BUF_STATE_QUEUED &&
-                       buf->state != UVC_BUF_STATE_ACTIVE)
+                       buf->state != UVC_BUF_STATE_ACTIVE &&
+                       buf->state != UVC_BUF_STATE_READY)
                        ? 0 : -EAGAIN;
        }
 
        return wait_event_interruptible(buf->wait,
                buf->state != UVC_BUF_STATE_QUEUED &&
-               buf->state != UVC_BUF_STATE_ACTIVE);
+               buf->state != UVC_BUF_STATE_ACTIVE &&
+               buf->state != UVC_BUF_STATE_READY);
 }
 
 /*
@@ -348,6 +351,7 @@ int uvc_dequeue_buffer(struct uvc_video_queue *queue,
        case UVC_BUF_STATE_IDLE:
        case UVC_BUF_STATE_QUEUED:
        case UVC_BUF_STATE_ACTIVE:
+       case UVC_BUF_STATE_READY:
        default:
                uvc_trace(UVC_TRACE_CAPTURE, "[E] Invalid buffer state %u "
                        "(driver bug?).\n", buf->state);
@@ -489,6 +493,7 @@ struct uvc_buffer *uvc_queue_next_buffer(struct uvc_video_queue *queue,
 
        spin_lock_irqsave(&queue->irqlock, flags);
        list_del(&buf->queue);
+       buf->state = UVC_BUF_STATE_DONE;
        if (!list_empty(&queue->irqqueue))
                nextbuf = list_first_entry(&queue->irqqueue, struct uvc_buffer,
                                           queue);

diff --git a/drivers/media/video/uvc/uvc_video.c b/drivers/media/video/uvc/uvc_video.c
index e8cc0a9ddadd94287d4de3ce52aa2ccdc6dea9f4..7dcf534a0cf350c28844a766335a3f121b0476c2 100644 (file)
--- a/drivers/media/video/uvc/uvc_video.c
+++ b/drivers/media/video/uvc/uvc_video.c
@@ -441,7 +441,7 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
        if (fid != stream->last_fid && buf->buf.bytesused != 0) {
                uvc_trace(UVC_TRACE_FRAME, "Frame complete (FID bit "
                                "toggled).\n");
-               buf->state = UVC_BUF_STATE_DONE;
+               buf->state = UVC_BUF_STATE_READY;
                return -EAGAIN;
        }
 
@@ -470,7 +470,7 @@ static void uvc_video_decode_data(struct uvc_streaming *stream,
        /* Complete the current frame if the buffer size was exceeded. */
        if (len > maxlen) {
                uvc_trace(UVC_TRACE_FRAME, "Frame complete (overflow).\n");
-               buf->state = UVC_BUF_STATE_DONE;
+               buf->state = UVC_BUF_STATE_READY;
        }
 }
 
@@ -482,7 +482,7 @@ static void uvc_video_decode_end(struct uvc_streaming *stream,
                uvc_trace(UVC_TRACE_FRAME, "Frame complete (EOF found).\n");
                if (data[0] == len)
                        uvc_trace(UVC_TRACE_FRAME, "EOF in empty payload.\n");
-               buf->state = UVC_BUF_STATE_DONE;
+               buf->state = UVC_BUF_STATE_READY;
                if (stream->dev->quirks & UVC_QUIRK_STREAM_NO_FID)
                        stream->last_fid ^= UVC_STREAM_FID;
        }
@@ -568,8 +568,7 @@ static void uvc_video_decode_isoc(struct urb *urb, struct uvc_streaming *stream,
                uvc_video_decode_end(stream, buf, mem,
                        urb->iso_frame_desc[i].actual_length);
 
-               if (buf->state == UVC_BUF_STATE_DONE ||
-                   buf->state == UVC_BUF_STATE_ERROR)
+               if (buf->state == UVC_BUF_STATE_READY)
                        buf = uvc_queue_next_buffer(&stream->queue, buf);
        }
 }
@@ -627,8 +626,7 @@ static void uvc_video_decode_bulk(struct urb *urb, struct uvc_streaming *stream,
                if (!stream->bulk.skip_payload && buf != NULL) {
                        uvc_video_decode_end(stream, buf, stream->bulk.header,
                                stream->bulk.payload_size);
-                       if (buf->state == UVC_BUF_STATE_DONE ||
-                           buf->state == UVC_BUF_STATE_ERROR)
+                       if (buf->state == UVC_BUF_STATE_READY)
                                buf = uvc_queue_next_buffer(&stream->queue,
                                                            buf);
                }
@@ -669,7 +667,7 @@ static void uvc_video_encode_bulk(struct urb *urb, struct uvc_streaming *stream,
            stream->bulk.payload_size == stream->bulk.max_payload_size) {
                if (buf->buf.bytesused == stream->queue.buf_used) {
                        stream->queue.buf_used = 0;
-                       buf->state = UVC_BUF_STATE_DONE;
+                       buf->state = UVC_BUF_STATE_READY;
                        uvc_queue_next_buffer(&stream->queue, buf);
                        stream->last_fid ^= UVC_STREAM_FID;
                }

diff --git a/drivers/media/video/uvc/uvcvideo.h b/drivers/media/video/uvc/uvcvideo.h
index 7ec9a04ced5055f0dde006788aa1437be7f6dedd..2337585001ea01be3d31c4300c9ec303e9580031 100644 (file)
--- a/drivers/media/video/uvc/uvcvideo.h
+++ b/drivers/media/video/uvc/uvcvideo.h
@@ -365,8 +365,9 @@ enum uvc_buffer_state {
        UVC_BUF_STATE_IDLE      = 0,
        UVC_BUF_STATE_QUEUED    = 1,
        UVC_BUF_STATE_ACTIVE    = 2,
-       UVC_BUF_STATE_DONE      = 3,
-       UVC_BUF_STATE_ERROR     = 4,
+       UVC_BUF_STATE_READY     = 3,
+       UVC_BUF_STATE_DONE      = 4,
+       UVC_BUF_STATE_ERROR     = 5,
 };
 
 struct uvc_buffer {