[syzbot] [fs?] KASAN: null-ptr-deref Read in ida_free (3)

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: da8e7da11e4b Merge tag 'nfsd-6.3-4' of git://git.kernel.or..
git tree: upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=1266331ec80000
kernel config: https://syzkaller.appspot.com/x/.config?x=acdb62bf488a8fe5
dashboard link: https://syzkaller.appspot.com/bug?extid=8ac3859139c685c4f597
compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11639815c80000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12128b1ec80000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/62e9c5f4bead/disk-da8e7da1.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/c11aa933e2a7/vmlinux-da8e7da1.xz
kernel image: https://storage.googleapis.com/syzbot-assets/7a21bdd49c84/bzImage-da8e7da1.xz

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

==================================================================
BUG: KASAN: null-ptr-deref in instrument_atomic_read include/linux/instrumented.h:72 [inline]
BUG: KASAN: null-ptr-deref in _test_bit include/asm-generic/bitops/instrumented-non-atomic.h:141 [inline]
BUG: KASAN: null-ptr-deref in ida_free+0x1b9/0x400 lib/idr.c:511
Read of size 8 at addr 0000000000000000 by task syz-executor237/5830

CPU: 1 PID: 5830 Comm: syz-executor237 Not tainted 6.3.0-rc3-syzkaller-00338-gda8e7da11e4b #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 03/02/2023
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x1e7/0x2d0 lib/dump_stack.c:106
print_report+0xe6/0x540 mm/kasan/report.c:433
kasan_report+0x176/0x1b0 mm/kasan/report.c:536
kasan_check_range+0x283/0x290 mm/kasan/generic.c:187
instrument_atomic_read include/linux/instrumented.h:72 [inline]
_test_bit include/asm-generic/bitops/instrumented-non-atomic.h:141 [inline]
ida_free+0x1b9/0x400 lib/idr.c:511
mnt_release_group_id fs/namespace.c:160 [inline]
cleanup_group_ids fs/namespace.c:2093 [inline]
do_mount_setattr fs/namespace.c:4188 [inline]
__do_sys_mount_setattr fs/namespace.c:4375 [inline]
__se_sys_mount_setattr+0xc44/0x1b00 fs/namespace.c:4334
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7efc4b190919
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 11 15 00 00 90 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 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007efc4b142318 EFLAGS: 00000246 ORIG_RAX: 00000000000001ba
RAX: ffffffffffffffda RBX: 00007efc4b2183e8 RCX: 00007efc4b190919
RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000003
RBP: 00007efc4b2183e0 R08: 0000000000000020 R09: 0000000000000000
R10: 0000000020000140 R11: 0000000000000246 R12: 0030656c69662f2e
R13: 00007ffe5a122bdf R14: 00007efc4b142400 R15: 0000000000022000
</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 syzk...@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches

[Hillf Danton]

On 29 Mar 2023 17:28:55 -0700

> HEAD commit: da8e7da11e4b Merge tag 'nfsd-6.3-4' of git://git.kernel.or..
> git tree: upstream

> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12128b1ec80000

Do cleanup if group id changed.

#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git da8e7da11e4b

--- x/fs/namespace.c
+++ y/fs/namespace.c
@@ -4126,6 +4126,7 @@ static int do_mount_setattr(struct path
{
struct mount *mnt = real_mount(path->mnt);
int err = 0;
+ int changed = 0;

if (path->dentry != mnt->mnt.mnt_root)
return -EINVAL;
@@ -4146,11 +4147,14 @@ static int do_mount_setattr(struct path
*/
namespace_lock();
if (kattr->propagation == MS_SHARED) {
+ int old = mnt->mnt_group_id;
err = invent_group_ids(mnt, kattr->recurse);
if (err) {
namespace_unlock();
return err;
}
+ if (old != mnt->mnt_group_id)
+ changed = 1;
}
}

@@ -4184,7 +4188,7 @@ out:

if (kattr->propagation) {
namespace_unlock();
- if (err)
+ if (err && changed)
cleanup_group_ids(mnt, NULL);
}

--

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+8ac385...@syzkaller.appspotmail.com

Tested on:

commit: da8e7da1 Merge tag 'nfsd-6.3-4' of git://git.kernel.or..
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=139a22b9c80000

kernel config: https://syzkaller.appspot.com/x/.config?x=acdb62bf488a8fe5
dashboard link: https://syzkaller.appspot.com/bug?extid=8ac3859139c685c4f597
compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2

patch: https://syzkaller.appspot.com/x/patch.diff?x=1765c20dc80000

Note: testing is done by a robot and is best-effort only.

[Christian Brauner]

On Wed, Mar 29, 2023 at 05:28:55PM -0700, syzbot wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: da8e7da11e4b Merge tag 'nfsd-6.3-4' of git://git.kernel.or..
> git tree: upstream
> console+strace: https://syzkaller.appspot.com/x/log.txt?x=1266331ec80000
> kernel config: https://syzkaller.appspot.com/x/.config?x=acdb62bf488a8fe5
> dashboard link: https://syzkaller.appspot.com/bug?extid=8ac3859139c685c4f597
> compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11639815c80000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12128b1ec80000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/62e9c5f4bead/disk-da8e7da1.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/c11aa933e2a7/vmlinux-da8e7da1.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/7a21bdd49c84/bzImage-da8e7da1.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+8ac385...@syzkaller.appspotmail.com

This bug deserves a #include <asm-generic/bitops/ffs.h>.

In any case, it might just be advisable to hold namespace_lock() while
cleaning up peer group ids...

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git b4/vfs-mount_setattr-propagation-fix

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+8ac385...@syzkaller.appspotmail.com

Tested on:

commit: 07cd4f12 fs: drop peer group ids under namespace lock
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git b4/vfs-mount_setattr-propagation-fix
console output: https://syzkaller.appspot.com/x/log.txt?x=163d4771c80000
kernel config: https://syzkaller.appspot.com/x/.config?x=9c35b3803e5ad668

dashboard link: https://syzkaller.appspot.com/bug?extid=8ac3859139c685c4f597
compiler: Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2

Note: no patches were applied.

[Christian Brauner]

When cleaning up peer group ids in the failure path we need to make sure
to hold on to the namespace lock. Otherwise another thread might just
turn the mount from a shared into a non-shared mount concurrently.

Reported-by: syzbot+8ac385...@syzkaller.appspotmail.com
Link: https://lore.kernel.org/lkml/00000000000088...@google.com
Fixes: 2a1867219c7b ("fs: add mount_setattr()")
Cc: sta...@vger.kernel.org # 5.12+
Signed-off-by: Christian Brauner <bra...@kernel.org>
---
fs/namespace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index bc0f15257b49..6836e937ee61 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4183,9 +4183,9 @@ static int do_mount_setattr(struct path *path, struct mount_kattr *kattr)
unlock_mount_hash();

if (kattr->propagation) {
- namespace_unlock();
if (err)
cleanup_group_ids(mnt, NULL);
+ namespace_unlock();
}

return err;

---
base-commit: 197b6b60ae7bc51dd0814953c562833143b292aa
change-id: 20230330-vfs-mount_setattr-propagation-fix-363b7c59d7fb

[Christian Brauner]

On Thu, 30 Mar 2023 09:13:16 +0200, Christian Brauner wrote:
> When cleaning up peer group ids in the failure path we need to make sure
> to hold on to the namespace lock. Otherwise another thread might just
> turn the mount from a shared into a non-shared mount concurrently.
>
>

Ok, syzbot is happy with this as well so let's get this fixed and backported,

tree: git://git.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git
branch: vfs.misc.fixes
[1/1] fs: drop peer group ids under namespace lock
commit: cb2239c198ad9fbd5aced22cf93e45562da781eb