From: YuanZhe Liu <25031212351@stu.xidian.edu.cn> gspca enables the data transfers before it knows whether start_streaming() will succeed: gspca_init_transfer() sets gspca_dev->streaming before the URBs are submitted, so an URB that has already been submitted can complete and gspca_frame_add() can hand its buffer to videobuf2 while the remaining URBs are still being submitted. When one of those submissions fails, gspca_start_streaming() returns the buffers that are still queued to vb2 in QUEUED state, but the buffer that has already been completed is no longer on the driver's queue, and vb2_buffer_done() cannot take it back (it only accepts active buffers). vb2_start_streaming() then warns because the driver left a buffer on the done list although it failed to start: ------------[ cut here ]------------ WARNING: drivers/media/common/videobuf2/videobuf2-core.c:1821 at vb2_start_streaming+0x312/0x680 ... Call Trace: vb2_core_streamon+0x285/0x470 __vb2_init_fileio+0xdc1/0x1120 __vb2_perform_fileio+0xa7a/0x15e0 vb2_fop_read+0x219/0x3e0 v4l2_read+0x1fc/0x320 vfs_read+0x1d9/0x980 ksys_read+0x111/0x210 ... gspca_main: set alt 0 err -19 gspca_main: usb_submit_urb alt 1 err -2 This was observed on a camera whose USB configuration was changed from userspace while a read() was starting the stream: the endpoint is removed, so usb_submit_urb() fails with -ENOENT, and the driver is unbound. Do not complete buffers before the transfers are fully set up. A new streaming_ready flag is set at the end of gspca_init_transfer() only when it succeeded, so a failing start_streaming() always leaves the done list empty. Frames that arrive before that are dropped, just like frames that arrive when no buffer is queued; the window lasts only as long as the remaining URBs are still being submitted. vb2_is_streaming() cannot be used for this check: videobuf2 sets q->streaming before calling start_streaming() when VIDIOC_STREAMON comes in with fewer buffers queued than min_queued_buffers, and start_streaming() is then called later from vb2_core_qbuf(). Fixes: 1f5965c4dfd7 ("media: gspca: convert to vb2") Cc: stable@vger.kernel.org Co-developed-by: Bin Yu Signed-off-by: Bin Yu Co-developed-by: Cheng Wen Signed-off-by: Cheng Wen Signed-off-by: YuanZhe Liu <25031212351@stu.xidian.edu.cn> --- Changes in v2: - Do not gate the completion on vb2_is_streaming(): videobuf2 sets q->streaming before calling start_streaming() when VIDIOC_STREAMON is issued with fewer buffers queued than min_queued_buffers, so that call site was still reachable and the warning could still be triggered. Track in the driver whether gspca_init_transfer() has finished setting up the transfers instead. drivers/media/usb/gspca/gspca.c | 23 +++++++++++++++++++++-- drivers/media/usb/gspca/gspca.h | 1 + 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c index 594d73e50b9f..3f5ece57d66a 100644 --- a/drivers/media/usb/gspca/gspca.c +++ b/drivers/media/usb/gspca/gspca.c @@ -392,8 +392,23 @@ void gspca_frame_add(struct gspca_dev *gspca_dev, spin_unlock_irqrestore(&gspca_dev->qlock, flags); if (packet_type == FIRST_PACKET) { - /* if there is no queued buffer, discard the whole frame */ - if (!buf) { + /* + * If there is no queued buffer, discard the whole frame. + * + * Frames are also discarded until gspca_init_transfer() has + * set up the transfers: the first URB can complete while the + * others are still being submitted, and if a submission then + * fails, start_streaming() must leave vb2's done list empty. + * A completed buffer cannot be taken back, so frames that + * run into that window are dropped. + * + * vb2_is_streaming() cannot be used for this: videobuf2 sets + * q->streaming before calling start_streaming() when + * VIDIOC_STREAMON comes in with fewer buffers queued than + * min_queued_buffers, and start_streaming() is only called + * later from vb2_core_qbuf(). + */ + if (!buf || !gspca_dev->streaming_ready) { gspca_dev->last_packet_type = DISCARD_PACKET; gspca_dev->sequence++; return; @@ -732,6 +747,7 @@ static int create_urbs(struct gspca_dev *gspca_dev, static void gspca_stream_off(struct gspca_dev *gspca_dev) { gspca_dev->streaming = false; + gspca_dev->streaming_ready = false; gspca_dev->usb_err = 0; if (gspca_dev->sd_desc->stopN) gspca_dev->sd_desc->stopN(gspca_dev); @@ -887,6 +903,9 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev) } } out: + /* the transfers are set up now, buffers may be completed */ + if (!ret) + gspca_dev->streaming_ready = true; gspca_input_create_urb(gspca_dev); return ret; } diff --git a/drivers/media/usb/gspca/gspca.h b/drivers/media/usb/gspca/gspca.h index a6554d5e9e1a..3e0b9e27f4b1 100644 --- a/drivers/media/usb/gspca/gspca.h +++ b/drivers/media/usb/gspca/gspca.h @@ -187,6 +187,7 @@ struct gspca_dev { __u8 last_packet_type; __s8 empty_packet; /* if (-1) don't check empty packets */ bool streaming; + bool streaming_ready; /* buffers can be completed */ __u8 curr_mode; /* current camera mode */ struct v4l2_pix_format pixfmt; /* current mode parameters */ -- 2.34.1