[PATCH] media: rtl2832_sdr: release URBs and stream buffers on start_streaming() failure

[PATCH] media: rtl2832_sdr: release URBs and stream buffers on start_streaming() failure

[Valery Borovsky]

rtl2832_sdr_start_streaming() calls rtl2832_sdr_alloc_stream_bufs(),
rtl2832_sdr_alloc_urbs() and rtl2832_sdr_submit_urbs() in sequence and
shares a single err: label that only unlocks the mutex and returns.
When alloc_urbs() succeeds but submit_urbs() fails, or when alloc_urbs()
itself returns -ENOMEM after alloc_stream_bufs() has already succeeded,
the URBs and/or the coherent DMA stream buffers stay allocated while
streaming reports failure to vb2. Two latent defects follow on the next
VIDIOC_STREAMON:

1) rtl2832_sdr_alloc_stream_bufs() unconditionally resets dev->buf_num
   to 0 and overwrites dev->buf_list[]/dev->dma_addr[], permanently
   leaking the coherent DMA memory allocated by the previous attempt.

2) rtl2832_sdr_alloc_urbs() never resets dev->urbs_initialized and only
   increments it. After a second successful pass urbs_initialized can
   exceed MAX_BULK_BUFS, so the subsequent rtl2832_sdr_free_urbs() walks
   from urbs_initialized - 1 down to 0 and reads past the end of
   dev->urb_list[], passing garbage pointers to usb_free_urb().

Mirror the teardown that stop_streaming() already performs: on the error
path call rtl2832_sdr_free_urbs() and rtl2832_sdr_free_stream_bufs()
before unlocking. Both helpers are idempotent (free_urbs kills and zeros
urbs_initialized; free_stream_bufs is gated on URB_BUF and clears the
buf_num counter), so partial-failure paths and the no-allocation paths
remain safe.

Issue identified by automated review of the INV-003 series at
https://sashiko.dev/

Fixes: 771138920eaf ("[media] rtl2832_sdr: Realtek RTL2832 SDR driver module")
Cc: stable@vger.kernel.org
Signed-off-by: Valery Borovsky <vebohr@gmail.com>
---
 drivers/media/dvb-frontends/rtl2832_sdr.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/dvb-frontends/rtl2832_sdr.c b/drivers/media/dvb-frontends/rtl2832_sdr.c
index 422d1a7b5456..efcef1317cf9 100644
--- a/drivers/media/dvb-frontends/rtl2832_sdr.c
+++ b/drivers/media/dvb-frontends/rtl2832_sdr.c
@@ -900,7 +900,13 @@ static int rtl2832_sdr_start_streaming(struct vb2_queue *vq, unsigned int count)
 	if (ret)
 		goto err;
 
+	mutex_unlock(&dev->v4l2_lock);
+
+	return 0;
+
 err:
+	rtl2832_sdr_free_urbs(dev);
+	rtl2832_sdr_free_stream_bufs(dev);
 	mutex_unlock(&dev->v4l2_lock);
 
 	return ret;
-- 
2.51.0

Re: [PATCH] media: rtl2832_sdr: release URBs and stream buffers on start_streaming() failure

[Hans Verkuil]

On 13/05/2026 07:57, Valery Borovsky wrote:
> rtl2832_sdr_start_streaming() calls rtl2832_sdr_alloc_stream_bufs(),
> rtl2832_sdr_alloc_urbs() and rtl2832_sdr_submit_urbs() in sequence and
> shares a single err: label that only unlocks the mutex and returns.
> When alloc_urbs() succeeds but submit_urbs() fails, or when alloc_urbs()
> itself returns -ENOMEM after alloc_stream_bufs() has already succeeded,
> the URBs and/or the coherent DMA stream buffers stay allocated while
> streaming reports failure to vb2. Two latent defects follow on the next
> VIDIOC_STREAMON:
> 
> 1) rtl2832_sdr_alloc_stream_bufs() unconditionally resets dev->buf_num
>    to 0 and overwrites dev->buf_list[]/dev->dma_addr[], permanently
>    leaking the coherent DMA memory allocated by the previous attempt.
> 
> 2) rtl2832_sdr_alloc_urbs() never resets dev->urbs_initialized and only
>    increments it. After a second successful pass urbs_initialized can
>    exceed MAX_BULK_BUFS, so the subsequent rtl2832_sdr_free_urbs() walks
>    from urbs_initialized - 1 down to 0 and reads past the end of
>    dev->urb_list[], passing garbage pointers to usb_free_urb().
> 
> Mirror the teardown that stop_streaming() already performs: on the error
> path call rtl2832_sdr_free_urbs() and rtl2832_sdr_free_stream_bufs()
> before unlocking. Both helpers are idempotent (free_urbs kills and zeros
> urbs_initialized; free_stream_bufs is gated on URB_BUF and clears the
> buf_num counter), so partial-failure paths and the no-allocation paths
> remain safe.
> 
> Issue identified by automated review of the INV-003 series at
> https://sashiko.dev/
> 
> Fixes: 771138920eaf ("[media] rtl2832_sdr: Realtek RTL2832 SDR driver module")
> Cc: stable@vger.kernel.org
> Signed-off-by: Valery Borovsky <vebohr@gmail.com>
> ---
>  drivers/media/dvb-frontends/rtl2832_sdr.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/dvb-frontends/rtl2832_sdr.c b/drivers/media/dvb-frontends/rtl2832_sdr.c
> index 422d1a7b5456..efcef1317cf9 100644
> --- a/drivers/media/dvb-frontends/rtl2832_sdr.c
> +++ b/drivers/media/dvb-frontends/rtl2832_sdr.c
> @@ -900,7 +900,13 @@ static int rtl2832_sdr_start_streaming(struct vb2_queue *vq, unsigned int count)
>  	if (ret)
>  		goto err;
>  
> +	mutex_unlock(&dev->v4l2_lock);
> +
> +	return 0;
> +
>  err:
> +	rtl2832_sdr_free_urbs(dev);
> +	rtl2832_sdr_free_stream_bufs(dev);
>  	mutex_unlock(&dev->v4l2_lock);
>  
>  	return ret;

This patch no longer applies to the next branch of
https://gitlab.freedesktop.org/linux-media/media-committers/

Please rebase and repost.

Regards,

	Hans

[PATCH v2] media: rtl2832_sdr: release URBs and stream buffers on start_streaming() failure

[Valery Borovsky]

rtl2832_sdr_start_streaming() calls rtl2832_sdr_alloc_stream_bufs(),
rtl2832_sdr_alloc_urbs() and rtl2832_sdr_submit_urbs() in sequence and
shares a single err: label that only unlocks the mutex and returns.
When alloc_urbs() succeeds but submit_urbs() fails, or when alloc_urbs()
itself returns -ENOMEM after alloc_stream_bufs() has already succeeded,
the URBs and/or the coherent DMA stream buffers stay allocated while
streaming reports failure to vb2. Two latent defects follow on the next
VIDIOC_STREAMON:

1) rtl2832_sdr_alloc_stream_bufs() unconditionally resets dev->buf_num
   to 0 and overwrites dev->buf_list[]/dev->dma_addr[], permanently
   leaking the coherent DMA memory allocated by the previous attempt.

2) rtl2832_sdr_alloc_urbs() never resets dev->urbs_initialized and only
   increments it. After a second successful pass urbs_initialized can
   exceed MAX_BULK_BUFS, so the subsequent rtl2832_sdr_free_urbs() walks
   from urbs_initialized - 1 down to 0 and reads past the end of
   dev->urb_list[], passing garbage pointers to usb_free_urb().

Mirror the teardown that stop_streaming() already performs: on the error
path call rtl2832_sdr_free_urbs() and rtl2832_sdr_free_stream_bufs()
before unlocking. Both helpers are idempotent (free_urbs kills and zeros
urbs_initialized; free_stream_bufs is gated on URB_BUF and clears the
buf_num counter), so partial-failure paths and the no-allocation paths
remain safe.

Issue identified by automated review of the INV-003 series at
https://sashiko.dev/

Fixes: 771138920eaf ("[media] rtl2832_sdr: Realtek RTL2832 SDR driver module")
Cc: stable@vger.kernel.org
Signed-off-by: Valery Borovsky <vebohr@gmail.com>
---
Changes since v1
(https://lore.kernel.org/linux-media/20260513055733.146905-1-vebohr@gmail.com/):
- Rebased on media-committers/next. The err: label in
  rtl2832_sdr_start_streaming() now also calls
  rtl2832_sdr_cleanup_queued_bufs(dev, VB2_BUF_STATE_QUEUED) from
  commit 33ca0aab6f4b ("media: rtl2832_sdr: Return queued buffers on
  start_streaming() failure"); free_urbs()/free_stream_bufs() are placed
  before that cleanup, matching the order in stop_streaming(). No
  semantic change to v1.

 drivers/media/dvb-frontends/rtl2832_sdr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/dvb-frontends/rtl2832_sdr.c b/drivers/media/dvb-frontends/rtl2832_sdr.c
index c564485e3bbb..036b67a17b7a 100644
--- a/drivers/media/dvb-frontends/rtl2832_sdr.c
+++ b/drivers/media/dvb-frontends/rtl2832_sdr.c
@@ -906,9 +906,12 @@ static int rtl2832_sdr_start_streaming(struct vb2_queue *vq, unsigned int count)
 		goto err;
 
 	mutex_unlock(&dev->v4l2_lock);
+
 	return 0;
 
 err:
+	rtl2832_sdr_free_urbs(dev);
+	rtl2832_sdr_free_stream_bufs(dev);
 	rtl2832_sdr_cleanup_queued_bufs(dev, VB2_BUF_STATE_QUEUED);
 	mutex_unlock(&dev->v4l2_lock);
 
-- 
2.51.0