Re: [PATCH usb-next v2] USB: gadgetfs: Fix use-after-free in gadgetfs_kill_sb

[Rafael Alejandro Díaz Cruz]

After spending a couple hours staring at the code and
asking claude for things I might have missed, I did come
up with a better explanation of how the error happens,
but not why it happens.

I began by removing the get_dev(dev) line I previously
added and keeping a mental score of all the times
that refcount is incremented and decremented and found
that in most cases, they are even. Except in one case
inside gadgetfs_bind():

static int gadgetfs_bind(struct usb_gadget *gadget, struct
usb_gadget_driver *driver) {
     struct dev_data *dev = the_device;
// ........
     + get_dev(dev)  // HERE
     if (!dev->req);
         goto enomem;

     if (activate_ep_files (dev) < 0)
        goto enomem;
// ......
     - get_dev (dev);  // Move this up
     return 0;

enomem:
     gadgetfs_unbind (gadget);
     return -ENOMEM;
}
Somehow the reproducer is triggering one of those two
if conditions and sending the execution down the error
path directly to the gadgetfs_unbind(gadget) and skipping
the get_dev(dev) right before it.

Inside gadgetfs_unbind() we call put_dev(dev) which
would be an uneven decrement. I can confirm this was
the problem by first making the change above and
then triggering the reproducer on my local QEMU which
does not trigger the UAF.

Yet, I don't understand why this is being triggered since
from what I can see, none of the syscalls coming from
the reproducer are directly related to this. And these
.._bind(), ..._unbind() functions are not part of the UAF
stack trace but rather, something that happens in between
the reproducer triggered by the usb protocol.

Not sure if this is good enough. I don't know what else to
look into.

On Mon, Aug 31, 2026 at 7:10 PM Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Aug 31, 2026 at 09:58:57AM -0700, Rafael Alejandro Díaz Cruz wrote:
> > Hello,
> > Sorry for the confusion. I assume I can just address the
> > concerns here and create another patch once the message
> > is clear.
> >
> > I suppose there are actually two errors but because they
> > are triggered by the same syzkaller reproducer, I'm
> > treating them as one. The initial problem is the UAF on
> > the_device pointer when put_dev() is called both by the
> > error path of gadgetfs_fill_super() and gadgetfs_kill_sb().
> > This is triggered by the fault_injection in the reproducer.
> > I'll assume that one is clear.
>
> Yes, that's okay.
>
> > The second underlying issue is still related to the_device
> > but not itself, rather the_device->count variable keeping
> > the references. The syzkaller reproducer still triggers the
> > same fault_injection in the reproducer but the
> > fault_injection might fail which leads to the success path
> > being taken. This means that the gadgetfs_fill_super()
> > will succeed and the the_device->count will be initialized
> > with 1 as you had mentioned. However, at the end of
> > the reproducer close() and umount() will each be called.
> > This leads to the following:
> >
> > close() -> dev_releave() -> put_dev()
> > umount -> gadgetfs_kill_sb() -> put_dev()
> >
> > the_device->count == 1 at the start but is decremented
> > twice. refcount < 0 triggers second UAF.
>
> But gadget_dev_open() increments the refcount (it calls get_dev()).  The
> increment caused by opening the device cancels out the decrement caused
> by closing it.  And the initial assignment to 1 caused by creating it
> cancels out the final decrement caused by gadgetfs_kill_sb().  So there
> shouldn't be any erroneous accesses.
>
> > This is fixed by incrementing the reference on the success
> > path of gadgetfs_fill_super(). That is what the get_dev(dev);
> > call is for.
>
> If you do that, what will cancel out the increment caused by
> gadget_dev_open()?
>
> > As a side note, I'm part of the Linux Kernel Mentorship
> > and I appreciate your feedback! Please don't hold back on
> > the suggestions. Every bit helps me!
>
> Don't mention it; we all have to start somewhere!
>
> Alan Stern