Re: [PATCH v2] media: em28xx: fix v4l2 use-after-free after failed init

[DRIF ABDELMALEK MOHAMED SAID]

Hello,
Just following up on this patch to see if anyone has had a chance to
review it or if any changes are needed.
Thanks,
Drif Abdelmalek Mohamed Said

On Sat, Oct 31, 2026 at 10:09 PM Drif Abdelmalek Mohamed Said
<drifabdelmalekmohamedsaid@xxxxxxxxx> wrote:
>
> 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: syzbot+1e71765881a6fc43185f@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=1e71765881a6fc43185f
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Drif Abdelmalek Mohamed Said <drifabdelmalekmohamedsaid@xxxxxxxxx>
> ---
> v2: added unlock in early return in em28xx_v4l2_fini
> v1:https://lore.kernel.org/all/20261031204723.1415-1-drifabdelmalekmohamedsaid@xxxxxxxxx/T/
>  drivers/media/usb/em28xx/em28xx-video.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/usb/em28xx/em28xx-video.c b/drivers/media/usb/em28xx/em28xx-video.c
> index 84424284d486..924a3f7977c9 100644
> --- a/drivers/media/usb/em28xx/em28xx-video.c
> +++ b/drivers/media/usb/em28xx/em28xx-video.c
> @@ -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
>