linux-kernel - Re: [PATCH v2 3/3] media: uvcvideo: Remove stream->is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANiDSCtyyJ9fHko5r2eFK06V7Mhrti1zTBfQ2h_GmNF5EDcU+w@mail.gmail.com>
Date: Mon, 2 Jun 2025 16:38:23 +0200
From: Ricardo Ribalda <ribalda@...omium.org>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Hans Verkuil <hans@...erkuil.nl>, Hans de Goede <hdegoede@...hat.com>, 
	Mauro Carvalho Chehab <mchehab@...nel.org>, linux-media@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] media: uvcvideo: Remove stream->is_streaming field

Hi Laurent, Hi Hans
> > If this isn't fixed, then at least add a comment explaining why you test for
> > != V4L2_BUF_TYPE_META_CAPTURE before calling uvc_pm_put. It's not obvious.
>
> Agreed.

Maybe this is better than a comment?

diff --git a/drivers/media/usb/uvc/uvc_queue.c
b/drivers/media/usb/uvc/uvc_queue.c
index 72c5494dee9f..7f9d731df32c 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -39,8 +39,6 @@ static inline struct uvc_buffer
*uvc_vbuf_to_buffer(struct vb2_v4l2_buffer *buf)

 /*
  * Return all queued buffers to videobuf2 in the requested state.
- *
- * This function must be called with the queue spinlock held.
  */
 static void uvc_queue_return_buffers(struct uvc_video_queue *queue,
                               enum uvc_buffer_state state)
@@ -49,6 +47,8 @@ static void uvc_queue_return_buffers(struct
uvc_video_queue *queue,
                                        ? VB2_BUF_STATE_ERROR
                                        : VB2_BUF_STATE_QUEUED;

+       spin_lock_irq(&queue->irqlock);
+
        while (!list_empty(&queue->irqqueue)) {
                struct uvc_buffer *buf = list_first_entry(&queue->irqqueue,
                                                          struct uvc_buffer,
@@ -57,6 +57,8 @@ static void uvc_queue_return_buffers(struct
uvc_video_queue *queue,
                buf->state = state;
                vb2_buffer_done(&buf->buf.vb2_buf, vb2_state);
        }
+
+       spin_unlock_irq(&queue->irqlock);
 }

 /* -----------------------------------------------------------------------------
@@ -157,7 +159,7 @@ static void uvc_buffer_finish(struct vb2_buffer *vb)
                uvc_video_clock_update(stream, vbuf, buf);
 }

-static int uvc_start_streaming(struct vb2_queue *vq, unsigned int count)
+static int uvc_start_streaming_video(struct vb2_queue *vq, unsigned int count)
 {
        struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
        struct uvc_streaming *stream = uvc_queue_to_stream(queue);
@@ -171,25 +173,29 @@ static int uvc_start_streaming(struct vb2_queue
*vq, unsigned int count)
        if (ret == 0)
                return 0;

-       spin_lock_irq(&queue->irqlock);
        uvc_queue_return_buffers(queue, UVC_BUF_STATE_QUEUED);
-       spin_unlock_irq(&queue->irqlock);

        return ret;
 }

-static void uvc_stop_streaming(struct vb2_queue *vq)
+static void uvc_stop_streaming_meta(struct vb2_queue *vq)
 {
        struct uvc_video_queue *queue = vb2_get_drv_priv(vq);

        lockdep_assert_irqs_enabled();

-       if (vq->type != V4L2_BUF_TYPE_META_CAPTURE)
-               uvc_video_stop_streaming(uvc_queue_to_stream(queue));
+       uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
+}
+
+static void uvc_stop_streaming_video(struct vb2_queue *vq)
+{
+       struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
+
+       lockdep_assert_irqs_enabled();
+
+       uvc_video_stop_streaming(uvc_queue_to_stream(queue));

-       spin_lock_irq(&queue->irqlock);
        uvc_queue_return_buffers(queue, UVC_BUF_STATE_ERROR);
-       spin_unlock_irq(&queue->irqlock);
 }

 static const struct vb2_ops uvc_queue_qops = {
@@ -197,15 +203,15 @@ static const struct vb2_ops uvc_queue_qops = {
        .buf_prepare = uvc_buffer_prepare,
        .buf_queue = uvc_buffer_queue,
        .buf_finish = uvc_buffer_finish,
-       .start_streaming = uvc_start_streaming,
-       .stop_streaming = uvc_stop_streaming,
+       .start_streaming = uvc_start_streaming_video,
+       .stop_streaming = uvc_stop_streaming_video,
 };

 static const struct vb2_ops uvc_meta_queue_qops = {
        .queue_setup = uvc_queue_setup,
        .buf_prepare = uvc_buffer_prepare,
        .buf_queue = uvc_buffer_queue,
-       .stop_streaming = uvc_stop_streaming,
+       .stop_streaming = uvc_stop_streaming_meta,
 };

 int uvc_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type)



-- 
Ricardo Ribalda