Skip to contentSkip to conversation

block: Avoid mounting the bdev pseudo-filesystem in userspace

PATCH Applied Denis Arefev 6 participants 13 messages May 21 – Jun 8

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 block: Avoid mounting the bdev pseudo-filesystem in userspace

The bdev pseudo-filesystem is an internal kernel filesystem with which userspace should not interfere. Unregister it so that userspace cannot even attempt to mount it.

This fixes a bug [1] that occurs when attempting to access files, because the system call move_mount() uses pointers declared in the inode_operations structure, which for the bdev pseudo-filesystem are always equal to 0. inode->i_op = &empty_iops;

[1]

 BUG: kernel NULL pointer dereference, address: 0000000000000000
 #PF: supervisor instruction fetch in kernel mode
 #PF: error_code(0x0010) - not-present page
 PGD 23380067 P4D 23380067 PUD 23381067 PMD 0 
 Oops: 0010 [#1] PREEMPT SMP KASAN NOPTI
 CPU: 2 PID: 17125 Comm: syz-executor.0 Not tainted 6.1.155-syzkaller-00350-g84221fde2681 #0
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
 RIP: 0010:0x0

 Call Trace:
 <TASK>
 lookup_open.isra.0+0x700/0x1180 fs/namei.c:3460
 open_last_lookups fs/namei.c:3550 [inline]
 path_openat+0x953/0x2700 fs/namei.c:3780
 do_filp_open+0x1c5/0x410 fs/namei.c:3810
 do_sys_openat2+0x171/0x4d0 fs/open.c:1318
 do_sys_open fs/open.c:1334 [inline]
 __do_sys_openat fs/open.c:1350 [inline]
 __se_sys_openat fs/open.c:1345 [inline]
 __x64_sys_openat+0x13c/0x1f0 fs/open.c:1345
 do_syscall_x64 arch/x86/entry/common.c:51 [inline]
 do_syscall_64+0x35/0x80 arch/x86/entry/common.c:81
 entry_SYSCALL_64_after_hwframe+0x6e/0xd8

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

---
 block/bdev.c | 5 -----
 1 file changed, 5 deletions(-)
block/bdev.c +0 -5
index bb0ffa3bb4df..107ac9eaac7f 100644
@@ -446,15 +446,10 @@ EXPORT_SYMBOL_GPL(blockdev_superblock);
 
 void __init bdev_cache_init(void)
 {
-	int err;
-
 	bdev_cachep = kmem_cache_create("bdev_cache", sizeof(struct bdev_inode),
 			0, (SLAB_HWCACHE_ALIGN|SLAB_RECLAIM_ACCOUNT|
 				SLAB_ACCOUNT|SLAB_PANIC),
 			init_once);
-	err = register_filesystem(&bd_type);
-	if (err)
-		panic("Cannot register bdev pseudo-fs");
 	blockdev_mnt = kern_mount(&bd_type);
 	if (IS_ERR(blockdev_mnt))
 		panic("Cannot create bdev pseudo-fs");
Signature · 1 line
2.43.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 8 lines · Denis Arefev

The bdev pseudo-filesystem is an internal kernel filesystem with which userspace should not interfere. Unregister it so that userspace cannot even attempt to mount it.

This fixes a bug [1] that occurs when attempting to access files, because the system call move_mount() uses pointers declared in the inode_operations structure, which for the bdev pseudo-filesystem are always equal to 0. inode->i_op = &empty_iops;

Looks good:

maintainer
#2 lore raw
Quoted 11 lines · Christoph Hellwig
Quoted 8 lines · Denis Arefev

The bdev pseudo-filesystem is an internal kernel filesystem with which userspace should not interfere. Unregister it so that userspace cannot even attempt to mount it.

This fixes a bug [1] that occurs when attempting to access files, because the system call move_mount() uses pointers declared in the inode_operations structure, which for the bdev pseudo-filesystem are always equal to 0. inode->i_op = &empty_iops;

Looks good:

It really, really does not. I would like to see the reproducer - analysis looks like random noise out of LLM.

I've no real problem with removing that register_filesystem(), but if it *does* fix some reproducer, I really want to see details.

maintainer Applied
#3 lore raw
In reply to Denis Arefev
Quoted 10 lines · Denis Arefev

The bdev pseudo-filesystem is an internal kernel filesystem with which userspace should not interfere. Unregister it so that userspace cannot even attempt to mount it.

This fixes a bug [1] that occurs when attempting to access files, because the system call move_mount() uses pointers declared in the inode_operations structure, which for the bdev pseudo-filesystem are always equal to 0. inode->i_op = &empty_iops;

[...]

Applied, thanks!

[1/1] block: Avoid mounting the bdev pseudo-filesystem in userspace

      commit: b518ae170f6c411cac2d5f320278c27d902bc628

Best regards,

Signature · 1 line
Jens Axboe
maintainer
#4 lore raw
In reply to Denis Arefev
Quoted 8 lines · Denis Arefev

The bdev pseudo-filesystem is an internal kernel filesystem with which userspace should not interfere. Unregister it so that userspace cannot even attempt to mount it.

This fixes a bug [1] that occurs when attempting to access files, because the system call move_mount() uses pointers declared in the inode_operations structure, which for the bdev pseudo-filesystem are always equal to 0. inode->i_op = &empty_iops;

What? init_pseudo() sets SB_NOUSER; what are you talking about? And assuming you've somehow managed to mount the sucker, which ->i_op method had been accessed?

maintainer
#5 lore raw
Quoted 11 lines · Al Viro
Quoted 8 lines · Denis Arefev

The bdev pseudo-filesystem is an internal kernel filesystem with which userspace should not interfere. Unregister it so that userspace cannot even attempt to mount it.

This fixes a bug [1] that occurs when attempting to access files, because the system call move_mount() uses pointers declared in the inode_operations structure, which for the bdev pseudo-filesystem are always equal to 0. inode->i_op = &empty_iops;

What? init_pseudo() sets SB_NOUSER; what are you talking about?

... which doesn't suffice, apparently, since now bdev has become mountable, along with the rest of pseudo-fs. *THAT* is a bug.

Quoted 2 lines

And assuming you've somehow managed to mount the sucker, which ->i_op method had been accessed?

->lookup(), apparently. Which means that 'directory' should've been rejected by d_can_lookup(), no matter which filesystem it's been from. Which might or might not be a bug in its own right.

In any case, NAK on that patch - it's papering over the real bug that has nothing to do with block layer.

mount -t bdev none /mnt

must fail, same as for pipefs, sockfs, etc. It doesn't.

fsdevel Cc'd, as it should've been from the very beginning.

maintainer
#6 lore raw

PATCH 1/1 make new mount API honour SB_NOUSER (was Re: [PATCH] block: Avoid mounting the bdev pseudo-filesystem in userspace)

one should *not* be allowed to mount one of those, new API or not.

---
[[ I still want to see the rest of the reproducer - report smells like a missing d_can_lookup() somewhere, on top of fsmount(2) bug]]

fs/namespace.c +4 -0
index fe919abd2f01..17777c837683 100644
@@ -4499,6 +4499,10 @@ SYSCALL_DEFINE3(fsmount, int, fs_fd, unsigned int, flags,
 	new_mnt = vfs_create_mount(fc);
 	if (IS_ERR(new_mnt))
 		return PTR_ERR(new_mnt);
+	if (new_mnt->mnt_sb->s_flags & SB_NOUSER) {
+		mntput(new_mnt);
+		return -EINVAL;
+	}
 	new_mnt->mnt_flags = mnt_flags;
 
 	new_path.dentry = dget(fc->root);
reviewer
#7 lore raw

Re: [PATCH] make new mount API honour SB_NOUSER (was Re: [PATCH] block: Avoid mounting the bdev pseudo-filesystem in userspace)

Quoted 4 lines · Al Viro

one should *not* be allowed to mount one of those, new API or not.

Won't it make sense to actually check fc->sb_flags before we call vfs_create_mount()? Otherwise it looks good to me.

Honza

Quoted 18 lines

---
[[ I still want to see the rest of the reproducer - report smells like a missing d_can_lookup() somewhere, on top of fsmount(2) bug]]

fs/namespace.c +4 -0
index fe919abd2f01..17777c837683 100644
@@ -4499,6 +4499,10 @@ SYSCALL_DEFINE3(fsmount, int, fs_fd, unsigned int, flags,
 	new_mnt = vfs_create_mount(fc);
 	if (IS_ERR(new_mnt))
 		return PTR_ERR(new_mnt);
+	if (new_mnt->mnt_sb->s_flags & SB_NOUSER) {
+		mntput(new_mnt);
+		return -EINVAL;
+	}
 	new_mnt->mnt_flags = mnt_flags;
 
 	new_path.dentry = dget(fc->root);
Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
author
#8 lore raw

Re: [PATCH] make new mount API honour SB_NOUSER (was Re: [PATCH] block: Avoid mounting the bdev pseudo-filesystem in userspace)

02.06.2026 12:11, Jan Kara пишет:

Quoted 9 lines · Al Viro
Quoted 4 lines

one should *not* be allowed to mount one of those, new API or not.

Won't it make sense to actually check fc->sb_flags before we call vfs_create_mount()? Otherwise it looks good to me.

Honza

Hi all.

The sequence of system calls before the crash could be as follows:

fsopen("bdev", ...)
fsconfig(fd_fs, FSCONFIG_CMD_CREATE, 0,0,0) fsmount(fd_fs, 0,0)
move_mount(fd_mnt, "", AT_FDCWD, "./file1", 0x46ul)

The system call executed at the time of the cras:

open("/dev/media0", ...);

Simplified stacktrace:

path_openat
|-> link_path_walk
   |-> walk_component
      |-> __lookup_slow
         |-> ld = inode->i_op->lookup(inode, dentry, flags);   <- Oops

Searching for possible solutions in the commit history yielded the following result:

commit fd3e007f6c6a0f677e4ee8aca4b9bab8ad6cab9a
commit 1a6e9e76b713d9632783efe78295ed3507fdad64
commit d6f2589ad561aa5fa39f347eca6942668b7560a1

Checking the fc->sb_flags flag before calling vfs_create_mount() is a great idea, if it helps prevent crashes in two more file systems, 'sockfs' and 'pipefs'.

Best regards, Denis.

Quoted 19 lines
Quoted 19 lines

---
[[ I still want to see the rest of the reproducer - report smells like a missing d_can_lookup() somewhere, on top of fsmount(2) bug]]

fs/namespace.c +4 -0
index fe919abd2f01..17777c837683 100644
@@ -4499,6 +4499,10 @@ SYSCALL_DEFINE3(fsmount, int, fs_fd, unsigned int, flags,
  	new_mnt = vfs_create_mount(fc);
  	if (IS_ERR(new_mnt))
  		return PTR_ERR(new_mnt);
+	if (new_mnt->mnt_sb->s_flags & SB_NOUSER) {
+		mntput(new_mnt);
+		return -EINVAL;
+	}
  	new_mnt->mnt_flags = mnt_flags;
 
  	new_path.dentry = dget(fc->root);
maintainer
#9 lore raw

Re: [PATCH] make new mount API honour SB_NOUSER (was Re: [PATCH] block: Avoid mounting the bdev pseudo-filesystem in userspace)

Quoted 6 lines · Arefev

The sequence of system calls before the crash could be as follows:

fsopen("bdev", ...)
fsconfig(fd_fs, FSCONFIG_CMD_CREATE, 0,0,0) fsmount(fd_fs, 0,0)
move_mount(fd_mnt, "", AT_FDCWD, "./file1", 0x46ul)

Huh? "file1" being a regular file or was it actually
a directory? AFAICS, the d_is_dir() mismatch would be rejected by do_move_mount()...

Quoted 11 lines

The system call executed at the time of the cras:

open("/dev/media0", ...);

Simplified stacktrace:

path_openat
|-> link_path_walk
   |-> walk_component
      |-> __lookup_slow
         |-> ld = inode->i_op->lookup(inode, dentry, flags);   <- Oops

How the hell does that thing bound on top of "./file1" lead to resolution of "/dev/media0" walking anywhere near it? Something's missing here.

Quoted 3 lines

Checking the fc->sb_flags flag before calling vfs_create_mount() is a great idea,
if it helps prevent crashes in two more file systems, 'sockfs' and 'pipefs'.

Calling vfs_create_mount() is not a problem; refusing to attach the result if SB_NOUSER has ended up in ->s_flags is the right thing to do, but I still would like to understand how did this call of walk_component() manage to evade

                if (unlikely(!d_can_lookup(nd->path.dentry))) {
			if (nd->flags & LOOKUP_RCU) {
				if (!try_to_unlazy(nd))
					return -ECHILD;
			}
			return -ENOTDIR;
		}

on the previous iteration through link_path_walk() or, if it had been the first one, the corresponding checks at chroot()/chdir()/fchdir() time.

Note that there are very legitimate objects with NULL ->lookup() - every regular file is like that, obviously, but there also exist ones that look like directories in mode bits, but still have NULL ->lookup(). See d_flags_for_inode() and look for DCACHE_AUTODIR_TYPE there.

So whatever scenario has played out, you've got a call of walk_component() with nd->path.dentry that should have failed d_can_lookup(). That ought to have been prevented and this prevention would better be much closer than anything fsmount(2) does.

Don't get me wrong - userland mounting of bdev and friends should not be allowed, but that's not the only thing that went wrong in the reproducer. BTW, how easy to trigger it is? Is that "you need to run for a few months on a bunch of boxen" or "run this sequence and it'll crash that way"?

maintainer
#10 lore raw

Re: [PATCH] make new mount API honour SB_NOUSER (was Re: [PATCH] block: Avoid mounting the bdev pseudo-filesystem in userspace)

In reply to Jan Kara
Quoted 8 lines · Jan Kara
Quoted 4 lines · Al Viro

one should *not* be allowed to mount one of those, new API or not.

Won't it make sense to actually check fc->sb_flags before we call vfs_create_mount()? Otherwise it looks good to me.

Interpretation of fc->sb_flags is up to your ->get_tree(). What matters is ->s_flags in the resulting superblock; that's type-independent and that's what we ought to check...

reviewer
#11 lore raw

Re: [PATCH] make new mount API honour SB_NOUSER (was Re: [PATCH] block: Avoid mounting the bdev pseudo-filesystem in userspace)

Quoted 7 lines · Al Viro
Quoted 6 lines · Jan Kara
Quoted 4 lines · Al Viro

one should *not* be allowed to mount one of those, new API or not.

Quoted 6 lines
Quoted 2 lines

Won't it make sense to actually check fc->sb_flags before we call vfs_create_mount()? Otherwise it looks good to me.

Interpretation of fc->sb_flags is up to your ->get_tree(). What matters is ->s_flags in the resulting superblock; that's type-independent and that's what we ought to check...

Ah, right. Thanks for explanation!

Honza

Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
maintainer Applied
#12 lore raw

Re: [PATCH] make new mount API honour SB_NOUSER (was Re: [PATCH] block: Avoid mounting the bdev pseudo-filesystem in userspace)

In reply to Al Viro
Quoted 1 line · Al Viro

one should *not* be allowed to mount one of those, new API or not.

Applied to the vfs-7.2.misc branch of the vfs/vfs.git tree. Patches in the vfs-7.2.misc branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase, trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs-7.2.misc

[1/1] mount: honour SB_NOUSER in the new mount API

      https://git.kernel.org/vfs/vfs/c/67d8c452fae1