* [PATCH v2] media: em28xx: fix use-after-free in em28xx_unregister_media_device()
@ 2026-04-21 17:41 Mauricio Faria de Oliveira
[not found] ` <69e7cc42.050a0220.31d583.2f7b@mx.google.com>
0 siblings, 1 reply; 2+ messages in thread
From: Mauricio Faria de Oliveira @ 2026-04-21 17:41 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: kernel-dev, linux-media, linux-kernel,
syzbot+07b93bb3189febcab189, Mauricio Faria de Oliveira
During a short probe/disconnect test, the em28xx driver may load the
em28xx-v4l sub-driver asynchronously and em28xx-v4l may fail to init,
releasing memory that may be used in the em28xx's disconnect handler
if a video device was opened during init (eg, udev's v4l_id program)
and remained open during disconnect, triggering an use-after-free.
Context:
In em28xx_v4l2_init() once video_register_device() returns, a struct
video_device (vdev) that is embedded in struct em28xx_v4l2 (v4l2) is
referenced by the list struct media_device.entities, which is looped
over when releasing resources in em28xx_usb_disconnect().
This list entry is only removed when the last reference to the video
device (struct video_device.dev) is put. However, this might occur
*after* the containing struct em28xx_v4l2 (v4l2) is released in the
error path of em28xx_v4l2_init() and *after* the disconnect handler
loops over the list, leading to a use-after-free.
Details:
If the video device is opened right after em28xx_v4l2_init() calls
video_register_device(), which adds the memory reference to the list,
and remains open after em28xx_v4l2_init()'s error path releases the
containing struct em28xx_v4l2 (as it does not find other references),
the memory reference to the contained struct video_device is still in
the list -- but it has been released!
Then, if em28xx_usb_disconnect() is called, it proceeds to loop over the
list, accessing the struct list_head's pointers in now released memory.
Solution:
In order to fix this, get references to struct em28xx_v4l2 (v4l2) when
initializing its struct video_device fields (vdev, vbi_dev, radio_dev).
Put these references in the new release callback for struct video_device
or if video_register_device() fails (when the callback is not invoked).
Even though this particular issue is specific to CONFIG_MEDIA_CONTROLLER
at this time, it is reasonable to get/put references due to the embedded
struct video_device fields in any case, for correctness (as a pointer to
it has been passed) and future-proofing to a non-CONFIG_MEDIA_CONTROLLER
case/issue that might happen in the future.
Problematic code path/race condition:
em28xx_usb_probe
em28xx_init_dev
em28xx_media_device_init(dev, ...)
mdev = kzalloc_obj(*mdev)
dev->media_dev = mdev
request_modules(dev)
INIT_WORK(&dev->request_module_wk, request_module_async)
schedule_work(&dev->request_module_wk)
media_device_register(dev->media_dev)
request_module_async
request_module("em28xx-v4l") // em28xx-video.c
module_init(em28xx_video_register)
em28xx_register_extension()
em28xx_v4l2_init()
v4l2 = kzalloc_obj(*v4l2)
video_register_device(&v4l2->vdev, ...)
__video_register_device(vdev = &v4l2->vdev, ...)
vdev->dev.release = v4l2_device_release
device_register(&vdev->dev)
video_register_media_controller(vdev)
media_device_register_entity(..., &vdev->entity)
media_gobj_create(..., &entity->graph_obj)
list_add_tail(&gobj->list,
&mdev->entities) // ref!
(*) /usr/lib/udev/v4l_id: open("/dev/video0")
// get_device(dev)
v4l2_mc_create_media_graph(dev->media_dev) // fails.
dev_err(..., "failed to create media graph\n")
goto unregister_dev;
unregister_dev:
video_unregister_device(&v4l2->vdev);
device_unregister(dev = &vdev->dev);
put_device(dev) // if open() happens before this
// no v4l2_device_release() yet.
kref_put(&v4l2->ref, em28xx_free_v4l2) // frees v4l2!
em28xx_usb_disconnect
em28xx_release_resources
em28xx_unregister_media_device
if (dev->media_dev)
media_device_unregister(dev->media_dev)
list_for_each_entry_safe(entity, next,
&mdev->entities, // ref!
graph_obj.list)
// access to v4l2.vdev.entity.graph_object.list!
(*) /usr/lib/udev/v4l_id: close("/dev/video0")
// put_device(dev)
// v4l2_device_release()
// media_device_unregister_entity(&vdev->entity)
// media_gobj_destroy(&entity->graph_obj)
// list_del(&gobj->list) // too late.
Note: commit d4352f3639d7 ("[media] em28xx: embed video_device") created
the problem of memory references to struct em28xx_v4l2's embedded fields
in struct media_device.entities, but this use-after-free didn't happen yet
as media_device_unregister() in the em28xx_usb_disconnect() path was added
in commit 37ecc7b1278f ("[media] em28xx: add media controller support").
Fixes: d4352f3639d7 ("[media] em28xx: embed video_device")
Fixes: 37ecc7b1278f ("[media] em28xx: add media controller support")
Reported-by: syzbot+07b93bb3189febcab189@syzkaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=07b93bb3189febcab189
Signed-off-by: Mauricio Faria de Oliveira <mfo@igalia.com>
---
It's possible to synthetically reproduce this bug with wait-for/delays
in the driver's code paths exercised by syzbot's USB probe/disconnect.
If you're interested in it, just let me know and I'll send it as well.
Tested on next-20260420.
Before:
[ 33.348056] usb 1-1: new high-speed USB device number 2 using dummy_hcd
...
[ 33.485616] em28xx 1-1:0.132: New device syz syz @ 480 Mbps (0413:6023, interface 132, class 132)
[ 33.487493] em28xx 1-1:0.132: Video interface 132 found:
...
[ 34.874703] em28xx 1-1:0.132: Registering V4L2 extension
...
[ 34.950464] em28xx 1-1:0.132: failed to create media graph
[ 34.951029] DBG Step 1 em28xx_v4l2_init(): Waiting for dev->v4l2 to be opened...
[ 34.966574] DBG Step 2 em28xx_v4l2_open(): dev->v4l2 opened; waiting 5 seconds.
[ 35.889389] usb 1-1: USB disconnect, device number 2
[ 35.893686] em28xx 1-1:0.132: Disconnecting em28xx
[ 35.981999] DBG Step 1 em28xx_v4l2_init(): Done.
[ 35.983443] em28xx 1-1:0.132: V4L2 device video0 deregistered
(differences below)
[ 35.988481] DBG Step 3 em28xx_free_v4l2(): Freeing dev->v4l2
[ 35.991212] DBG Step 3 em28xx_free_v4l2(): Done
[ 35.993331] DBG Step 4 em28xx_usb_disconnect(): Waiting for dev->v4l2 to be freed...
[ 35.996234] DBG Step 4 em28xx_usb_disconnect(): Done.
[ 35.999200] DBG Step 5 em28xx_unregister_media_device(): use-after-free expected...
[ 36.004739] ==================================================================
[ 36.008074] BUG: KASAN: slab-use-after-free in media_device_unregister+0x542/0x560
[ 36.009908] Read of size 8 at addr ffff88800a1dc160 by task kworker/1:2/90
[ 36.011677]
[ 36.012045] CPU: 1 UID: 0 PID: 90 Comm: kworker/1:2 Not tainted 7.0.0-next-20260420-dirty #37 PREEMPT(lazy)
[ 36.012065] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 36.012075] Workqueue: usb_hub_wq hub_event
[ 36.012097] Call Trace:
[ 36.012106] <TASK>
[ 36.012114] dump_stack_lvl+0x4d/0x70
[ 36.012133] print_report+0x153/0x4c6
[ 36.012192] kasan_report+0xda/0x110
[ 36.012239] media_device_unregister+0x542/0x560
[ 36.012255] em28xx_unregister_media_device.part.0+0x4f/0xf0
[ 36.012273] em28xx_release_resources+0xa1/0x1a0
[ 36.012289] em28xx_usb_disconnect.cold+0x280/0x323
[ 36.012306] usb_unbind_interface+0x154/0x690
[ 36.012324] device_release_driver_internal+0x36b/0x500
[ 36.012354] bus_remove_device+0x259/0x3e0
[ 36.012408] device_del+0x364/0xce0
[ 36.012482] usb_disable_device+0x270/0x4f0
[ 36.012509] usb_disconnect+0x26a/0x910
[ 36.012523] hub_event+0x966/0x3600
[ 36.012589] process_one_work+0x618/0xf20
[ 36.012613] worker_thread+0x422/0xbb0
[ 36.012658] kthread+0x2cb/0x3a0
[ 36.012686] ret_from_fork+0x338/0x520
[ 36.012719] ret_from_fork_asm+0x1a/0x30
[ 36.012729] </TASK>
[ 36.012731]
[ 36.036089] Allocated by task 24:
[ 36.036416] kasan_save_stack+0x30/0x50
[ 36.036886] kasan_save_track+0x14/0x30
[ 36.037578] __kasan_kmalloc+0x7f/0x90
[ 36.037953] em28xx_v4l2_init.cold+0xa7/0x39f0
[ 36.038411] em28xx_init_extension+0xfb/0x1c0
[ 36.038864] process_one_work+0x618/0xf20
[ 36.039522] worker_thread+0x422/0xbb0
[ 36.039784] kthread+0x2cb/0x3a0
[ 36.039982] ret_from_fork+0x338/0x520
[ 36.040298] ret_from_fork_asm+0x1a/0x30
[ 36.040600]
[ 36.040703] Freed by task 24:
[ 36.040916] kasan_save_stack+0x30/0x50
[ 36.041265] kasan_save_track+0x14/0x30
[ 36.041586] kasan_save_free_info+0x3b/0x70
[ 36.041902] __kasan_slab_free+0x47/0x70
[ 36.042313] kfree+0x16d/0x430
[ 36.042612] em28xx_free_v4l2+0x9c/0xc0
[ 36.043024] em28xx_v4l2_init.cold+0x306/0x39f0
[ 36.043425] em28xx_init_extension+0xfb/0x1c0
[ 36.043765] process_one_work+0x618/0xf20
[ 36.044156] worker_thread+0x422/0xbb0
[ 36.044480] kthread+0x2cb/0x3a0
[ 36.044749] ret_from_fork+0x338/0x520
[ 36.045038] ret_from_fork_asm+0x1a/0x30
[ 36.045308]
...
[ 36.059006] ==================================================================
...
[ 36.060423] em28xx 1-1:0.132: Freeing device
[ 40.078033] DBG Step 2 em28xx_v4l2_open(): Done.
After:
[ 22.624582] usb 1-1: new high-speed USB device number 2 using dummy_hcd
...
[ 22.786600] em28xx 1-1:0.132: New device syz syz @ 480 Mbps (0413:6023, interface 132, class 132)
[ 22.792225] em28xx 1-1:0.132: Video interface 132 found:
...
[ 24.184584] em28xx 1-1:0.132: Registering V4L2 extension
...
[ 24.262562] em28xx 1-1:0.132: failed to create media graph
[ 24.263116] DBG Step 1 em28xx_v4l2_init(): Waiting for dev->v4l2 to be opened...
[ 24.277644] DBG Step 2 em28xx_v4l2_open(): dev->v4l2 opened; waiting 5 seconds.
[ 25.186376] usb 1-1: USB disconnect, device number 2
[ 25.190080] em28xx 1-1:0.132: Disconnecting em28xx
[ 25.295219] DBG Step 1 em28xx_v4l2_init(): Done.
[ 25.299018] em28xx 1-1:0.132: V4L2 device video0 deregistered
(differences below)
[ 25.308234] DBG Step 4 em28xx_usb_disconnect(): Waiting for dev->v4l2 to be freed...
[ 29.326594] DBG Step 2 em28xx_v4l2_open(): Done.
[ 29.328570] videodev: DBG v4l2_device_release(): removing dev->v4l2 media entity
[ 29.332302] DBG Step 3 em28xx_free_v4l2(): Freeing dev->v4l2
[ 29.333098] DBG Step 3 em28xx_free_v4l2(): Done
[ 29.454556] DBG Step 4 em28xx_usb_disconnect(): Done.
[ 29.455721] DBG Step 5 em28xx_unregister_media_device(): use-after-free expected...
[ 29.460142] em28xx 1-1:0.132: Freeing device
(No use-after-free.)
---
Changes in v2:
- Fix/improve logic:
- Remove CONFIG_MEDIA_CONTROLLER guards of kref_{get,put}().
- Explain that in commit message.
- Improve readability:
- Use em28xx_vdev_release() to kref_put() if video_register_device()
fails, as it pairs more clearly with em28xx_vdev_init().
- Simplify comments in em28xx_vdev_{init,release}().
- Commit message:
- Rephrase it a bit.
- Add Fixes: tags and trailing note about Fixed commits.
- Link to v1: https://lore.kernel.org/r/20260413-em28xx-v4l-uaf-v1-1-52a5a71493ce@igalia.com
---
drivers/media/usb/em28xx/em28xx-video.c | 32 ++++++++++++++++++++++++++++++--
1 file changed, 30 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
index 4a0ce9c5ee4b4abde681cc373e1c6db2f3b3af70..c95370f298c1f9967e6031ebafb773487b0e8c8b 100644
--- a/drivers/media/usb/em28xx/em28xx-video.c
+++ b/drivers/media/usb/em28xx/em28xx-video.c
@@ -2495,6 +2495,8 @@ static int em28xx_v4l2_close(struct file *filp)
return 0;
}
+static void em28xx_vdev_release(struct video_device *vfd);
+
static const struct v4l2_file_operations em28xx_v4l_fops = {
.owner = THIS_MODULE,
.open = em28xx_v4l2_open,
@@ -2552,7 +2554,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,
};
@@ -2581,7 +2583,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 */
@@ -2619,6 +2621,29 @@ static void em28xx_vdev_init(struct em28xx *dev,
dev_name(&dev->intf->dev), type_name);
video_set_drvdata(vfd, dev);
+
+ /*
+ * Get a reference to struct em28xx_v4l2 as embedded struct video_device
+ * may be accessed from elsewhere after video_register_device() returns.
+ *
+ * If video_register_device() succeeds, this reference will be released
+ * automatically in em28xx_vdev_release(); on error it must be released
+ * explicitly.
+ */
+ kref_get(&dev->v4l2->ref);
+}
+
+static void em28xx_vdev_release(struct video_device *vfd)
+{
+ struct em28xx_v4l2 *v4l2;
+
+ /*
+ * Find struct em28xx_v4l2 with its embedded struct v4l2_device,
+ * as video_get_drvdata() returns dev and dev->v4l2 may be NULL
+ * (e.g., after em28xx_v4l2_init()'s error path).
+ */
+ v4l2 = container_of(vfd->v4l2_dev, struct em28xx_v4l2, v4l2_dev);
+ kref_put(&v4l2->ref, em28xx_free_v4l2);
}
static void em28xx_tuner_setup(struct em28xx *dev, unsigned short tuner_addr)
@@ -2961,6 +2986,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
if (ret) {
dev_err(&dev->intf->dev,
"unable to register video device (error=%i).\n", ret);
+ em28xx_vdev_release(&v4l2->vdev);
goto unregister_dev;
}
@@ -2995,6 +3021,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
if (ret < 0) {
dev_err(&dev->intf->dev,
"unable to register vbi device\n");
+ em28xx_vdev_release(&v4l2->vbi_dev);
goto unregister_dev;
}
}
@@ -3008,6 +3035,7 @@ static int em28xx_v4l2_init(struct em28xx *dev)
if (ret < 0) {
dev_err(&dev->intf->dev,
"can't register radio device\n");
+ em28xx_vdev_release(&v4l2->radio_dev);
goto unregister_dev;
}
dev_info(&dev->intf->dev,
---
base-commit: 97e797263a5e963da3d1e66e743fd518567dfe37
change-id: 20260413-em28xx-v4l-uaf-1d196e53dc50
Best regards,
--
Mauricio Faria de Oliveira <mfo@igalia.com>
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [v2] media: em28xx: fix use-after-free in em28xx_unregister_media_device()
[not found] ` <69e7cc42.050a0220.31d583.2f7b@mx.google.com>
@ 2026-04-22 20:30 ` Mauricio Faria de Oliveira
0 siblings, 0 replies; 2+ messages in thread
From: Mauricio Faria de Oliveira @ 2026-04-22 20:30 UTC (permalink / raw)
To: linux-media
On 2026-04-21 16:13, Patchwork Integration wrote:
> Thanks for your patches! Unfortunately the Media CI robot detected some
> issues:
...
> https://linux-media.pages.freedesktop.org/-/users/patchwork/-/jobs/97938357/artifacts/report.htm .
...
> If you believe that the CI is wrong, kindly open an issue at
> https://gitlab.freedesktop.org/linux-media/media-ci/-/issues or reply-all
> to this message.
I opened
https://gitlab.freedesktop.org/linux-media/media-ci/-/work_items/128
as the same (unrelated) errors happened again, and other patches also
seem to
be affected, taking a look at patchwork.
Thanks,
--
Mauricio
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-04-22 20:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-21 17:41 [PATCH v2] media: em28xx: fix use-after-free in em28xx_unregister_media_device() Mauricio Faria de Oliveira
[not found] ` <69e7cc42.050a0220.31d583.2f7b@mx.google.com>
2026-04-22 20:30 ` [v2] " Mauricio Faria de Oliveira
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox