USB: gadget: Fix UAF in gadgetfs_fill_super()

PATCH Superseded Rafael Alejandro Diaz Cruz 3 participants 6 messages Sep 2 – Sep 4 1 message not in the archive USB

lore

Share & tools

lore

Rafael Alejandro Diaz Cruz

author

2026-09-02 05:01 UTC #0 lore raw

PATCH 1/1 USB: gadget: Fix UAF in gadgetfs_fill_super()

UAF is caused by syzkaller reproducer forcing the failure of gadgetfs_fill_super()

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

Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")

Reported-by [email protected]

Link https://syzkaller.appspot.com/bug?extid=4a5c87a01894ca37f25c

Signed-off-by Rafael Alejandro Diaz Cruz <[email protected]>

---
 drivers/usb/gadget/legacy/inode.c | 1 +
 1 file changed, 1 insertion(+)

drivers/usb/gadget/legacy/inode.c +1 -0

index d87a8ab51510..3e2bce7543d4 100644
@@ -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;
 	}

Signature · 1 line

2.43.0

Alan Stern

Reviewed-by on v1 1/1

2026-09-02 14:38 UTC #1 lore raw

Quoted 2 lines · Rafael Alejandro Diaz Cruz

UAF is caused by syzkaller reproducer forcing the failure of gadgetfs_fill_super()

Just a minor comment: The fact that you used syzkaller to cause the UAF in your testing isn't relevant. Memory allocation failures can occur in real life, without fuzzing, and the driver needs to deal with them properly.

Quoted 9 lines

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.

Quoted 8 lines

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

Since this fixes a real bug, you should add:

CC <[email protected]>

Quoted 1 line

---

Reviewed-by Alan Stern <[email protected]>

Alan Stern

Quoted 19 lines

 drivers/usb/gadget/legacy/inode.c | 1 +
 1 file changed, 1 insertion(+)

drivers/usb/gadget/legacy/inode.c +1 -0

index d87a8ab51510..3e2bce7543d4 100644
@@ -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;
 	}

Signature · 1 line

2.43.0

A message that is not in the archive (sent off-list or not yet received): CALp66yF0MZy9UKo_qGGeekxV0RNMNKZ_foqADVnqg+_YgEYqEA@mail.gmail.com

Alan Stern

2026-09-03 14:29 UTC #3 lore raw

Quoted 1 line · Rafael Alejandro Díaz Cruz

Do I need to create a new patch for this?

You don't really need to, but it wouldn't hurt and it would make things easier for Greg KH. Don't forget to list the changes from v1 below the "---" line.

Same goes for the second patch.

Alan Stern

Quoted 62 lines

Quoted 59 lines · Alan Stern
Quoted 2 lines · Rafael Alejandro Diaz Cruz
UAF is caused by syzkaller reproducer forcing the failure of gadgetfs_fill_super()
Just a minor comment: The fact that you used syzkaller to cause the UAF in your testing isn't relevant. Memory allocation failures can occur in real life, without fuzzing, and the driver needs to deal with them properly.
Quoted 9 lines
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.
Quoted 8 lines
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().
Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by [email protected]
Link https://syzkaller.appspot.com/bug?extid=4a5c87a01894ca37f25c
Signed-off-by Rafael Alejandro Diaz Cruz <[email protected]>
Since this fixes a real bug, you should add:
CC <[email protected]>
Quoted 1 line
---
Reviewed-by Alan Stern <[email protected]>
Alan Stern
Review
Quoted 4 lines
 drivers/usb/gadget/legacy/inode.c | 1 +
 1 file changed, 1 insertion(+)
(quoted diff) +0 -0
b/drivers/usb/gadget/legacy/inode.c
drivers/usb/gadget/legacy/inode.c gadgetfs_fill_super (struct super_block *sb,
Quoted 4 lines
index d87a8ab51510..3e2bce7543d4 100644
drivers/usb/gadget/legacy/inode.c +0 -0
@@ -2059,6 +2059,7 @@ gadgetfs_fill_super (struct super_block *sb,
struct fs_context *fc)
Quoted 11 lines
drivers/usb/gadget/legacy/inode.c +1 -1
      rc = gadgetfs_create_file(sb, CHIP, dev, &ep0_operations);
      if (rc) {
              put_dev(dev);
+             the_device = NULL;
              goto Enomem;
      }
 
--
2.43.0

Rafael Alejandro Díaz Cruz

author

2026-09-04 05:12 UTC #4 lore raw

Which changes? Currently I'm on the new v1 and all I would do is remove the top paragraph you said was unnecessary. Or do you mean add the "Reviewed-by" tag that you added? (Thanks btw!)

I'm not sure either about adding it below the "---" line. I haven't done that before....

I saw an example from some other patches and I guess I would do the following:

20 Reported-by: [email protected] 21 Link: https://syzkaller.appspot.com/bug?extid=4a5c87a01894ca37f25c
22 Signed-off-by: Rafael Alejandro Diaz Cruz <[email protected]>
23 --- // ADD
THE PART BETWEEN THE "---" ?
24 v2
25 - Reviewed-by: Alan Stern <[email protected]> 26 ---

 27  drivers/usb/gadget/legacy/inode.c | 1 +

28 1 file changed, 1 insertion(+)
29
30 diff --git a/drivers/usb

Is this correct?

Greg KH

maintainer

2026-09-04 05:18 UTC #5 lore raw

Quoted 18 lines · Rafael Alejandro Díaz Cruz

Which changes? Currently I'm on the new v1 and all I would do is remove the top paragraph you said was unnecessary. Or do you mean add the "Reviewed-by" tag that you added? (Thanks btw!)
I'm not sure either about adding it below the "---" line. I haven't done that before....
I saw an example from some other patches and I guess I would do the following:
20 Reported-by: [email protected] 21 Link: https://syzkaller.appspot.com/bug?extid=4a5c87a01894ca37f25c
22 Signed-off-by: Rafael Alejandro Diaz Cruz <[email protected]>
23 --- // ADD
THE PART BETWEEN THE "---" ?
24 v2
25 - Reviewed-by: Alan Stern <[email protected]> 26 ---

Quoted 6 lines

 27  drivers/usb/gadget/legacy/inode.c | 1 +
28 1 file changed, 1 insertion(+)
29
30 diff --git a/drivers/usb
Is this correct?

The kernel documentation should explain how to do a new version, perhaps re-read it?

thanks,

greg k-h

Alan Stern

Reviewed-by

2026-09-04 16:33 UTC #6 lore raw

In reply to Rafael Alejandro Díaz Cruz

Quoted 1 line · Rafael Alejandro Díaz Cruz

Which changes?

I had to go back and look at the old email messages to figure out what you are talking about here. Don't reply to emails without including enough context to let your reader (who may have to deal with hundreds of emails every day) know what's going on.

In this case, I was referring to the changes between your most recent patch submission (v1) and the submission you're planning to make (which will presumably be v2).

Quoted 3 lines

Currently I'm on the new v1 and all I would do
is remove the top paragraph you said was unnecessary. Or do you mean add the "Reviewed-by" tag that you added? (Thanks btw!)

I meant everything that will change between v1 and v2. Tags and all.

Quoted 2 lines

I'm not sure either about adding it below the "---" line. I haven't done that before....

Now's your chance. :-)

Quoted 11 lines

I saw an example from some other patches and I guess I would do the following:
20 Reported-by: [email protected] 21 Link: https://syzkaller.appspot.com/bug?extid=4a5c87a01894ca37f25c
22 Signed-off-by: Rafael Alejandro Diaz Cruz <[email protected]>
23 --- // ADD
THE PART BETWEEN THE "---" ?
24 v2
25 - Reviewed-by: Alan Stern <[email protected]> 26 ---

Quoted 6 lines

 27  drivers/usb/gadget/legacy/inode.c | 1 +
28 1 file changed, 1 insertion(+)
29
30 diff --git a/drivers/usb
Is this correct?

No.

All you have to do is copy the format of the other patch submissions in the email archives. It should look something like this (note that there is only one "---" line!):

----------------------------------------------------------------

Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")

Reported-by [email protected]

Link https://syzkaller.appspot.com/bug?extid=4a5c87a01894ca37f25c

Signed-off-by Rafael Alejandro Diaz Cruz <[email protected]>

Reviewed-by Alan Stern <[email protected]>

---

Changes from v1:

Removed unnecessary reference to syzkaller in the description.

Added Reviewed-by: tag.

 drivers/usb/gadget/legacy/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/gadget/legacy/inode.c +0 -0

----------------------------------------------------------------

Alan Stern