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

Jeongjun Park posted 1 patch 5 months ago
There is a newer version of this series
drivers/media/usb/as102/as102_usb_drv.c | 1 +
1 file changed, 1 insertion(+)
[PATCH v2 RESEND] media: as102: fix to not free memory after the device is registered in as102_usb_probe()
Posted by Jeongjun Park 5 months ago
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;
 failed:
 	usb_put_dev(as102_dev->bus_adap.usb_dev);
 	usb_set_intfdata(intf, NULL);
--
Re: [PATCH v2 RESEND] media: as102: fix to not free memory after the device is registered in as102_usb_probe()
Posted by Hans Verkuil 3 months, 3 weeks ago
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);
> --
>