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

Rafael Alejandro Diaz Cruz posted 1 patch 4 weeks ago
drivers/usb/gadget/legacy/inode.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH usb-next v2] USB: gadgetfs: Fix use-after-free in gadgetfs_kill_sb
Posted by Rafael Alejandro Diaz Cruz 4 weeks ago
When gadgetfs_fill_super() fails, it's error path calls
put_dev() which drops refcount inside the_device to 0
and frees the objet. But the_device pointer is not
cleared, leading to point at freed memory.

VFS will then call gadgetfs_kill_sb() after mount
failure leading to put_dev() to be called on the
already freed pointer.

Fix by setting the_device = NULL during error path
before calling put_dev() inside gadgetfs_fill_super()
so that gadgetfs_kill_sb() skips put_dev().

However, if the fault injection from syzbot failed 
and it began the open()/write()/close()/unmount()
sequence then close() and umount() will each trigger
the refcount to drop once via put_dev(). This will 
still cause UAF since refcount is only incremented 
once on creation but decremented twice. 

Reported-by: syzbot+4a5c87a01894ca37f25c@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=4a5c87a01894ca37f25c
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Rafael Alejandro Diaz Cruz <rafad900@gmail.com>
---
 drivers/usb/gadget/legacy/inode.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index d87a8ab51510..77efa984ce84 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -2059,6 +2059,7 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
 	rc = gadgetfs_create_file(sb, CHIP, dev, &ep0_operations);
 	if (rc) {
 		put_dev(dev);
+		the_device = NULL;
 		goto Enomem;
 	}
 
@@ -2066,6 +2067,7 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
 	 * from binding to a controller.
 	 */
 	the_device = dev;
+	get_dev(dev);
 	rc = 0;
 	goto Done;
 
-- 
2.43.0
Re: [PATCH usb-next v2] USB: gadgetfs: Fix use-after-free in gadgetfs_kill_sb
Posted by Alan Stern 4 weeks ago
On Fri, Aug 28, 2026 at 06:19:01PM -0700, Rafael Alejandro Diaz Cruz wrote:
> When gadgetfs_fill_super() fails, it's error path calls
> put_dev() which drops refcount inside the_device to 0
> and frees the objet. But the_device pointer is not
> cleared, leading to point at freed memory.
> 
> VFS will then call gadgetfs_kill_sb() after mount
> failure leading to put_dev() to be called on the
> already freed pointer.
> 
> Fix by setting the_device = NULL during error path
> before calling put_dev() inside gadgetfs_fill_super()
> so that gadgetfs_kill_sb() skips put_dev().
> 
> However, if the fault injection from syzbot failed 
> and it began the open()/write()/close()/unmount()
> sequence then close() and umount() will each trigger
> the refcount to drop once via put_dev(). This will 
> still cause UAF since refcount is only incremented 
> once on creation but decremented twice. 

I don't understand this last paragraph at all.  What fault injection 
from syzbot are you talking about?  The earlier part of the description 
doesn't mention syzbot at all.

Why do you spell "unmount" the first time with an 'n' but "umount" the 
second time without an 'n'?

Is there any reason why close and unmount shouldn't both do a 
put_dev()?  Doesn't the open increment the refcount to 2, so close 
and unmount will set it to 0, causing a deallocation but not a UAF?

Why is the refcount incremented upon creation?  Normally refcounts are 
created with an initial value of 1 so they don't need to be incremented.

> Reported-by: syzbot+4a5c87a01894ca37f25c@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=4a5c87a01894ca37f25c
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Rafael Alejandro Diaz Cruz <rafad900@gmail.com>
> ---
>  drivers/usb/gadget/legacy/inode.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
> index d87a8ab51510..77efa984ce84 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
> @@ -2059,6 +2059,7 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
>  	rc = gadgetfs_create_file(sb, CHIP, dev, &ep0_operations);
>  	if (rc) {
>  		put_dev(dev);
> +		the_device = NULL;
>  		goto Enomem;
>  	}
>  
> @@ -2066,6 +2067,7 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
>  	 * from binding to a controller.
>  	 */
>  	the_device = dev;
> +	get_dev(dev);

Does this have something to do with that mysterious last paragraph in 
the description?  I can't see any relation between the two.  In 
particular, that paragraph doesn't say anything about adding a 
get_dev().

Alan Stern

>  	rc = 0;
>  	goto Done;
>  
> -- 
> 2.43.0
Re: [PATCH usb-next v2] USB: gadgetfs: Fix use-after-free in gadgetfs_kill_sb
Posted by Rafael Alejandro Díaz Cruz 3 weeks, 5 days ago
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.

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.

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.

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!

Thanks,
Rafael.

On Sat, Aug 29, 2026 at 8:05 AM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> On Fri, Aug 28, 2026 at 06:19:01PM -0700, Rafael Alejandro Diaz Cruz wrote:
> > When gadgetfs_fill_super() fails, it's error path calls
> > put_dev() which drops refcount inside the_device to 0
> > and frees the objet. But the_device pointer is not
> > cleared, leading to point at freed memory.
> >
> > VFS will then call gadgetfs_kill_sb() after mount
> > failure leading to put_dev() to be called on the
> > already freed pointer.
> >
> > Fix by setting the_device = NULL during error path
> > before calling put_dev() inside gadgetfs_fill_super()
> > so that gadgetfs_kill_sb() skips put_dev().
> >
> > However, if the fault injection from syzbot failed
> > and it began the open()/write()/close()/unmount()
> > sequence then close() and umount() will each trigger
> > the refcount to drop once via put_dev(). This will
> > still cause UAF since refcount is only incremented
> > once on creation but decremented twice.
>
> I don't understand this last paragraph at all.  What fault injection
> from syzbot are you talking about?  The earlier part of the description
> doesn't mention syzbot at all.
>
> Why do you spell "unmount" the first time with an 'n' but "umount" the
> second time without an 'n'?
>
> Is there any reason why close and unmount shouldn't both do a
> put_dev()?  Doesn't the open increment the refcount to 2, so close
> and unmount will set it to 0, causing a deallocation but not a UAF?
>
> Why is the refcount incremented upon creation?  Normally refcounts are
> created with an initial value of 1 so they don't need to be incremented.
>
> > Reported-by: syzbot+4a5c87a01894ca37f25c@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=4a5c87a01894ca37f25c
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Rafael Alejandro Diaz Cruz <rafad900@gmail.com>
> > ---
> >  drivers/usb/gadget/legacy/inode.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
> > index d87a8ab51510..77efa984ce84 100644
> > --- a/drivers/usb/gadget/legacy/inode.c
> > +++ b/drivers/usb/gadget/legacy/inode.c
> > @@ -2059,6 +2059,7 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
> >       rc = gadgetfs_create_file(sb, CHIP, dev, &ep0_operations);
> >       if (rc) {
> >               put_dev(dev);
> > +             the_device = NULL;
> >               goto Enomem;
> >       }
> >
> > @@ -2066,6 +2067,7 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
> >        * from binding to a controller.
> >        */
> >       the_device = dev;
> > +     get_dev(dev);
>
> Does this have something to do with that mysterious last paragraph in
> the description?  I can't see any relation between the two.  In
> particular, that paragraph doesn't say anything about adding a
> get_dev().
>
> Alan Stern
>
> >       rc = 0;
> >       goto Done;
> >
> > --
> > 2.43.0
Re: [PATCH usb-next v2] USB: gadgetfs: Fix use-after-free in gadgetfs_kill_sb
Posted by Alan Stern 3 weeks, 4 days ago
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
Re: [PATCH usb-next v2] USB: gadgetfs: Fix use-after-free in gadgetfs_kill_sb
Posted by Rafael Alejandro Díaz Cruz 3 weeks, 4 days ago
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@rowland.harvard.edu> 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
Re: [PATCH usb-next v2] USB: gadgetfs: Fix use-after-free in gadgetfs_kill_sb
Posted by Alan Stern 3 weeks, 4 days ago
On Mon, Aug 31, 2026 at 10:06:23PM -0700, Rafael Alejandro Díaz Cruz wrote:
> 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.

That definitely looks like a bug.  The get_dev() call should be moved up 
before the set_gadget_data() call.

> 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.

You can make debugging easier by adding dev_info() or pr_info() calls at 
various strategic places in the code.  Then the kernel log should tell 
you exactly what is happening.

However, no matter how you decide to attack the problem, you shouldn't 
submit a patch until you truly understand what is going wrong and how 
the patch will fix it.  This means thinking for yourself, not relying on 
an AI to do all the thinking for you.

Alan Stern
Re: [PATCH usb-next v2] USB: gadgetfs: Fix use-after-free in gadgetfs_kill_sb
Posted by Rafael Alejandro Díaz Cruz 3 weeks, 3 days ago
I kept going at it and finally realized what was going on.

I added some logging statements as you suggested and
found that my 'how this bug is caused' is correct. It does
come from calling get_dev() after the error
paths, and the simple solution was to move get_dev()
right after the set_gadget_data().

As for 'why' the bug is happening, it basically comes from
the reproducer using failslab to force kmalloc() to fail inside
of usb_ep_alloc_request(). This will cause the following
error path to be taken:

dev->req = usb_ep_alloc_request (gadget->ep0, GFP_KERNEL);
if (!dev->req)         // This is true on failslab
     goto enomem;

This will then skip get_dev() and go directly to
gadgetfs_unbind() -> put_dev()
creating the unbalanced refcount.

It was tricky to understand given that it was failing only
on the N-th failslab attempt. Luckily gdb could do all the
things I needed.

I'm also happy to share that even Claude was not able to
figure this out and could only confirm this was the cause
after I pointed it out.

I'm certain of what I found and will proceed to create the
next patch with an updated message.

Please let me know if there is anything else I should add.
I'll wait for your response before sending it out.
Re: [PATCH usb-next v2] USB: gadgetfs: Fix use-after-free in gadgetfs_kill_sb
Posted by Alan Stern 3 weeks, 3 days ago
On Tue, Sep 01, 2026 at 04:12:17PM -0700, Rafael Alejandro Díaz Cruz wrote:
> I kept going at it and finally realized what was going on.
> 
> I added some logging statements as you suggested and
> found that my 'how this bug is caused' is correct. It does
> come from calling get_dev() after the error
> paths, and the simple solution was to move get_dev()
> right after the set_gadget_data().
> 
> As for 'why' the bug is happening, it basically comes from
> the reproducer using failslab to force kmalloc() to fail inside
> of usb_ep_alloc_request(). This will cause the following
> error path to be taken:
> 
> dev->req = usb_ep_alloc_request (gadget->ep0, GFP_KERNEL);
> if (!dev->req)         // This is true on failslab
>      goto enomem;
> 
> This will then skip get_dev() and go directly to
> gadgetfs_unbind() -> put_dev()
> creating the unbalanced refcount.

Well, yes, it was pretty clear that either the usb_ep_alloc_request() 
call or the activate_ep_files() call had to be failing, given that the 
unbalanced refcount came from gadgetfs_bind()'s error path.  The only 
questions were which one, and how.  With the reproducer introducing 
random memory allocation failures, either one of those calls could go 
wrong.

> It was tricky to understand given that it was failing only
> on the N-th failslab attempt. Luckily gdb could do all the
> things I needed.
> 
> I'm also happy to share that even Claude was not able to
> figure this out and could only confirm this was the cause
> after I pointed it out.
> 
> I'm certain of what I found and will proceed to create the
> next patch with an updated message.
> 
> Please let me know if there is anything else I should add.
> I'll wait for your response before sending it out.

No, this seems good.  Make sure that your patch description is both 
precise and concise, and that it doesn't presume the reader is already 
familiar with the details of your testing.

In fact, since you're fixing two distinct (albeit similar) bugs, you 
might want to split this up into two separate patches.

Alan Stern