mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-30 16:53:20 -04:00
media: airspy: use vb2_video_unregister_device() on disconnect to fix NULL deref
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: 634fe50339 ("[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>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
This commit is contained in:
committed by
Hans Verkuil
parent
dabb047c62
commit
2f378dc45e
@@ -464,14 +464,21 @@ static void airspy_disconnect(struct usb_interface *intf)
|
||||
|
||||
dev_dbg(s->dev, "\n");
|
||||
|
||||
mutex_lock(&s->vb_queue_lock);
|
||||
mutex_lock(&s->v4l2_lock);
|
||||
/* No need to keep the urbs around after disconnection */
|
||||
s->udev = NULL;
|
||||
/*
|
||||
* 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);
|
||||
video_unregister_device(&s->vdev);
|
||||
vb2_video_unregister_device(&s->vdev);
|
||||
|
||||
mutex_lock(&s->v4l2_lock);
|
||||
s->udev = NULL;
|
||||
mutex_unlock(&s->v4l2_lock);
|
||||
mutex_unlock(&s->vb_queue_lock);
|
||||
|
||||
v4l2_device_put(&s->v4l2_dev);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user