media: em28xx: fix v4l2 use-after-free after failed init

em28xx_v4l2_init() publishes the new object in dev->v4l2 before the video device is registered. If a later initialization step fails -past v4l2_device_register-, the error path unregisters the device nodes and drops the initial v4l2_device reference, but previously left dev->v4l2 pointing at the freed object.

syzbot reported:

BUG: KASAN: slab-use-after-free in v4l2_device_disconnect+0xa4/0xc0
                drivers/media/v4l2-core/v4l2-device.c:78

em28xx_v4l2_fini() is called later from USB disconnect. It reads dev->v4l2 before taking dev->lock, and on the failed-init path it owns no reference. The last close of the video node can therefore drop the final v4l2_device reference and free the object between that read and its use in v4l2_device_disconnect(), causing a use-after-free.

Fixing this by clearing dev->v4l2 under dev->lock in the init error path before dropping the initial reference, and read it under dev->lock in fini. This makes fini see NULL after a failed init, while the successful path continues to hold the initial reference until fini is done.

Fixes 288254383674 ("media: em28xx: use v4l2_device release callback")

Reported-by [email protected]

Closes https://syzkaller.appspot.com/bug?extid=1e71765881a6fc43185f

Cc [email protected]

Signed-off-by Drif Abdelmalek Mohamed Said <[email protected]>

---
v2: added unlock in early return in em28xx_v4l2_fini v1:https://lore.kernel.org/all/[email protected]/T/

 drivers/media/usb/em28xx/em28xx-video.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

drivers/media/usb/em28xx/em28xx-video.c +11 -5

index 84424284d486..924a3f7977c9 100644
@@ -2414,7 +2414,7 @@ static int em28xx_v4l2_open(struct file *filp)
  */
 static int em28xx_v4l2_fini(struct em28xx *dev)
 {
-       struct em28xx_v4l2 *v4l2 = dev->v4l2;
+       struct em28xx_v4l2 *v4l2;
 
        if (dev->is_audio_only) {
                /* Shouldn't initialize IR for this interface */

@@ -2426,13 +2426,17 @@ static int em28xx_v4l2_fini(struct em28xx *dev)
                return 0;
        }
 
-       if (!v4l2)
-               return 0;
-
        dev_info(&dev->intf->dev, "Closing video extension\n");
 
        mutex_lock(&dev->lock);
 
+       /* matches em28xx_v4l2_init error paths */
+       v4l2 = dev->v4l2;
+       if (!v4l2){
+               mutex_unlock(&dev->lock);
+               return 0;
+       }
+
        v4l2_device_disconnect(&v4l2->v4l2_dev);
 
        em28xx_uninit_usb_xfer(dev, EM28XX_ANALOG_MODE);

@@ -3129,6 +3133,9 @@ static int em28xx_v4l2_init(struct em28xx *dev)
                vb2_video_unregister_device(&v4l2->vdev);
        }
 
+       /* matches check in em28xx_v4l2_fini (under the same lock) */
+       dev->v4l2 = NULL;
+
        mutex_unlock(&dev->lock);
 
        /*
--

2.43.0