1. Patchew
  2. linux
  3. View series

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

Jeongjun Park posted 1 patch 5 months ago
There is a newer version of this series
drivers/media/usb/hackrf/hackrf.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH v2 RESEND] media: hackrf: fix to not free memory after the device is registered in hackrf_probe()
Posted by Jeongjun Park 5 months ago 
In hackrf driver, the following race condition occurs:
```
		CPU0						CPU1
hackrf_probe()
  kzalloc(); // alloc hackrf_dev
  ....
  v4l2_device_register();
  ....
						open("/path/to/dev"); // open hackrf dev
						....
  v4l2_device_unregister();
  ....
  kfree(); // free hackrf_dev
  ....
						ioctl(fd, ...);
						  v4l2_ioctl();
						    video_is_registered() // UAF!!
						....
						close(fd);
						  v4l2_release() // UAF!!
						    hackrf_video_release()
						      kfree(); // DFB!!
```

When a V4L2 or video device is unregistered, the device node is removed so
new open() calls are blocked.

However, file descriptors that are already open-and any in-flight I/O-do
not terminate immediately; they remain valid until the last reference is
dropped and the driver's release() is invoked.

Therefore, freeing device memory on the error path after hackrf_probe()
has registered dev it will lead to a race to use-after-free vuln, since
those already-open handles haven't been released yet.

And since release() free memory too, race to use-after-free and 
double-free vuln occur.

To prevent this, if device is registered from probe(), it should be
modified to free memory only through release() rather than calling
kfree() directly.

Cc: <stable@vger.kernel.org>
Reported-by: syzbot+6ffd76b5405c006a46b7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=6ffd76b5405c006a46b7
Reported-by: syzbot+f1b20958f93d2d250727@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f1b20958f93d2d250727
Fixes: 8bc4a9ed8504 ("[media] hackrf: add support for transmitter")
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/20250822142729.1156816-1-aha310510@gmail.com/
---
 drivers/media/usb/hackrf/hackrf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/usb/hackrf/hackrf.c b/drivers/media/usb/hackrf/hackrf.c
index 0b50de8775a3..d7a84422193d 100644
--- a/drivers/media/usb/hackrf/hackrf.c
+++ b/drivers/media/usb/hackrf/hackrf.c
@@ -1515,6 +1515,8 @@ static int hackrf_probe(struct usb_interface *intf,
 	video_unregister_device(&dev->rx_vdev);
 err_v4l2_device_unregister:
 	v4l2_device_unregister(&dev->v4l2_dev);
+	dev_dbg(&intf->dev, "failed=%d\n", ret);
+	return ret;
 err_v4l2_ctrl_handler_free_tx:
 	v4l2_ctrl_handler_free(&dev->tx_ctrl_handler);
 err_v4l2_ctrl_handler_free_rx:
--
Re: [PATCH v2 RESEND] media: hackrf: fix to not free memory after the device is registered in hackrf_probe()
Posted by Hans Verkuil 3 months, 3 weeks ago 
Hi Jeongjun Park,

On 04/09/2025 07:42, Jeongjun Park wrote:
> In hackrf driver, the following race condition occurs:
> ```
> 		CPU0						CPU1
> hackrf_probe()
>   kzalloc(); // alloc hackrf_dev
>   ....
>   v4l2_device_register();
>   ....
> 						open("/path/to/dev"); // open hackrf dev
> 						....
>   v4l2_device_unregister();
>   ....
>   kfree(); // free hackrf_dev
>   ....
> 						ioctl(fd, ...);
> 						  v4l2_ioctl();
> 						    video_is_registered() // UAF!!
> 						....
> 						close(fd);
> 						  v4l2_release() // UAF!!
> 						    hackrf_video_release()
> 						      kfree(); // DFB!!
> ```
> 
> When a V4L2 or video device is unregistered, the device node is removed so
> new open() calls are blocked.
> 
> However, file descriptors that are already open-and any in-flight I/O-do
> not terminate immediately; they remain valid until the last reference is
> dropped and the driver's release() is invoked.
> 
> Therefore, freeing device memory on the error path after hackrf_probe()
> has registered dev it will lead to a race to use-after-free vuln, since
> those already-open handles haven't been released yet.
> 
> And since release() free memory too, race to use-after-free and 
> double-free vuln occur.
> 
> To prevent this, if device is registered from probe(), it should be
> modified to free memory only through release() rather than calling
> kfree() directly.
> 
> Cc: <stable@vger.kernel.org>
> Reported-by: syzbot+6ffd76b5405c006a46b7@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=6ffd76b5405c006a46b7
> Reported-by: syzbot+f1b20958f93d2d250727@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=f1b20958f93d2d250727
> Fixes: 8bc4a9ed8504 ("[media] hackrf: add support for transmitter")
> 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/20250822142729.1156816-1-aha310510@gmail.com/
> ---
>  drivers/media/usb/hackrf/hackrf.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/usb/hackrf/hackrf.c b/drivers/media/usb/hackrf/hackrf.c
> index 0b50de8775a3..d7a84422193d 100644
> --- a/drivers/media/usb/hackrf/hackrf.c
> +++ b/drivers/media/usb/hackrf/hackrf.c
> @@ -1515,6 +1515,8 @@ static int hackrf_probe(struct usb_interface *intf,
>  	video_unregister_device(&dev->rx_vdev);
>  err_v4l2_device_unregister:
>  	v4l2_device_unregister(&dev->v4l2_dev);

Instead of v4l2_device_unregister() this should call v4l2_device_put().
Otherwise the memory will never be freed since the v4l2_device refcount
will never reach 0.

> +	dev_dbg(&intf->dev, "failed=%d\n", ret);

Drop this line, it doesn't add anything.

Regards,

	Hans

> +	return ret;
>  err_v4l2_ctrl_handler_free_tx:
>  	v4l2_ctrl_handler_free(&dev->tx_ctrl_handler);
>  err_v4l2_ctrl_handler_free_rx:
> --
>
Re: [PATCH v2 RESEND] media: hackrf: fix to not free memory after the device is registered in hackrf_probe()
Posted by Jeongjun Park 3 weeks, 6 days ago 
Hi Hans,

Hans Verkuil <hverkuil+cisco@kernel.org> wrote:
>
> Hi Jeongjun Park,
>
> On 04/09/2025 07:42, Jeongjun Park wrote:
> > In hackrf driver, the following race condition occurs:
> > ```
> >               CPU0                                            CPU1
> > hackrf_probe()
> >   kzalloc(); // alloc hackrf_dev
> >   ....
> >   v4l2_device_register();
> >   ....
> >                                               open("/path/to/dev"); // open hackrf dev
> >                                               ....
> >   v4l2_device_unregister();
> >   ....
> >   kfree(); // free hackrf_dev
> >   ....
> >                                               ioctl(fd, ...);
> >                                                 v4l2_ioctl();
> >                                                   video_is_registered() // UAF!!
> >                                               ....
> >                                               close(fd);
> >                                                 v4l2_release() // UAF!!
> >                                                   hackrf_video_release()
> >                                                     kfree(); // DFB!!
> > ```
> >
> > When a V4L2 or video device is unregistered, the device node is removed so
> > new open() calls are blocked.
> >
> > However, file descriptors that are already open-and any in-flight I/O-do
> > not terminate immediately; they remain valid until the last reference is
> > dropped and the driver's release() is invoked.
> >
> > Therefore, freeing device memory on the error path after hackrf_probe()
> > has registered dev it will lead to a race to use-after-free vuln, since
> > those already-open handles haven't been released yet.
> >
> > And since release() free memory too, race to use-after-free and
> > double-free vuln occur.
> >
> > To prevent this, if device is registered from probe(), it should be
> > modified to free memory only through release() rather than calling
> > kfree() directly.
> >
> > Cc: <stable@vger.kernel.org>
> > Reported-by: syzbot+6ffd76b5405c006a46b7@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=6ffd76b5405c006a46b7
> > Reported-by: syzbot+f1b20958f93d2d250727@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=f1b20958f93d2d250727
> > Fixes: 8bc4a9ed8504 ("[media] hackrf: add support for transmitter")
> > 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/20250822142729.1156816-1-aha310510@gmail.com/
> > ---
> >  drivers/media/usb/hackrf/hackrf.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/media/usb/hackrf/hackrf.c b/drivers/media/usb/hackrf/hackrf.c
> > index 0b50de8775a3..d7a84422193d 100644
> > --- a/drivers/media/usb/hackrf/hackrf.c
> > +++ b/drivers/media/usb/hackrf/hackrf.c
> > @@ -1515,6 +1515,8 @@ static int hackrf_probe(struct usb_interface *intf,
> >       video_unregister_device(&dev->rx_vdev);
> >  err_v4l2_device_unregister:
> >       v4l2_device_unregister(&dev->v4l2_dev);
>
> Instead of v4l2_device_unregister() this should call v4l2_device_put().
> Otherwise the memory will never be freed since the v4l2_device refcount
> will never reach 0.

Oh, you're right.

Calling v4l2_device_unregister() instead of v4l2_device_put() causes a
memory leak here.

I'll write a v3 patch for hackrf and as102 and send it to you right away.

>
> > +     dev_dbg(&intf->dev, "failed=%d\n", ret);
>
> Drop this line, it doesn't add anything.
>
> Regards,
>
>         Hans
>
> > +     return ret;
> >  err_v4l2_ctrl_handler_free_tx:
> >       v4l2_ctrl_handler_free(&dev->tx_ctrl_handler);
> >  err_v4l2_ctrl_handler_free_rx:
> > --
> >
>

Regards,
Jeongjun Park

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.