[PATCH] media: em28xx: keep device state alive for registered video nodes

[Yousef Alhouseen]

The V4L2 core takes a video_device reference before invoking the
driver open callback. That reference does not protect em28xx state
because all three video_device objects are embedded in em28xx_v4l2 and
use video_device_release_empty().

If initialization fails after registering a node, the error path can
unregister it and drop the last em28xx_v4l2 reference while a concurrent
open has passed the core registration check. The open callback then
dereferences the freed video_device in video_drvdata(), as observed by
KASAN. A disconnect has the same lifetime gap.

Give each successfully registered video node references to both the
enclosing V4L2 state and the parent em28xx device. Release those
references from the video_device release callback, after the core has
drained pending opens and existing file references.

Fixes: ef74a0b9ff56 ("[media] em28xx: move video_device structs from struct em28xx to struct v4l2")
Reported-by: syzbot+39ff299961a7c07f00f0@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=39ff299961a7c07f00f0
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yousef Alhouseen <alhouseenyousef@xxxxxxxxx>
---
 drivers/media/usb/em28xx/em28xx-video.c | 35 +++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index da0422c65e5f..4274a9bcb432 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2289,6 +2289,31 @@ static void em28xx_free_v4l2(struct kref *ref)
 	kfree(v4l2);
 }
 
+static void em28xx_vdev_release(struct video_device *vdev)
+{
+	struct em28xx_v4l2 *v4l2;
+	struct em28xx *dev;
+
+	switch (vdev->vfl_type) {
+	case VFL_TYPE_VIDEO:
+		v4l2 = container_of(vdev, struct em28xx_v4l2, vdev);
+		break;
+	case VFL_TYPE_VBI:
+		v4l2 = container_of(vdev, struct em28xx_v4l2, vbi_dev);
+		break;
+	case VFL_TYPE_RADIO:
+		v4l2 = container_of(vdev, struct em28xx_v4l2, radio_dev);
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return;
+	}
+
+	dev = v4l2->dev;
+	kref_put(&v4l2->ref, em28xx_free_v4l2);
+	kref_put(&dev->ref, em28xx_free_device);
+}
+
 /*
  * em28xx_v4l2_open()
  * inits the device and starts isoc transfer
@@ -2554,7 +2579,7 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 static const struct video_device em28xx_video_template = {
 	.fops		= &em28xx_v4l_fops,
 	.ioctl_ops	= &video_ioctl_ops,
-	.release	= video_device_release_empty,
+	.release	= em28xx_vdev_release,
 	.tvnorms	= V4L2_STD_ALL,
 };
 
@@ -2583,7 +2608,7 @@ static const struct v4l2_ioctl_ops radio_ioctl_ops = {
 static struct video_device em28xx_radio_template = {
 	.fops		= &radio_fops,
 	.ioctl_ops	= &radio_ioctl_ops,
-	.release	= video_device_release_empty,
+	.release	= em28xx_vdev_release,
 };
 
 /* I2C possible address to saa7115, tvp5150, msp3400, tvaudio */
@@ -2965,6 +2990,8 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 			"unable to register video device (error=%i).\n", ret);
 		goto unregister_dev;
 	}
+	kref_get(&v4l2->ref);
+	kref_get(&dev->ref);
 
 	/* Allocate and fill vbi video_device struct */
 	if (em28xx_vbi_supported(dev) == 1) {
@@ -2999,6 +3026,8 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 				"unable to register vbi device\n");
 			goto unregister_dev;
 		}
+		kref_get(&v4l2->ref);
+		kref_get(&dev->ref);
 	}
 
 	if (em28xx_boards[dev->model].radio.type == EM28XX_RADIO) {
@@ -3012,6 +3041,8 @@ static int em28xx_v4l2_init(struct em28xx *dev)
 				"can't register radio device\n");
 			goto unregister_dev;
 		}
+		kref_get(&v4l2->ref);
+		kref_get(&dev->ref);
 		dev_info(&dev->intf->dev,
 			 "Registered radio device as %s\n",
 			 video_device_node_name(&v4l2->radio_dev));
-- 
2.54.0