[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1a6e25c6-820e-4610-80bb-518edca18bd5@kernel.org>
Date: Tue, 14 Oct 2025 12:06:52 +0200
From: Hans Verkuil <hverkuil+cisco@...nel.org>
To: Jeongjun Park <aha310510@...il.com>, mchehab@...nel.org,
khoroshilov@...ras.ru
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
stable@...r.kernel.org, syzbot+47321e8fd5a4c84088db@...kaller.appspotmail.com
Subject: Re: [PATCH v2 RESEND] media: as102: fix to not free memory after the
device is registered in as102_usb_probe()
On 04/09/2025 07:46, Jeongjun Park wrote:
> In as102_usb driver, the following race condition occurs:
> ```
> CPU0 CPU1
> as102_usb_probe()
> kzalloc(); // alloc as102_dev_t
> ....
> usb_register_dev();
> open("/path/to/dev"); // open as102 dev
> ....
> usb_deregister_dev();
> ....
> kfree(); // free as102_dev_t
> ....
> close(fd);
> as102_release() // UAF!!
> as102_usb_release()
> kfree(); // DFB!!
> ```
>
> When a USB character device registered with usb_register_dev() is later
> unregistered (via usb_deregister_dev() or disconnect), the device node is
> removed so new open() calls fail. However, file descriptors that are
> already open do not go away immediately: they remain valid until the last
> reference is dropped and the driver's .release() is invoked.
>
> In as102, as102_usb_probe() calls usb_register_dev() and then, on an
> error path, does usb_deregister_dev() and frees as102_dev_t right away.
> If userspace raced a successful open() before the deregistration, that
> open FD will later hit as102_release() --> as102_usb_release() and access
> or free as102_dev_t again, occur a race to use-after-free and
> double-free vuln.
>
> The fix is to never kfree(as102_dev_t) directly once usb_register_dev()
> has succeeded. After deregistration, defer freeing memory to .release().
>
> In other words, let release() perform the last kfree when the final open
> FD is closed.
>
> Cc: <stable@...r.kernel.org>
> Reported-by: syzbot+47321e8fd5a4c84088db@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=47321e8fd5a4c84088db
> Fixes: cd19f7d3e39b ("[media] as102: fix leaks at failure paths in as102_usb_probe()")
> Signed-off-by: Jeongjun Park <aha310510@...il.com>
> ---
> v2: Fix incorrect patch description style and CC stable mailing list
> - Link to v1: https://lore.kernel.org/all/20250822143539.1157329-1-aha310510@gmail.com/
> ---
> drivers/media/usb/as102/as102_usb_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/media/usb/as102/as102_usb_drv.c b/drivers/media/usb/as102/as102_usb_drv.c
> index e0ef66a522e2..abde5666b2ee 100644
> --- a/drivers/media/usb/as102/as102_usb_drv.c
> +++ b/drivers/media/usb/as102/as102_usb_drv.c
> @@ -404,6 +404,7 @@ static int as102_usb_probe(struct usb_interface *intf,
> as102_free_usb_stream_buffer(as102_dev);
> failed_stream:
> usb_deregister_dev(intf, &as102_usb_class_driver);
> + return ret;
Here too I wonder if the memory is actually freed. I suspect a usb_put_intf()
is needed. I'm not a USB expert, though. But you should check if you didn't
replace a UAF by a memory leak.
Regards,
Hans
> failed:
> usb_put_dev(as102_dev->bus_adap.usb_dev);
> usb_set_intfdata(intf, NULL);
> --
>
Powered by blists - more mailing lists