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

[Alan Stern]

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@xxxxxxxxxxxxxxxxxxxxxxxxx
> Link: https://syzkaller.appspot.com/bug?extid=4a5c87a01894ca37f25c
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Rafael Alejandro Diaz Cruz <rafad900@xxxxxxxxx>
> ---
>  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