Re: [PATCH 1/2] media: em28xx-video: add the unregister of video/VBI entity

[hverkuil+cisco]

On 20/03/2026 12:45, Edward Adam Davis wrote:
> On Mon, 16 Mar 2026 15:03:23 +0100, Hans Verkuil wrote:
>> On 11/01/2026 06:29, Edward Adam Davis wrote:
>>> When creating a media graph, a failure occurred due to the lack of
>>> a corresponding decoder. During the subsequent media device release
>>> process, the video and VBI devices were not properly unregistered,
>>> leading to a use-after-free vulnerability reported by syzbot [1].
>>>
>>> The fix involves adding the necessary unregister operations.
>>>
>>> [1]
>>> BUG: KASAN: slab-use-after-free in media_device_unregister+0x141/0x430 drivers/media/mc/mc-device.c:804
>>> Read of size 8 at addr ffff88807c114210 by task kworker/1:9/6093
>>> Call Trace:
>>>  media_device_unregister+0x141/0x430 drivers/media/mc/mc-device.c:804
>>>  em28xx_unregister_media_device drivers/media/usb/em28xx/em28xx-cards.c:3511 [inline]
>>>  em28xx_release_resources+0xac/0x240 drivers/media/usb/em28xx/em28xx-cards.c:3532
>>>  em28xx_usb_disconnect+0x19f/0x2f0 drivers/media/usb/em28xx/em28xx-cards.c:4201
>>>  usb_unbind_interface+0x26e/0x910 drivers/usb/core/driver.c:458
>>>
>>> Allocated by task 5932:
>>>  em28xx_v4l2_init+0x10b/0x2e70 drivers/media/usb/em28xx/em28xx-video.c:2532
>>>  em28xx_init_extension+0x120/0x1c0 drivers/media/usb/em28xx/em28xx-core.c:1117
>>>
>>> Freed by task 5932:
>>>  em28xx_free_v4l2 drivers/media/usb/em28xx/em28xx-video.c:2118 [inline]
>>>  kref_put include/linux/kref.h:65 [inline]
>>>  em28xx_v4l2_init+0x1683/0x2e70 drivers/media/usb/em28xx/em28xx-video.c:2901
>>>
>>> Reported-by: syzbot+16062f26c6480975e5ed@xxxxxxxxxxxxxxxxxxxxxxxxx
>>> Closes: https://syzkaller.appspot.com/bug?extid=16062f26c6480975e5ed
>>> Tested-by: syzbot+16062f26c6480975e5ed@xxxxxxxxxxxxxxxxxxxxxxxxx
>>> Signed-off-by: Edward Adam Davis <eadavis@xxxxxx>
>>> ---
>>>  drivers/media/usb/em28xx/em28xx-video.c | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
>>> index 2dfa3242a7ab..45b68ebf2e9c 100644
>>> --- a/drivers/media/usb/em28xx/em28xx-video.c
>>> +++ b/drivers/media/usb/em28xx/em28xx-video.c
>>> @@ -882,9 +882,12 @@ static void em28xx_v4l2_media_release(struct em28xx *dev)
>>>
>>>  	for (i = 0; i < MAX_EM28XX_INPUT; i++) {
>>>  		if (!INPUT(i)->type)
>>> -			return;
>>> +			break;
>>>  		media_device_unregister_entity(&dev->input_ent[i]);
>>>  	}
>>> +	media_device_unregister_entity(&dev->v4l2->vdev.entity);
>>> +	if (em28xx_vbi_supported(dev))
>>> +		media_device_unregister_entity(&dev->v4l2->vbi_dev.entity);
>>
>> This is definitely wrong: these are registered and unregistered in v4l2-dev.c,
>> so it makes no sense to unregister them here.
> You are only half right. Under normal circumstances, these devices
> would indeed be unregistered via media_device_unregister() in v4l2-dev.c.
> However, did you notice that if em28xx_v4l2_init() fails to create
> the media graph, it will release the corresponding v4l2, including
> its members vdev and vbi_dev, before media_device_unregister() is called?
> 
> Furthermore, both vdev and vbi_dev are instances of struct video_device.
> Since they are inserted into the media framework as entities, they are,
> in essence, the corresponding video_device instances themselves (as the
> first member of struct video_device is, in fact, the entity instance).
> 
> Therefore, the UAF vulnerability described in [1] is ultimately triggered
> when media_device_unregister() attempts to remove the entity from the
> mdev instance at a em28xx usb disconnect.
> 
> Why is the execution of media_device_unregister() delayed in this context?
> There is a high probability that the open syscall acquires em28xx->lock
> *before* the unregistration path for media devices can proceed. This
> scenario is identical to the one described in patch [2]; consequently,
> this issue can also be resolved by applying the fix provided in [2].

To properly fix em28xx it has to be redesigned: all clean up would have to
be done in the v4l2_device release callback, similar to how au0282 does that.

There is no point in working around issues, without going for that approach.

The em28xx driver predates that callback, and nobody ever took the time and
substantial effort to convert it.

We regularly get patches for em28xx, but they are basically addressing symptoms
rather than the cause.

If someone wants to take on this task, then that would be great and I'm willing
to help with advice etc., but you also need to have at least some em28xx devices
to test with.

Regards,

	Hans

> 
> [2] https://lore.kernel.org/all/tencent_5DCCB375C3694964A3A5A44677775777E605@xxxxxx
> 
> BR,
> Edward
>