Re: [PATCH v2 RESEND] media: as102: fix to not free memory after the device is registered in as102_usb_probe()

[Hans Verkuil <hverkuil+cisco@kernel.org>]

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@vger.kernel.org>
> Reported-by: syzbot+47321e8fd5a4c84088db@syzkaller.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@gmail.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);
> --
>