[syzbot] [fs?] WARNING in configfs_symlink

[syzbot] [fs?] WARNING in configfs_symlink

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    3b5f4b83c4ab Merge tag 'for-7.2-rc5-tag' of git://git.kern..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=139672a9580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=145fa60d73086782
dashboard link: https://syzkaller.appspot.com/bug?extid=608f7f2a86361e18ba0b
compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1431c561580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=102ae401580000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-3b5f4b83.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/ed6d6b480fd7/vmlinux-3b5f4b83.xz
kernel image: https://storage.googleapis.com/syzbot-assets/d5a591a74024/bzImage-3b5f4b83.xz

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

------------[ cut here ]------------
!atomic_read(&sd->s_count)
WARNING: fs/configfs/configfs_internal.h:146 at configfs_get fs/configfs/configfs_internal.h:146 [inline], CPU#0: syz.2.1991/9996
WARNING: fs/configfs/configfs_internal.h:146 at create_link fs/configfs/symlink.c:92 [inline], CPU#0: syz.2.1991/9996
WARNING: fs/configfs/configfs_internal.h:146 at configfs_symlink+0xdd4/0x11f0 fs/configfs/symlink.c:199, CPU#0: syz.2.1991/9996
Modules linked in:
CPU: 0 UID: 0 PID: 9996 Comm: syz.2.1991 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
RIP: 0010:configfs_get fs/configfs/configfs_internal.h:146 [inline]
RIP: 0010:create_link fs/configfs/symlink.c:92 [inline]
RIP: 0010:configfs_symlink+0xdd4/0x11f0 fs/configfs/symlink.c:199
Code: 48 c1 e8 03 80 3c 10 00 0f 85 63 03 00 00 48 8b 74 24 18 48 8b 3d 8c 26 bb 18 e8 97 6e ad ff e9 4f fe ff ff e8 dd df 48 ff 90 <0f> 0b 90 e9 a6 f8 ff ff e8 cf df 48 ff 48 c7 c7 40 49 eb 8e e8 a3
RSP: 0018:ffffc900020bfcc8 EFLAGS: 00010293
RAX: 0000000000000000 RBX: ffff888039383be8 RCX: ffffffff82c1790a
RDX: ffff888034e38000 RSI: ffffffff82c18063 RDI: ffff888034e38000
RBP: 0000000000000000 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff888046851c80
R13: ffff888046851d58 R14: ffff88802bdacc80 R15: ffff888058814000
FS:  00007fbd7c4276c0(0000) GS:ffff8880d5ddc000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fbd7c426ff8 CR3: 000000002d0d2000 CR4: 0000000000352ef0
Call Trace:
 <TASK>
 vfs_symlink fs/namei.c:5660 [inline]
 vfs_symlink+0x178/0x4d0 fs/namei.c:5639
 filename_symlinkat+0x2a6/0x560 fs/namei.c:5685
 __do_sys_symlinkat fs/namei.c:5705 [inline]
 __se_sys_symlinkat fs/namei.c:5700 [inline]
 __x64_sys_symlinkat+0x9c/0xe0 fs/namei.c:5700
 do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
 do_syscall_64+0x115/0x870 arch/x86/entry/syscall_64.c:94
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fbd7b59de99
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fbd7c427028 EFLAGS: 00000246 ORIG_RAX: 000000000000010a
RAX: ffffffffffffffda RBX: 00007fbd7b825fa0 RCX: 00007fbd7b59de99
RDX: 0000200000000340 RSI: ffffffffffffff9c RDI: 0000200000000300
RBP: 00007fbd7b633eaf R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fbd7b826038 R14: 00007fbd7b825fa0 R15: 00007fff25ff7358
 </TASK>


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

[PATCH] configfs: fix UAF race between rmdir and symlink

[Marcelo Mendes Spessoto Junior]

To avoid touching a target that is concurrently being removed,
configfs_get_config_item() and create_link() both rely on a hashed
dentry as proof that the config_item/configfs_dirent behind it is
still alive (using locks could lead to deadlocks, as described on
configfs_symlink).

However, configfs_remove_dir drops the last reference without
enforcing proper unhash over the dentry. This enables a possible
race condition where a symlink leads to UAF over a dentry that
has no reference but is still stale on the hash. Therefore, the
dentry removal must also enforce proper unhash, avoiding this
specific UAF scenario.

Verified against syzbot's C reproducer: no longer triggers the
WARN_ON/KASAN panics after this change.

Reported-by: syzbot+608f7f2a86361e18ba0b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=608f7f2a86361e18ba0b
Signed-off-by: Marcelo Mendes Spessoto Junior <marcelomspessoto@gmail.com>
---
 fs/configfs/dir.c     | 15 +++++++++++++++
 fs/configfs/symlink.c | 30 ++++++++++++++++++++++++++----
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 3c88f13f1ca2..ec6f3550178a 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -411,6 +411,21 @@ static void configfs_remove_dir(struct dentry *d)
 {
 	struct dentry * parent = dget(d->d_parent);
 
+	/*
+	 * Unhash before dropping any reference to the dirent/item this
+	 * dentry pins: a concurrent configfs_get_config_item() (e.g. from
+	 * configfs_symlink()'s target resolution, which runs unlocked
+	 * against directories it doesn't otherwise own) only checks
+	 * d_unhashed() under d_lock before pinning the item.  Unhashing
+	 * first ensures that check reliably fails once we're past this
+	 * point, instead of racing the dirent/item's refcount reaching
+	 * zero while the dentry is still (briefly) hashed.
+	 */
+	spin_lock(&d->d_lock);
+	if (simple_positive(d))
+		__d_drop(d);
+	spin_unlock(&d->d_lock);
+
 	configfs_remove_dirent(d);
 
 	if (d_really_is_positive(d)) {
diff --git a/fs/configfs/symlink.c b/fs/configfs/symlink.c
index 31eb28b27309..b8043a6b0b42 100644
--- a/fs/configfs/symlink.c
+++ b/fs/configfs/symlink.c
@@ -78,18 +78,40 @@ static int create_link(struct config_item *parent_item,
 		       struct config_item *item,
 		       struct dentry *dentry)
 {
-	struct configfs_dirent *target_sd = item->ci_dentry->d_fsdata;
+	struct dentry *target_dentry = item->ci_dentry;
+	struct configfs_dirent *target_sd;
 	char *body;
 	int ret;
 
-	if (!configfs_dirent_is_ready(target_sd))
+	/*
+	 * item is pinned by the caller, but that only keeps the config_item
+	 * itself alive.  item->ci_dentry's configfs_dirent (and thus its
+	 * s_count) is a separate refcount that a concurrent rmdir of this
+	 * same directory can drop to zero and free independently -- see the
+	 * matching d_lock/d_unhashed() dance in configfs_get_config_item()
+	 * and the unhash-before-free ordering configfs_remove_dir() now
+	 * guarantees.  Do the same check here instead of trusting
+	 * target_dentry->d_fsdata unconditionally.
+	 */
+	spin_lock(&target_dentry->d_lock);
+	if (d_unhashed(target_dentry)) {
+		spin_unlock(&target_dentry->d_lock);
 		return -ENOENT;
+	}
+	target_sd = configfs_get(target_dentry->d_fsdata);
+	spin_unlock(&target_dentry->d_lock);
+
+	if (!configfs_dirent_is_ready(target_sd)) {
+		configfs_put(target_sd);
+		return -ENOENT;
+	}
 
 	body = kzalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!body)
+	if (!body) {
+		configfs_put(target_sd);
 		return -ENOMEM;
+	}
 
-	configfs_get(target_sd);
 	spin_lock(&configfs_dirent_lock);
 	if (target_sd->s_type & CONFIGFS_USET_DROPPING) {
 		spin_unlock(&configfs_dirent_lock);
-- 
2.55.0