LWN: Comments on "configfd() and shifting bind mounts"

configfd() and shifting bind mounts

Cyberax — Tue, 14 Jan 2020 21:09:31 +0000

Yeah, security is a problem.

Probably at this point creating something like procfs2 and then mandating it would be the best approach. But then there's a question of what exactly is an "unsafe file"...

configfd() and shifting bind mounts

cyphar — Tue, 14 Jan 2020 21:06:58 +0000

The new mount API (in particular fsopen(2)) could work for this.

The problem is that there is a security issue with giving a program access to a /proc without any over-mounts if the /proc they already have access to has locked mounts on top of it (container runtimes use this technique to mask certain dangerous procfs files from containers). If we want to have a simple API that gives us a /proc handle, we'll need to make some kind of procfs2 (which has been suggested several times in the past) which removes all of the patently unsafe files so that untrusted programs can get access to all of it.

configfd() and shifting bind mounts

smurf — Tue, 14 Jan 2020 06:44:49 +0000

New special FD arguments to "openat()" and friends should be sufficient, no need for a new syscall.
Alternately, the new "mount" syscalls can give you a handle to /proc or /sys without actually mounting them.
Alternately, just acknowledge that not mounting /dev, /proc and /sys is not supported and going to cause problems, and leave it at that.

configfd() and shifting bind mounts

Cyberax — Mon, 13 Jan 2020 21:45:44 +0000

sysctl() was broken, it had only casual connection with /proc.

Perhaps it would be better to add a new syscall like 'open_special(fs_type)' to open '/proc', '/sys', '/sys/fs/...' directories without them being mounted.

configfd() and shifting bind mounts

mirabilos — Mon, 13 Jan 2020 21:40:57 +0000

Yeah, bad decision, that.

configfd() and shifting bind mounts

roc — Mon, 13 Jan 2020 03:34:25 +0000

... which has been removed

configfd() and shifting bind mounts

mirabilos — Sun, 12 Jan 2020 14:57:16 +0000

it’s called sysctl…

configfd() and shifting bind mounts

mirabilos — Sun, 12 Jan 2020 14:56:52 +0000

*so* a problem for /proc, which may not exist in your chroot

configfd() and shifting bind mounts

pbonzini — Sat, 11 Jan 2020 20:09:17 +0000

Thanks James, that makes sense.

configfd() and shifting bind mounts

smurf — Sat, 11 Jan 2020 17:58:20 +0000

Some more special negative pseudo-file-descriptors for openat() and friends?

configfd() and shifting bind mounts

jejb — Sat, 11 Jan 2020 16:51:06 +0000

> However, it does seem to me that passing an O_PATH file descriptor to fspick, plus a new flag for fspick that says "create a bind mount", would be a good API. The article hints that "fsconfig() is designed to work with superblocks" but it's not clear why.

I did explain that problem in the original email: all the hooks for fsconfig actions are in sb->fs_type->init_fs_context() which the fs_context allocation uses. Now it is possible to special case this for bind mounts, but you also have to special case fsmount and fsconfig/reconfigure. By the time you've done all that, you've effectively got two separate paths through the same code, which isn't really such a good idea, which is why I asked the question "what would the generalisation of fsconfig look like".

configfd() and shifting bind mounts

roc — Sat, 11 Jan 2020 01:12:16 +0000

OK, maybe it wouldn't. I have no experience with fds for paths in filesystems that aren't actually mounted anywhere.

configfd() and shifting bind mounts

quotemstr — Fri, 10 Jan 2020 23:37:02 +0000

> As a solution to the "what if procfs isn't mounted?" problem, that seems far more elegant than the alternative of creating new syscall APIs for every single feature in procfs that someone might need to use without procfs mounted

Agreed. We don't need duplicate APIs. We just need some way to get a directory FD for /proc, /sys, whatever without going through the mount table.

configfd() and shifting bind mounts

quotemstr — Fri, 10 Jan 2020 23:36:16 +0000

> which might cause issues if the filesystem is not in fact mounted anywhere,

Why would it cause problems? The actual FD would refer to a magical internal non-rooted mount, e.g., like the one the kernel sets up for pipefs on boot.

configfd() and shifting bind mounts

quotemstr — Fri, 10 Jan 2020 23:35:25 +0000

I'm not saying that we should add stuff to proc. I'm saying that putting a virtual filesystem in the rooted filesystem namespace works fine, whether that virtual filesystem is proc or something else.

configfd() and shifting bind mounts

roc — Fri, 10 Jan 2020 23:26:49 +0000

I like that idea, but rather than a directory file descriptor, which might cause issues if the filesystem is not in fact mounted anywhere, it might be better to have new open() flags or AT_ values that select specific magic filesystems --- e.g. procfs.

As a solution to the "what if procfs isn't mounted?" problem, that seems far more elegant than the alternative of creating new syscall APIs for every single feature in procfs that someone might need to use without procfs mounted. (Same goes for other magic filesystems.)

configfd() and shifting bind mounts

cyphar — Fri, 10 Jan 2020 22:41:37 +0000

Adding more things to /proc isn't a great idea (it's already full of lots of other crap that arguably shouldn't be there), and there are lots of problems with safely resolving paths in /proc. Any new kernel interfaces (*especially* ones that will be implemented through magic-links) should have a non-procfs counterpart.

configfd() and shifting bind mounts

pbonzini — Fri, 10 Jan 2020 22:38:44 +0000

> why there isn't a way to change the fd you get from fsopen of the existing filesystem into a separate filesystem with separate options for "bind"?

Bind mounts can point to any file, even one that is not a mount point--or even one that isn't a directory.

However, it does seem to me that passing an O_PATH file descriptor to fspick, plus a new flag for fspick that says "create a bind mount", would be a good API. The article hints that "fsconfig() is designed to work with superblocks" but it's not clear why.

configfd() and shifting bind mounts

quotemstr — Fri, 10 Jan 2020 22:24:20 +0000

> Because /dev or /dev/config/fs might not exist in your namespace.

Not a problem for /proc

And if it *is* a problem, the right approach isn't some random new twice on open(2), but a system call that retrieves a directory file descriptor for /dev/config or whatever, one that you could then use with openat ---

open(get_configfs_fd(), "fs/tmpfs/create", O_CLOEXEC | O_RDWR)

configfd() and shifting bind mounts

josh — Fri, 10 Jan 2020 21:53:21 +0000

Because /dev or /dev/config/fs might not exist in your namespace.

If you're binding a filesystem, though, I wonder why there isn't a way to change the fd you get from fsopen of the existing filesystem into a separate filesystem with separate options for "bind"?

configfd() and shifting bind mounts

quotemstr — Fri, 10 Jan 2020 21:30:59 +0000

I don't like the special configfd_open system call. Why not just use a regular open with a special file?

Instead of

configfd_open("tmpfs", O_CLOEXEC, CONFIGFD_CMD_CREATE)

write

open("/dev/config/fs/tmpfs/create", O_CLOEXEC | O_RDWR)