[syzbot] [usb?] KASAN: slab-use-after-free Read in ep_open

[syzbot] [usb?] KASAN: slab-use-after-free Read in ep_open

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    365f9c7a6b81 Merge branches 'for-next/core' and 'for-next/..
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci
console output: https://syzkaller.appspot.com/x/log.txt?x=13731632580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=72c97575381cec47
dashboard link: https://syzkaller.appspot.com/bug?extid=df9e891bf8ea586f846b
compiler:       Debian clang version 22.1.8 (++20260613092233+e80beda6e255-1~exp1~20260613092250.77), Debian LLD 22.1.8
userspace arch: arm64
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=136d6499580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=138d6449580000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/eafe0adb7386/disk-365f9c7a.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/29422d607579/vmlinux-365f9c7a.xz
kernel image: https://storage.googleapis.com/syzbot-assets/98ca955a8b54/Image-365f9c7a.gz.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+df9e891bf8ea586f846b@syzkaller.appspotmail.com

==================================================================
BUG: KASAN: slab-use-after-free in __mutex_lock_common kernel/locking/mutex.c:625 [inline]
BUG: KASAN: slab-use-after-free in __mutex_lock+0x130/0xf14 kernel/locking/mutex.c:821
Read of size 8 at addr ffff0000efe3d058 by task syz-executor/5909

CPU: 1 UID: 0 PID: 5909 Comm: syz-executor Not tainted syzkaller #0 PREEMPT 
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 07/02/2026
Call trace:
 show_stack+0x2c/0x3c arch/arm64/kernel/stacktrace.c:499 (C)
 __dump_stack+0x30/0x40 lib/dump_stack.c:94
 dump_stack_lvl+0xd8/0x12c lib/dump_stack.c:120
 print_address_description+0xb0/0x238 mm/kasan/report.c:378
 print_report+0x68/0x84 mm/kasan/report.c:482
 kasan_report+0x8c/0xc4 mm/kasan/report.c:595
 __asan_report_load8_noabort+0x20/0x2c mm/kasan/report_generic.c:381
 __mutex_lock_common kernel/locking/mutex.c:625 [inline]
 __mutex_lock+0x130/0xf14 kernel/locking/mutex.c:821
 mutex_lock_interruptible_nested+0x24/0x30 kernel/locking/mutex.c:899
 ep_open+0x58/0x25c drivers/usb/gadget/legacy/inode.c:823
 do_dentry_open+0x5c4/0xfc0 fs/open.c:947
 vfs_open+0x44/0x2dc fs/open.c:1052
 do_open fs/namei.c:4700 [inline]
 path_openat+0x21fc/0x2a60 fs/namei.c:4863
 do_file_open+0x1c8/0x2e8 fs/namei.c:4892
 do_sys_openat2+0x114/0x1e8 fs/open.c:1368
 do_sys_open+0xb0/0xe0 fs/open.c:1374
 __do_sys_openat fs/open.c:1390 [inline]
 __se_sys_openat fs/open.c:1385 [inline]
 __arm64_sys_openat+0xa0/0xbc fs/open.c:1385
 __invoke_syscall arch/arm64/kernel/syscall.c:35 [inline]
 invoke_syscall+0x98/0x244 arch/arm64/kernel/syscall.c:49
 el0_svc_common+0xec/0x23c arch/arm64/kernel/syscall.c:121
 do_el0_svc+0x4c/0x5c arch/arm64/kernel/syscall.c:140
 el0_svc+0x64/0x260 arch/arm64/kernel/entry-common.c:736
 el0t_64_sync_handler+0x48/0x148 arch/arm64/kernel/entry-common.c:755
 el0t_64_sync+0x198/0x19c arch/arm64/kernel/entry.S:594

Allocated by task 6282:
 kasan_save_stack mm/kasan/common.c:57 [inline]
 kasan_save_track+0x40/0x78 mm/kasan/common.c:78
 kasan_save_alloc_info+0x44/0x54 mm/kasan/generic.c:570
 poison_kmalloc_redzone mm/kasan/common.c:398 [inline]
 __kasan_kmalloc+0x9c/0xb4 mm/kasan/common.c:415
 kasan_kmalloc include/linux/kasan.h:263 [inline]
 __kmalloc_cache_noprof+0x2e8/0x5ec mm/slub.c:5515
 _kmalloc_noprof include/linux/slab.h:969 [inline]
 _kzalloc_noprof include/linux/slab.h:1290 [inline]
 activate_ep_files drivers/usb/gadget/legacy/inode.c:1601 [inline]
 gadgetfs_bind+0x204/0x75c drivers/usb/gadget/legacy/inode.c:1692
 gadget_bind_driver+0x274/0x834 drivers/usb/gadget/udc/core.c:1662
 call_driver_probe drivers/base/dd.c:-1 [inline]
 really_probe+0x29c/0x800 drivers/base/dd.c:706
 __driver_probe_device+0x1e0/0x350 drivers/base/dd.c:868
 driver_probe_device+0x6c/0x19c drivers/base/dd.c:898
 __driver_attach+0x168/0x384 drivers/base/dd.c:1292
 bus_for_each_dev+0x128/0x1b4 drivers/base/bus.c:383
 driver_attach+0x50/0x60 drivers/base/dd.c:1310
 bus_add_driver+0x208/0x500 drivers/base/bus.c:763
 driver_register+0x220/0x310 drivers/base/driver.c:174
 usb_gadget_register_driver_owner+0xf0/0x25c drivers/usb/gadget/udc/core.c:1752
 dev_config+0x91c/0xcc0 drivers/usb/gadget/legacy/inode.c:1883
 vfs_write+0x2a4/0xa0c fs/read_write.c:685
 ksys_write+0x12c/0x228 fs/read_write.c:739
 __do_sys_write fs/read_write.c:750 [inline]
 __se_sys_write fs/read_write.c:747 [inline]
 __arm64_sys_write+0x7c/0x90 fs/read_write.c:747
 __invoke_syscall arch/arm64/kernel/syscall.c:35 [inline]
 invoke_syscall+0x98/0x244 arch/arm64/kernel/syscall.c:49
 el0_svc_common+0xec/0x23c arch/arm64/kernel/syscall.c:121
 do_el0_svc+0x4c/0x5c arch/arm64/kernel/syscall.c:140
 el0_svc+0x64/0x260 arch/arm64/kernel/entry-common.c:736
 el0t_64_sync_handler+0x48/0x148 arch/arm64/kernel/entry-common.c:755
 el0t_64_sync+0x198/0x19c arch/arm64/kernel/entry.S:594

Freed by task 6283:
 kasan_save_stack mm/kasan/common.c:57 [inline]
 kasan_save_track+0x40/0x78 mm/kasan/common.c:78
 kasan_save_free_info+0x58/0x70 mm/kasan/generic.c:584
 poison_slab_object mm/kasan/common.c:253 [inline]
 __kasan_slab_free+0x74/0xa4 mm/kasan/common.c:285
 kasan_slab_free include/linux/kasan.h:235 [inline]
 slab_free_hook mm/slub.c:2705 [inline]
 slab_free mm/slub.c:6405 [inline]
 kfree+0x188/0x690 mm/slub.c:6720
 put_ep+0xd0/0x144 drivers/usb/gadget/legacy/inode.c:225
 destroy_ep_files+0x244/0x2dc drivers/usb/gadget/legacy/inode.c:1582
 gadgetfs_unbind+0x114/0x1f0 drivers/usb/gadget/legacy/inode.c:1655
 gadget_unbind_driver+0x188/0x790 drivers/usb/gadget/udc/core.c:1724
 device_remove+0xc4/0x134 drivers/base/dd.c:616
 __device_release_driver drivers/base/dd.c:1349 [inline]
 device_release_driver_internal+0x124/0x20c drivers/base/dd.c:1372
 driver_detach+0x124/0x1b4 drivers/base/dd.c:1435
 bus_remove_driver+0x124/0x22c drivers/base/bus.c:832
 driver_unregister+0x78/0xac drivers/base/driver.c:202
 usb_gadget_unregister_driver+0x50/0x78 drivers/usb/gadget/udc/core.c:1784
 dev_release+0x8c/0x15c drivers/usb/gadget/legacy/inode.c:1198
 __fput+0x338/0x74c fs/file_table.c:512
 ____fput+0x20/0x30 fs/file_table.c:540
 task_work_run+0x1cc/0x25c kernel/task_work.c:233
 exit_task_work include/linux/task_work.h:40 [inline]
 do_exit+0x58c/0x1a74 kernel/exit.c:1009
 __do_sys_exit kernel/exit.c:1119 [inline]
 __se_sys_exit kernel/exit.c:1117 [inline]
 do_group_exit+0x0/0x238 kernel/exit.c:1117
 __invoke_syscall arch/arm64/kernel/syscall.c:35 [inline]
 invoke_syscall+0x98/0x244 arch/arm64/kernel/syscall.c:49
 el0_svc_common+0xec/0x23c arch/arm64/kernel/syscall.c:121
 do_el0_svc+0x4c/0x5c arch/arm64/kernel/syscall.c:140
 el0_svc+0x64/0x260 arch/arm64/kernel/entry-common.c:736
 el0t_64_sync_handler+0x48/0x148 arch/arm64/kernel/entry-common.c:755
 el0t_64_sync+0x198/0x19c arch/arm64/kernel/entry.S:594

The buggy address belongs to the object at ffff0000efe3d000
 which belongs to the cache kmalloc-512 of size 512
The buggy address is located 88 bytes inside of
 freed 512-byte region [ffff0000efe3d000, ffff0000efe3d200)

The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff0000efe3dc00 pfn:0x12fe3c


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Forwarded: [PATCH] usb: gadgetfs: fix use-after-free in ep_open()

[syzbot]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] usb: gadgetfs: fix use-after-free in ep_open()
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci


ep_open() dereferenced inode->i_private (an ep_data pointer) and
locked its embedded mutex before ever validating that the object was
still alive:

    struct ep_data *data = inode->i_private;
    if (mutex_lock_interruptible(&data->lock) != 0)
            return -EINTR;
    spin_lock_irq(&data->dev->lock);
    if (data->dev->state == STATE_DEV_UNBOUND)
            ...

The liveness check (dev->state == STATE_DEV_UNBOUND) only happened
*after* data->lock had already been touched. destroy_ep_files(),
called from gadgetfs_unbind(), can free the same ep_data
concurrently via put_ep(), since nothing prevented an in-flight
open() from racing the teardown. KASAN reports a slab-use-after-free
read inside __mutex_lock_common() from ep_open(), with the object
freed by a concurrent gadgetfs_unbind() -> destroy_ep_files() ->
put_ep() -> kfree() on another task.

This exact race was identified by Al Viro on LKML in 2006 ("races in
drivers/usb/gadget/inode.c"), including the correct fix direction,
but it was never applied to ep_open() itself. Later fixes to this
file (the udc_usage counter, CVE-2022-4382) hardened the I/O paths
and the mount/unmount race, but left this specific ordering bug in
place.

Fix it by reordering ep_open() so that dev->state and inode->i_private
are only read while holding dev->lock -- reached via
inode->i_sb->s_fs_info, which stays valid for the life of the mount,
independent of any individual ep_data -- and by pinning the object
with get_ep() before dev->lock is dropped and before data->lock is
touched. This guarantees put_ep()/kfree() cannot race the mutex lock
in ep_open().

This fix depends on dev->state being set to STATE_DEV_UNBOUND under
dev->lock before any call to destroy_ep_files(), so that ep_open()
seeing a "bound" state is a guarantee that no free is in progress.
gadgetfs_unbind() already does this correctly. activate_ep_files()'s
enomem0 error-cleanup path did not: it called destroy_ep_files()
directly without first setting dev->state, which would have left the
same race open on the bind-failure path. This patch adds the missing
state transition there as well, so the invariant holds for every
caller of destroy_ep_files().

Reported-by: syzbot+df9e891bf8ea586f846b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=df9e891bf8ea586f846b
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 drivers/usb/gadget/legacy/inode.c | 37 ++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index d87a8ab51510..95e3bcedd33d 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -817,25 +817,39 @@ ep_config (struct ep_data *data, const char *buf, size_t len)
 static int
 ep_open (struct inode *inode, struct file *fd)
 {
-	struct ep_data		*data = inode->i_private;
-	int			value = -EBUSY;
+	struct dev_data *dev = inode->i_sb->s_fs_info;
+	struct ep_data *data;
+	int value = -ENODEV;
+
+	spin_lock_irq (&dev->lock);
+	if (dev->state == STATE_DEV_UNBOUND) {
+		spin_unlock_irq (&dev->lock);
+		return -ENOENT;
+	}
+	data = inode->i_private;
+	get_ep (data);
+	spin_unlock_irq (&dev->lock);
 
-	if (mutex_lock_interruptible(&data->lock) != 0)
+	if (mutex_lock_interruptible(&data->lock) != 0) {
+		put_ep (data);
 		return -EINTR;
-	spin_lock_irq (&data->dev->lock);
-	if (data->dev->state == STATE_DEV_UNBOUND)
+	}
+
+	value = -EBUSY;
+	spin_lock_irq (&dev->lock);
+	if (dev->state == STATE_DEV_UNBOUND)
 		value = -ENOENT;
 	else if (data->state == STATE_EP_DISABLED) {
 		value = 0;
 		data->state = STATE_EP_READY;
-		get_ep (data);
 		fd->private_data = data;
-		VDEBUG (data->dev, "%s ready\n", data->name);
+		VDEBUG (dev, "%s ready\n", data->name);
 	} else
-		DBG (data->dev, "%s state %d\n",
-			data->name, data->state);
-	spin_unlock_irq (&data->dev->lock);
+		DBG (dev, "%s state %d\n", data->name, data->state);
+	spin_unlock_irq (&dev->lock);
 	mutex_unlock(&data->lock);
+	if (value)
+		put_ep (data);
 	return value;
 }
 
@@ -1632,6 +1646,9 @@ static int activate_ep_files (struct dev_data *dev)
 	kfree (data);
 enomem0:
 	DBG (dev, "%s enomem\n", __func__);
+	spin_lock_irq (&dev->lock);
+	dev->state = STATE_DEV_UNBOUND;
+	spin_unlock_irq (&dev->lock);
 	destroy_ep_files (dev);
 	return -ENOMEM;
 }
-- 
2.43.0

Forwarded: [PATCH] usb: gadgetfs: fix use-after-free in ep_open()

[syzbot]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] usb: gadgetfs: fix use-after-free in ep_open()
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci


ep_open() dereferenced inode->i_private and locked its embedded
mutex before checking whether the underlying ep_data was still
alive. destroy_ep_files(), called from gadgetfs_unbind(), can free
that same ep_data concurrently, since nothing prevented an in-flight
open() from racing the teardown. KASAN reports a slab-use-after-free
read inside __mutex_lock_common() from ep_open(), with the object
freed by a concurrent gadgetfs_unbind() -> destroy_ep_files() ->
put_ep() -> kfree().

This race was identified by Al Viro on LKML in 2006 ("races in
drivers/usb/gadget/inode.c") but never fixed in ep_open() itself.

Fix it by checking dev->state and reading inode->i_private only
under dev->lock, and pinning the result with get_ep() before
dev->lock is dropped and before data->lock is touched. dev is
reached via the file-scope singleton the_device, which stays valid
for the duration of any open() in progress, so no dereference of
ep_data happens before its liveness is confirmed.

This depends on dev->state being set to STATE_DEV_UNBOUND under
dev->lock before destroy_ep_files() is called. gadgetfs_unbind()
already does this; activate_ep_files()'s enomem0 cleanup path did
not, leaving the same race open on the bind-failure path. This patch
adds the missing state transition there too.

Reported-by: syzbot+df9e891bf8ea586f846b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=df9e891bf8ea586f846b
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 drivers/usb/gadget/legacy/inode.c | 40 +++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index d87a8ab51510..0d0989d79020 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -235,6 +235,7 @@ static void put_ep (struct ep_data *data)
 
 static const char *CHIP;
 static DEFINE_MUTEX(sb_mutex);		/* Serialize superblock operations */
+static struct dev_data *the_device;
 
 /*----------------------------------------------------------------------*/
 
@@ -817,25 +818,39 @@ ep_config (struct ep_data *data, const char *buf, size_t len)
 static int
 ep_open (struct inode *inode, struct file *fd)
 {
-	struct ep_data		*data = inode->i_private;
-	int			value = -EBUSY;
+	struct dev_data *dev = the_device;
+	struct ep_data *data;
+	int value = -ENODEV;
+
+	spin_lock_irq (&dev->lock);
+	if (dev->state == STATE_DEV_UNBOUND) {
+		spin_unlock_irq (&dev->lock);
+		return -ENOENT;
+	}
+	data = inode->i_private;
+	get_ep (data);
+	spin_unlock_irq (&dev->lock);
 
-	if (mutex_lock_interruptible(&data->lock) != 0)
+	if (mutex_lock_interruptible(&data->lock) != 0) {
+		put_ep (data);
 		return -EINTR;
-	spin_lock_irq (&data->dev->lock);
-	if (data->dev->state == STATE_DEV_UNBOUND)
+	}
+
+	value = -EBUSY;
+	spin_lock_irq (&dev->lock);
+	if (dev->state == STATE_DEV_UNBOUND)
 		value = -ENOENT;
 	else if (data->state == STATE_EP_DISABLED) {
 		value = 0;
 		data->state = STATE_EP_READY;
-		get_ep (data);
 		fd->private_data = data;
-		VDEBUG (data->dev, "%s ready\n", data->name);
+		VDEBUG (dev, "%s ready\n", data->name);
 	} else
-		DBG (data->dev, "%s state %d\n",
-			data->name, data->state);
-	spin_unlock_irq (&data->dev->lock);
+		DBG (dev, "%s state %d\n", data->name, data->state);
+	spin_unlock_irq (&dev->lock);
 	mutex_unlock(&data->lock);
+	if (value)
+		put_ep (data);
 	return value;
 }
 
@@ -1632,6 +1647,9 @@ static int activate_ep_files (struct dev_data *dev)
 	kfree (data);
 enomem0:
 	DBG (dev, "%s enomem\n", __func__);
+	spin_lock_irq (&dev->lock);
+	dev->state = STATE_DEV_UNBOUND;
+	spin_unlock_irq (&dev->lock);
 	destroy_ep_files (dev);
 	return -ENOMEM;
 }
@@ -1663,8 +1681,6 @@ gadgetfs_unbind (struct usb_gadget *gadget)
 	put_dev (dev);
 }
 
-static struct dev_data		*the_device;
-
 static int gadgetfs_bind(struct usb_gadget *gadget,
 		struct usb_gadget_driver *driver)
 {
-- 
2.43.0

Forwarded: [PATCH] usb: gadgetfs: fix use-after-free in ep_open()

[syzbot]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] usb: gadgetfs: fix use-after-free in ep_open()
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci


ep_open() dereferenced inode->i_private and locked its embedded
mutex before checking whether the underlying ep_data was still
alive. destroy_ep_files(), called from gadgetfs_unbind(), can free
that same ep_data concurrently, since nothing prevented an in-flight
open() from racing the teardown. KASAN reports a slab-use-after-free
in ep_open(), with the object freed by a concurrent gadgetfs_unbind()
-> destroy_ep_files() -> put_ep() -> kfree().

This race was identified by Al Viro on LKML in 2006 ("races in
drivers/usb/gadget/inode.c"), including the fix direction, but it
was never applied to ep_open() itself.

Fix it as originally suggested: add an inode back-pointer to
ep_data, have destroy_ep_files() clear inode->i_private to NULL
under dev->lock before the object can be freed, and have ep_open()
read inode->i_private and pin the result with get_ep() under that
same lock, bailing out with -ENOENT if it finds NULL. This makes
liveness self-evident at the point of use instead of being inferred
from device state, so it holds regardless of which path frees the
endpoint.

gadgetfs_create_file() gains an optional inode-out parameter so
activate_ep_files() can capture the inode it creates; the ep0 caller
in gadgetfs_fill_super() passes NULL, since dev_data is unaffected
by this fix.

Reported-by: syzbot+df9e891bf8ea586f846b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=df9e891bf8ea586f846b
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 drivers/usb/gadget/legacy/inode.c | 51 +++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index d87a8ab51510..f566b2ec7c38 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -207,6 +207,7 @@ struct ep_data {
 	struct usb_endpoint_descriptor	desc, hs_desc;
 	struct list_head		epfiles;
 	wait_queue_head_t		wait;
+	struct inode                    *inode;
 };
 
 static inline void get_ep (struct ep_data *data)
@@ -235,6 +236,7 @@ static void put_ep (struct ep_data *data)
 
 static const char *CHIP;
 static DEFINE_MUTEX(sb_mutex);		/* Serialize superblock operations */
+static struct dev_data *the_device;
 
 /*----------------------------------------------------------------------*/
 
@@ -817,25 +819,39 @@ ep_config (struct ep_data *data, const char *buf, size_t len)
 static int
 ep_open (struct inode *inode, struct file *fd)
 {
-	struct ep_data		*data = inode->i_private;
-	int			value = -EBUSY;
+	struct dev_data *dev = the_device;
+	struct ep_data *data;
+	int value = -ENODEV;
+
+	spin_lock_irq (&dev->lock);
+	data = inode->i_private;
+	if (data == NULL) {
+		spin_unlock_irq (&dev->lock);
+		return -ENOENT;
+	}
+	get_ep (data);
+	spin_unlock_irq (&dev->lock);
 
-	if (mutex_lock_interruptible(&data->lock) != 0)
+	if (mutex_lock_interruptible(&data->lock) != 0) {
+		put_ep (data);
 		return -EINTR;
-	spin_lock_irq (&data->dev->lock);
-	if (data->dev->state == STATE_DEV_UNBOUND)
+	}
+
+	value = -EBUSY;
+	spin_lock_irq (&dev->lock);
+	if (dev->state == STATE_DEV_UNBOUND)
 		value = -ENOENT;
 	else if (data->state == STATE_EP_DISABLED) {
 		value = 0;
 		data->state = STATE_EP_READY;
-		get_ep (data);
 		fd->private_data = data;
-		VDEBUG (data->dev, "%s ready\n", data->name);
+		VDEBUG (dev, "%s ready\n", data->name);
 	} else
-		DBG (data->dev, "%s state %d\n",
-			data->name, data->state);
-	spin_unlock_irq (&data->dev->lock);
+		DBG (dev, "%s state %d\n", data->name, data->state);
+	spin_unlock_irq (&dev->lock);
 	mutex_unlock(&data->lock);
+	if (value)
+		put_ep (data);
 	return value;
 }
 
@@ -1563,6 +1579,7 @@ static void destroy_ep_files (struct dev_data *dev)
 		/* break link to FS */
 		ep = list_first_entry (&dev->epfiles, struct ep_data, epfiles);
 		list_del_init (&ep->epfiles);
+		ep->inode->i_private = NULL;
 		spin_unlock_irq (&dev->lock);
 
 		/* break link to controller */
@@ -1588,7 +1605,8 @@ static void destroy_ep_files (struct dev_data *dev)
 
 
 static int gadgetfs_create_file (struct super_block *sb, char const *name,
-		void *data, const struct file_operations *fops);
+		void *data, const struct file_operations *fops,
+                struct inode **inode_out);
 
 static int activate_ep_files (struct dev_data *dev)
 {
@@ -1618,7 +1636,7 @@ static int activate_ep_files (struct dev_data *dev)
 			goto enomem1;
 
 		err = gadgetfs_create_file (dev->sb, data->name,
-				data, &ep_io_operations);
+				data, &ep_io_operations, &data->inode);
 		if (err)
 			goto enomem2;
 		list_add_tail (&data->epfiles, &dev->epfiles);
@@ -1663,8 +1681,6 @@ gadgetfs_unbind (struct usb_gadget *gadget)
 	put_dev (dev);
 }
 
-static struct dev_data		*the_device;
-
 static int gadgetfs_bind(struct usb_gadget *gadget,
 		struct usb_gadget_driver *driver)
 {
@@ -1984,7 +2000,7 @@ gadgetfs_make_inode (struct super_block *sb,
  * so inode and dentry are paired, until device reconfig.
  */
 static int gadgetfs_create_file (struct super_block *sb, char const *name,
-		void *data, const struct file_operations *fops)
+		void *data, const struct file_operations *fops, struct inode **inode_out)
 {
 	struct dentry	*dentry;
 	struct inode	*inode;
@@ -1994,6 +2010,9 @@ static int gadgetfs_create_file (struct super_block *sb, char const *name,
 	if (!inode)
 		return -ENOMEM;
 
+	if(inode_out)
+		*inode_out = inode;
+
 	dentry = simple_start_creating(sb->s_root, name);
 	if (IS_ERR(dentry)) {
 		iput(inode);
@@ -2056,7 +2075,7 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
 		goto Enomem;
 
 	dev->sb = sb;
-	rc = gadgetfs_create_file(sb, CHIP, dev, &ep0_operations);
+	rc = gadgetfs_create_file(sb, CHIP, dev, &ep0_operations, NULL);
 	if (rc) {
 		put_dev(dev);
 		goto Enomem;
-- 
2.43.0

Forwarded: [PATCH v2] usb: gadgetfs: fix use-after-free in ep_open()

[syzbot]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH v2] usb: gadgetfs: fix use-after-free in ep_open()
Author: kartikey406@gmail.com

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci


ep_open() dereferenced inode->i_private and locked its embedded
mutex before checking whether the underlying ep_data was still
alive. destroy_ep_files(), called from gadgetfs_unbind(), can free
that same ep_data concurrently, since nothing prevented an in-flight
open() from racing the teardown. KASAN reports a slab-use-after-free
in ep_open(), with the object freed by a concurrent gadgetfs_unbind()
-> destroy_ep_files() -> put_ep() -> kfree().

Fix it by adding an inode back-pointer to ep_data, having
destroy_ep_files() clear inode->i_private to NULL under dev->lock
before the object can be freed, and having ep_open() read
inode->i_private and pin the result with get_ep() under that same
lock, bailing out with -ENOENT if it finds NULL. This makes liveness
self-evident at the point of use instead of being inferred from
device state, so it holds regardless of which path frees the
endpoint.

dev is derived from inode->i_sb->s_fs_info, set in
gadgetfs_fill_super(), rather than from the file's the_device
singleton, so ep_open() is correctly scoped to the specific mount
the inode belongs to.

gadgetfs_create_file() now returns the created inode (or an
ERR_PTR) directly instead of taking an output parameter, so
activate_ep_files() can store it in data->inode without an
unexplained NULL at call sites that don't need it.

Reported-by: syzbot+df9e891bf8ea586f846b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=df9e891bf8ea586f846b
Fixes: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 ("Linux-2.6.12-rc2")
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
v2:
 - Added Fixes: tag; git blame traces the bug to the 2005 git
   import boundary, confirmed no later commit changed the ordering
 - Derive dev from inode->i_sb->s_fs_info instead of the new
   the_device singleton, so the lookup is scoped to the inode's
   own mount instead of a global (Greg)
 - gadgetfs_create_file() now returns struct inode * / ERR_PTR
   instead of taking an inode_out output parameter, so call sites
   don't need an unexplained NULL argument (Greg)
---
 drivers/usb/gadget/legacy/inode.c | 61 ++++++++++++++++++++-----------
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
index d87a8ab51510..e5a316777db2 100644
--- a/drivers/usb/gadget/legacy/inode.c
+++ b/drivers/usb/gadget/legacy/inode.c
@@ -207,6 +207,7 @@ struct ep_data {
 	struct usb_endpoint_descriptor	desc, hs_desc;
 	struct list_head		epfiles;
 	wait_queue_head_t		wait;
+	struct inode                    *inode;
 };
 
 static inline void get_ep (struct ep_data *data)
@@ -235,6 +236,7 @@ static void put_ep (struct ep_data *data)
 
 static const char *CHIP;
 static DEFINE_MUTEX(sb_mutex);		/* Serialize superblock operations */
+static struct dev_data *the_device;
 
 /*----------------------------------------------------------------------*/
 
@@ -817,25 +819,39 @@ ep_config (struct ep_data *data, const char *buf, size_t len)
 static int
 ep_open (struct inode *inode, struct file *fd)
 {
-	struct ep_data		*data = inode->i_private;
-	int			value = -EBUSY;
+	struct dev_data *dev = inode->i_sb->s_fs_info;
+	struct ep_data *data;
+	int value = -ENODEV;
+
+	spin_lock_irq (&dev->lock);
+	data = inode->i_private;
+	if (data == NULL) {
+		spin_unlock_irq (&dev->lock);
+		return -ENOENT;
+	}
+	get_ep (data);
+	spin_unlock_irq (&dev->lock);
 
-	if (mutex_lock_interruptible(&data->lock) != 0)
+	if (mutex_lock_interruptible(&data->lock) != 0) {
+		put_ep (data);
 		return -EINTR;
-	spin_lock_irq (&data->dev->lock);
-	if (data->dev->state == STATE_DEV_UNBOUND)
+	}
+
+	value = -EBUSY;
+	spin_lock_irq (&dev->lock);
+	if (dev->state == STATE_DEV_UNBOUND)
 		value = -ENOENT;
 	else if (data->state == STATE_EP_DISABLED) {
 		value = 0;
 		data->state = STATE_EP_READY;
-		get_ep (data);
 		fd->private_data = data;
-		VDEBUG (data->dev, "%s ready\n", data->name);
+		VDEBUG (dev, "%s ready\n", data->name);
 	} else
-		DBG (data->dev, "%s state %d\n",
-			data->name, data->state);
-	spin_unlock_irq (&data->dev->lock);
+		DBG (dev, "%s state %d\n", data->name, data->state);
+	spin_unlock_irq (&dev->lock);
 	mutex_unlock(&data->lock);
+	if (value)
+		put_ep (data);
 	return value;
 }
 
@@ -1563,6 +1579,7 @@ static void destroy_ep_files (struct dev_data *dev)
 		/* break link to FS */
 		ep = list_first_entry (&dev->epfiles, struct ep_data, epfiles);
 		list_del_init (&ep->epfiles);
+		ep->inode->i_private = NULL;
 		spin_unlock_irq (&dev->lock);
 
 		/* break link to controller */
@@ -1587,14 +1604,13 @@ static void destroy_ep_files (struct dev_data *dev)
 }
 
 
-static int gadgetfs_create_file (struct super_block *sb, char const *name,
+static struct inode *gadgetfs_create_file (struct super_block *sb, char const *name,
 		void *data, const struct file_operations *fops);
 
 static int activate_ep_files (struct dev_data *dev)
 {
 	struct usb_ep	*ep;
 	struct ep_data	*data;
-	int err;
 
 	gadget_for_each_ep (ep, dev->gadget) {
 
@@ -1617,9 +1633,9 @@ static int activate_ep_files (struct dev_data *dev)
 		if (!data->req)
 			goto enomem1;
 
-		err = gadgetfs_create_file (dev->sb, data->name,
+		data->inode = gadgetfs_create_file (dev->sb, data->name,
 				data, &ep_io_operations);
-		if (err)
+		if (IS_ERR(data->inode))
 			goto enomem2;
 		list_add_tail (&data->epfiles, &dev->epfiles);
 	}
@@ -1663,8 +1679,6 @@ gadgetfs_unbind (struct usb_gadget *gadget)
 	put_dev (dev);
 }
 
-static struct dev_data		*the_device;
-
 static int gadgetfs_bind(struct usb_gadget *gadget,
 		struct usb_gadget_driver *driver)
 {
@@ -1983,7 +1997,7 @@ gadgetfs_make_inode (struct super_block *sb,
 /* creates in fs root directory, so non-renamable and non-linkable.
  * so inode and dentry are paired, until device reconfig.
  */
-static int gadgetfs_create_file (struct super_block *sb, char const *name,
+static struct inode *gadgetfs_create_file (struct super_block *sb, char const *name,
 		void *data, const struct file_operations *fops)
 {
 	struct dentry	*dentry;
@@ -1992,18 +2006,18 @@ static int gadgetfs_create_file (struct super_block *sb, char const *name,
 	inode = gadgetfs_make_inode (sb, data, fops,
 			S_IFREG | (default_perm & S_IRWXUGO));
 	if (!inode)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	dentry = simple_start_creating(sb->s_root, name);
 	if (IS_ERR(dentry)) {
 		iput(inode);
-		return PTR_ERR(dentry);
+		return ERR_CAST(dentry);
 	}
 
 	d_make_persistent(dentry, inode);
 
 	simple_done_creating(dentry);
-	return 0;
+	return inode;
 }
 
 static const struct super_operations gadget_fs_operations = {
@@ -2056,8 +2070,11 @@ gadgetfs_fill_super (struct super_block *sb, struct fs_context *fc)
 		goto Enomem;
 
 	dev->sb = sb;
-	rc = gadgetfs_create_file(sb, CHIP, dev, &ep0_operations);
-	if (rc) {
+	sb->s_fs_info = dev;
+	inode = gadgetfs_create_file(sb, CHIP, dev, &ep0_operations);
+	if (IS_ERR(inode)) {
+		rc = PTR_ERR(inode);
+		sb->s_fs_info = NULL;
 		put_dev(dev);
 		goto Enomem;
 	}
-- 
2.43.0