linux-kernel - Re: [PATCH v2 RESEND] media: hackrf: fix to not free memory after the device is registered in hackrf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAO9qdTEgnft880Hm2V2jF8Li0m3d96iu2Zwi_jHN9ErHH7TOTQ@mail.gmail.com>
Date: Sat, 10 Jan 2026 12:46:07 +0900
From: Jeongjun Park <aha310510@...il.com>
To: Hans Verkuil <hverkuil+cisco@...nel.org>
Cc: mchehab@...nel.org, hverkuil@...nel.org, 
	laurent.pinchart+renesas@...asonboard.com, crope@....fi, 
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org, 
	stable@...r.kernel.org, syzbot+6ffd76b5405c006a46b7@...kaller.appspotmail.com, 
	syzbot+f1b20958f93d2d250727@...kaller.appspotmail.com
Subject: Re: [PATCH v2 RESEND] media: hackrf: fix to not free memory after the
 device is registered in hackrf_probe()

Hi Hans,

Hans Verkuil <hverkuil+cisco@...nel.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@...r.kernel.org>
> > Reported-by: syzbot+6ffd76b5405c006a46b7@...kaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=6ffd76b5405c006a46b7
> > Reported-by: syzbot+f1b20958f93d2d250727@...kaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=f1b20958f93d2d250727
> > Fixes: 8bc4a9ed8504 ("[media] hackrf: add support for transmitter")
> > 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/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