1. Patchew
  2. linux
  3. View series

[PATCH] media: airspy: Guard stop_streaming() against disconnected device

Valery Borovsky posted 1 patch 1 month ago 
drivers/media/usb/airspy/airspy.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
[PATCH] media: airspy: Guard stop_streaming() against disconnected device
Posted by Valery Borovsky 1 month ago 
airspy_disconnect() clears s->udev under v4l2_lock, but
airspy_stop_streaming() unconditionally calls airspy_ctrl_msg() and
airspy_free_stream_bufs() afterwards. If a streaming user closes the
device after disconnect, stop_streaming() runs and dereferences the
NULL s->udev:

  airspy_stop_streaming()
    airspy_ctrl_msg(s, CMD_RECEIVER_MODE, 0, 0, NULL, 0)
      usb_sndctrlpipe(s->udev, 0)         /* NULL deref */
    airspy_free_stream_bufs(s)
      usb_free_coherent(s->udev, ...)     /* NULL deref */

Mirror the precedent set by sibling SDR drivers msi2500 and pwc, which
already guard their hardware teardown block with an "if (udev)" check.
The queued-buffer drain via airspy_cleanup_queued_bufs() must still
run unconditionally so vb2 sees its buffers returned.

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

Fixes: 634fe5033951 ("[media] airspy: AirSpy SDR driver")
Cc: stable@vger.kernel.org
Signed-off-by: Valery Borovsky <vebohr@gmail.com>
---
 drivers/media/usb/airspy/airspy.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/airspy/airspy.c b/drivers/media/usb/airspy/airspy.c
index 8f6b721ba107..50db02d35213 100644
--- a/drivers/media/usb/airspy/airspy.c
+++ b/drivers/media/usb/airspy/airspy.c
@@ -584,12 +584,14 @@ static void airspy_stop_streaming(struct vb2_queue *vq)
 
 	mutex_lock(&s->v4l2_lock);
 
-	/* stop hardware streaming */
-	airspy_ctrl_msg(s, CMD_RECEIVER_MODE, 0, 0, NULL, 0);
+	if (s->udev) {
+		/* stop hardware streaming */
+		airspy_ctrl_msg(s, CMD_RECEIVER_MODE, 0, 0, NULL, 0);
 
-	airspy_kill_urbs(s);
-	airspy_free_urbs(s);
-	airspy_free_stream_bufs(s);
+		airspy_kill_urbs(s);
+		airspy_free_urbs(s);
+		airspy_free_stream_bufs(s);
+	}
 
 	airspy_cleanup_queued_bufs(s);
 
-- 
2.51.0
Re: [PATCH] media: airspy: Guard stop_streaming() against disconnected device
Posted by Hans Verkuil 3 weeks, 2 days ago 
On 13/05/2026 07:26, Valery Borovsky wrote:
> airspy_disconnect() clears s->udev under v4l2_lock, but
> airspy_stop_streaming() unconditionally calls airspy_ctrl_msg() and
> airspy_free_stream_bufs() afterwards. If a streaming user closes the
> device after disconnect, stop_streaming() runs and dereferences the
> NULL s->udev:
> 
>   airspy_stop_streaming()
>     airspy_ctrl_msg(s, CMD_RECEIVER_MODE, 0, 0, NULL, 0)
>       usb_sndctrlpipe(s->udev, 0)         /* NULL deref */
>     airspy_free_stream_bufs(s)
>       usb_free_coherent(s->udev, ...)     /* NULL deref */
> 
> Mirror the precedent set by sibling SDR drivers msi2500 and pwc, which
> already guard their hardware teardown block with an "if (udev)" check.
> The queued-buffer drain via airspy_cleanup_queued_bufs() must still
> run unconditionally so vb2 sees its buffers returned.
> 
> Issue identified by automated review of the INV-003 series at
> https://sashiko.dev/
> 
> Fixes: 634fe5033951 ("[media] airspy: AirSpy SDR driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Valery Borovsky <vebohr@gmail.com>
> ---
>  drivers/media/usb/airspy/airspy.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/usb/airspy/airspy.c b/drivers/media/usb/airspy/airspy.c
> index 8f6b721ba107..50db02d35213 100644
> --- a/drivers/media/usb/airspy/airspy.c
> +++ b/drivers/media/usb/airspy/airspy.c
> @@ -584,12 +584,14 @@ static void airspy_stop_streaming(struct vb2_queue *vq)
>  
>  	mutex_lock(&s->v4l2_lock);
>  
> -	/* stop hardware streaming */
> -	airspy_ctrl_msg(s, CMD_RECEIVER_MODE, 0, 0, NULL, 0);
> +	if (s->udev) {
> +		/* stop hardware streaming */
> +		airspy_ctrl_msg(s, CMD_RECEIVER_MODE, 0, 0, NULL, 0);
>  
> -	airspy_kill_urbs(s);
> -	airspy_free_urbs(s);
> -	airspy_free_stream_bufs(s);
> +		airspy_kill_urbs(s);
> +		airspy_free_urbs(s);
> +		airspy_free_stream_bufs(s);
> +	}
>  
>  	airspy_cleanup_queued_bufs(s);
>  

Here too it is better to replace video_unregister_device(&dev->vdev);
by vb2_video_unregister_device(&dev->vdev);

Similar to what I suggested for rtl2832_sdr.

Regards,

	Hans
[PATCH v2] media: airspy: use vb2_video_unregister_device() on disconnect to fix NULL deref
Posted by Valery Borovsky 2 weeks, 6 days ago 
airspy_disconnect() clears s->udev under v4l2_lock, but
airspy_stop_streaming() unconditionally calls airspy_ctrl_msg() and
airspy_free_stream_bufs() afterwards. If a streaming user closes the
device after disconnect, stop_streaming() runs and dereferences the
NULL s->udev:

  airspy_stop_streaming()
    airspy_ctrl_msg(s, CMD_RECEIVER_MODE, 0, 0, NULL, 0)
      usb_sndctrlpipe(s->udev, 0)         /* NULL deref */
    airspy_free_stream_bufs(s)
      usb_free_coherent(s->udev, ...)     /* NULL deref */

The airspy driver uses vb2_fop_release() in its file_operations, so
replace video_unregister_device(&s->vdev) with
vb2_video_unregister_device(&s->vdev) and move it before clearing
s->udev. vb2_video_unregister_device() releases the vb2 queue, which
synchronously runs airspy_stop_streaming() if streaming is active, so
the URBs, coherent DMA stream buffers and the hardware stop control
message all execute while s->udev is still valid.

vb2_video_unregister_device() locks vdev->queue->lock (vb_queue_lock)
internally, and stop_streaming() locks v4l2_lock, so the previous outer
mutex_lock(&s->vb_queue_lock) / mutex_lock(&s->v4l2_lock) pair around
the unregister sequence would self-deadlock and has been removed. A
short v4l2_lock critical section around s->udev = NULL remains so any
ioctl path that still holds the file descriptor sees coherent state.

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

Fixes: 634fe5033951 ("[media] airspy: AirSpy SDR driver")
Cc: stable@vger.kernel.org
Suggested-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Valery Borovsky <vebohr@gmail.com>
---
Changes since v1
(https://lore.kernel.org/linux-media/20260513052617.140688-1-vebohr@gmail.com/):
- Rewritten per Hans Verkuil's review
  (https://lore.kernel.org/linux-media/f202c8ae-554f-49de-a9d1-add337e28515@kernel.org/):
  fix the root cause in airspy_disconnect() by replacing
  video_unregister_device() with vb2_video_unregister_device() and
  moving it before clearing s->udev, instead of guarding the hardware
  teardown in airspy_stop_streaming() with "if (s->udev)".
  vb2_video_unregister_device() releases the queue, which synchronously
  calls stop_streaming() while s->udev is still valid, so the guard is
  no longer needed; airspy_stop_streaming() is unchanged.
- Dropped the outer mutex_lock(&s->vb_queue_lock) /
  mutex_lock(&s->v4l2_lock) around the unregister sequence:
  vb2_video_unregister_device() acquires vb_queue_lock internally and
  stop_streaming() acquires v4l2_lock, so holding either of those
  while calling the unregister helper self-deadlocks.
- Rebased on media-committers/next.

 drivers/media/usb/airspy/airspy.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/airspy/airspy.c b/drivers/media/usb/airspy/airspy.c
index 57edb42463e8..358a66ab8e48 100644
--- a/drivers/media/usb/airspy/airspy.c
+++ b/drivers/media/usb/airspy/airspy.c
@@ -464,14 +464,21 @@ static void airspy_disconnect(struct usb_interface *intf)
 
 	dev_dbg(s->dev, "\n");
 
-	mutex_lock(&s->vb_queue_lock);
+	/*
+	 * vb2_video_unregister_device() releases the vb2 queue, which
+	 * triggers airspy_stop_streaming() if streaming is active.
+	 * stop_streaming() dereferences s->udev via airspy_ctrl_msg() and
+	 * airspy_free_stream_bufs(), so it must run before s->udev is
+	 * cleared. vb2_video_unregister_device() locks vb_queue_lock
+	 * internally and stop_streaming() locks v4l2_lock, so neither may
+	 * be held by the caller.
+	 */
+	v4l2_device_disconnect(&s->v4l2_dev);
+	vb2_video_unregister_device(&s->vdev);
+
 	mutex_lock(&s->v4l2_lock);
-	/* No need to keep the urbs around after disconnection */
 	s->udev = NULL;
-	v4l2_device_disconnect(&s->v4l2_dev);
-	video_unregister_device(&s->vdev);
 	mutex_unlock(&s->v4l2_lock);
-	mutex_unlock(&s->vb_queue_lock);
 
 	v4l2_device_put(&s->v4l2_dev);
 }
-- 
2.51.0

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.