[PATCH] media: em28xx: hold the vb2 queue lock when releasing the queue on close

[PATCH] media: em28xx: hold the vb2 queue lock when releasing the queue on close

[Christopher Snowhill]

Commit de626073dfcb ("media: em28xx: drop 'users' field") replaced the
vb2_fop_release() call in em28xx_v4l2_close() with
_vb2_fop_release(filp, NULL) under dev->lock. vb2_fop_release() took
the vb2 queue lock, but the new call takes no lock at all.

The video and VBI queues have their own locks (vb_queue_lock and
vb_vbi_queue_lock), and v4l2_ioctl_get_lock() serializes the queue
ioctls with those rather than dev->lock. The queue can therefore be
released on close while another file handle is in the middle of
VIDIOC_REQBUFS. __vb2_queue_alloc() adds each buffer to the queue before
allocating its planes, so the release can find an MMAP buffer whose
mem_priv is still NULL and hand it to vb2_vmalloc_put():

  BUG: kernel NULL pointer dereference, address: 0000000000000020
  RIP: 0010:vb2_vmalloc_put+0xe/0x50 [videobuf2_vmalloc]
  Call Trace:
   __vb2_queue_free+0x14e/0x350 [videobuf2_common]
   vb2_core_queue_release+0x3b/0x80 [videobuf2_common]
   _vb2_fop_release+0x44/0x70 [videobuf2_v4l2]
   em28xx_v4l2_close+0x60/0x140 [em28xx_v4l]
   v4l2_release+0x9d/0xf0 [videodev]
   __fput+0xfd/0x280

This was hit with OBS on a MyGica iGrabber (card=105).

Pass the queue lock to _vb2_fop_release(). Taking it while holding
dev->lock matches the existing order in em28xx_v4l2_fini(), where
vb2_video_unregister_device() takes the queue lock under dev->lock.

Fixes: de626073dfcb ("media: em28xx: drop 'users' field")
Assisted-by: LLM
Signed-off-by: Christopher Snowhill <chris@kode54.net>
---
 drivers/media/usb/em28xx/em28xx-video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index c418add65bb5..85e7430fad57 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2474,6 +2474,7 @@ static int em28xx_v4l2_resume(struct em28xx *dev)
  */
 static int em28xx_v4l2_close(struct file *filp)
 {
+	struct video_device   *vdev = video_devdata(filp);
 	struct em28xx         *dev  = video_drvdata(filp);
 	struct em28xx_v4l2    *v4l2 = dev->v4l2;
 	struct usb_device *udev = interface_to_usbdev(dev->intf);
@@ -2482,7 +2483,7 @@ static int em28xx_v4l2_close(struct file *filp)
 
 	mutex_lock(&dev->lock);
 	last_user = v4l2_fh_is_singular_file(filp);
-	_vb2_fop_release(filp, NULL);
+	_vb2_fop_release(filp, vdev->queue->lock);
 
 	if (last_user) {
 		/* No sense to try to write to the device */
-- 
2.55.0