[PATCH 0/3] media: em28xx: fix multiple disconnect races and UAFs

[PATCH 0/3] media: em28xx: fix multiple disconnect races and UAFs

[Nguyen Duy Nhat Anh]

This patch series addresses three distinct lifecycle and race condition bugs in
the em28xx driver triggered during concurrent USB disconnect and userspace
file operations (open/close).

Fixing the initial refcount underflow (addressed in Patch 3/3) allowed
syzbot re-testing to progress further down the teardown pipeline, which
subsequently unveiled two deeper Use-After-Free (UAF) bugs in sequence
(Patches 1/3 and 2/3). 

Summary of changes in the patches:

1. media: em28xx: check dev->disconnected in em28xx_v4l2_open()
   Rejects open() attempts with -ENODEV when the device is disconnected,
   preventing hardware register I/O on freed dev->intf memory.

2. media: em28xx: clear v4l2_dev.mdev on media device unregister
   Zeros out v4l2_dev.mdev in em28xx_unregister_media_device() before freeing
   dev->media_dev, avoiding a dangling pointer read in v4l2_release() during
   file close.

3. media: em28xx: fix refcount underflow in em28xx_free_v4l2()
   Removes v4l2_device_unregister() from the v4l2_dev release callback
   em28xx_free_v4l2() and invokes it in em28xx_v4l2_fini() instead, eliminating
   an illegal double-decrement on a zero reference count.

Nguyen Duy Nhat Anh (3):
  media: em28xx: check dev->disconnected in em28xx_v4l2_open()
  media: em28xx: clear v4l2_dev.mdev on media device unregister
  media: em28xx: fix refcount underflow in em28xx_free_v4l2()

 drivers/media/usb/em28xx/em28xx-cards.c | 2 ++
 drivers/media/usb/em28xx/em28xx-video.c | 5 ++---
 2 files changed, 4 insertions(+), 3 deletions(-)

-- 
2.55.0

[PATCH 1/3] media: em28xx: check dev->disconnected in em28xx_v4l2_open()

[Nguyen Duy Nhat Anh]

em28xx_v4l2_open() attempts hardware register initialization via dev->intf.
If a USB disconnect occurs concurrently, dev->intf is freed while open() is
executing, causing a KASAN slab-use-after-free read in em28xx_write_regs_req().

Fix this by checking dev->disconnected under dev->lock in em28xx_v4l2_open()
and returning -ENODEV before attempting hardware I/O.

Reported-by: syzbot+809dfc586f33fc44b6ac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=809dfc586f33fc44b6ac
Signed-off-by: Nguyen Duy Nhat Anh <neganhat@gmail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 84424284d486..d653341ffd16 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2367,7 +2367,7 @@ static int em28xx_v4l2_open(struct file *filp)
 		return -ERESTARTSYS;
 
 	v4l2 = dev->v4l2;
-	if (!v4l2) {
+	if (!v4l2 || dev->disconnected) {
 		mutex_unlock(&dev->lock);
 		return -ENODEV;
 	}
-- 
2.55.0

[PATCH 2/3] media: em28xx: clear v4l2_dev.mdev on media device unregister

[Nguyen Duy Nhat Anh]

em28xx_unregister_media_device() frees dev->media_dev, but leaves
v4l2_dev.mdev holding a dangling pointer to the freed allocation.

When a process closes a video node after disconnect, v4l2_release() calls
v4l2_device_supports_requests(), dereferencing v4l2_dev.mdev and causing a
KASAN slab-use-after-free read.

Fix this by setting v4l2_dev.mdev to NULL in em28xx_unregister_media_device()
prior to calling kfree(dev->media_dev).

Reported-by: syzbot+809dfc586f33fc44b6ac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=809dfc586f33fc44b6ac
Signed-off-by: Nguyen Duy Nhat Anh <neganhat@gmail.com>
---
 drivers/media/usb/em28xx/em28xx-cards.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/usb/em28xx/em28xx-cards.c b/drivers/media/usb/em28xx/em28xx-cards.c
index 8440eafad44e..40fec8b025e8 100644
--- a/drivers/media/usb/em28xx/em28xx-cards.c
+++ b/drivers/media/usb/em28xx/em28xx-cards.c
@@ -3776,6 +3776,8 @@ static void em28xx_unregister_media_device(struct em28xx *dev)
 	if (dev->media_dev) {
 		media_device_unregister(dev->media_dev);
 		media_device_cleanup(dev->media_dev);
+		if (dev->v4l2)
+			dev->v4l2->v4l2_dev.mdev = NULL;
 		kfree(dev->media_dev);
 		dev->media_dev = NULL;
 	}
-- 
2.55.0

[PATCH 3/3] media: em28xx: fix refcount underflow in em28xx_free_v4l2()

[Nguyen Duy Nhat Anh]

em28xx_free_v4l2() is registered as the v4l2_dev release callback, invoked
when v4l2_dev's reference count drops to zero.

Calling v4l2_device_unregister() from inside this callback causes an extra
call to v4l2_device_put() on a zero reference count, triggering a refcount_t
underflow (0 -> -1) and a kernel BUG in lib/refcount.c.

Fix this by removing v4l2_device_unregister() from em28xx_free_v4l2() and
calling it in em28xx_v4l2_fini() during teardown instead.

Reported-by: syzbot+809dfc586f33fc44b6ac@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=809dfc586f33fc44b6ac
Signed-off-by: Nguyen Duy Nhat Anh <neganhat@gmail.com>
---
 drivers/media/usb/em28xx/em28xx-video.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index d653341ffd16..45b62655ca3b 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2332,7 +2332,6 @@ static void em28xx_free_v4l2(struct v4l2_device *v4l2_dev)
 	struct em28xx *dev = v4l2->dev;
 
 	v4l2_ctrl_handler_free(&v4l2->ctrl_handler);
-	v4l2_device_unregister(v4l2_dev);
 	dev->v4l2 = NULL;
 	kfree(v4l2);
 	kref_put(&dev->ref, em28xx_free_device);
@@ -2462,7 +2461,7 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
 	 * The em28xx_free_v4l2() release callback will be invoked once all
 	 * userspace file handles to the video device nodes are closed.
 	 */
-	v4l2_device_put(&v4l2->v4l2_dev);
+	v4l2_device_unregister(&v4l2->v4l2_dev);
 
 	return 0;
 }
-- 
2.55.0