[PATCH 0/2] media: fix em28xx v4l2 init failure refcount underflow and a media core unregister race

[Deepanshu Kartikey]

syzbot reported a slab-use-after-free in dev_dbg_obj() on the close path
of a video device node. It is triggered when em28xx_v4l2_init() fails
after the video devices are registered while userspace still holds one
of the nodes open:

  https://syzkaller.appspot.com/bug?extid=54648a6c1187a7ef46a6

The cause is a double drop of the initial v4l2_device reference. When
init fails (e.g. in v4l2_mc_create_media_graph()), the unregister_dev
error path drops the initial reference but leaves dev->v4l2 set. On USB
disconnect, em28xx_v4l2_fini() sees the non-NULL pointer and drops the
reference a second time. The refcount underflows, em28xx_free_v4l2()
frees struct em28xx_v4l2 while the file handle is still open, and the
later close() makes v4l2_device_release() call
media_device_unregister_entity() on a media entity embedded in the freed
memory.

Patch 1 clears dev->v4l2 in the init error path, so that
em28xx_v4l2_fini() returns early and does not put the reference again.

With patch 1 applied, the same reproducer hits a second, separate
crash: a general protection fault in ida_free(), called from
__media_device_unregister_entity() via
v4l2_open() -> video_put() -> v4l2_device_release() ->
media_device_unregister_entity(). This is a race in
media_device_unregister_entity(). It checks entity->graph_obj.mdev for
NULL before taking mdev->graph_mutex, but not again afterwards. When
media_device_unregister() runs concurrently on USB disconnect (via
em28xx_release_resources()), it unregisters every entity and clears
graph_obj.mdev while the other thread waits for the mutex. The waiter
then calls __media_device_unregister_entity() with a NULL mdev and
faults on &mdev->entity_internal_idx.

Patch 2 re-checks graph_obj.mdev under graph_mutex and skips the
unregister if the entity is already gone. This GPF has no syzbot report
of its own, so patch 2 carries no Reported-by/Closes tags.

Testing:
Both patches were applied together on top of current mainline master and
tested with syzbot using the reproducer for the bug above. syzbot
reported no crash.

Known limitation:
Patch 2 does not close a remaining window in em28xx. After
media_device_unregister() drops graph_mutex,
em28xx_unregister_media_device() calls media_device_cleanup() and
kfree()s dev->media_dev without waiting for the last video device to be
released. A thread already waiting on graph_mutex could in principle
still lock a freed mutex. A complete fix would tie the media_device
lifetime to the last v4l2_device reference. I left that out because it
is a larger change to em28xx. I am happy to look at it as a follow-up if
the maintainers agree with the approach.

Deepanshu Kartikey (2):
  media: em28xx: clear dev->v4l2 on v4l2 init failure
  media: mc: fix race in media_device_unregister_entity()

 drivers/media/mc/mc-device.c            | 3 ++-
 drivers/media/usb/em28xx/em28xx-video.c | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
2.43.0