[PATCH 0/7] proc: block mounts below /proc/<pid>/net and on the links into /proc/self

[PATCH 0/7] proc: block mounts below /proc/<pid>/net and on the links into /proc/self

[Christian Brauner]

In Linux 6.12 I stopped allowing mounts on the entries below
/proc/<pid>/fd/, /proc/<pid>/fdinfo/ and /proc/<pid>/map_files/ (cf.
[1]). This extends that to two more groups of entries:

(1) Everything below /proc/<pid>/net/.

    proc_net_d_revalidate() fails on every lookup since 1fde6f21d90f
    ("proc: fix /proc/net/* after setns(2)"). lookup_fast() answers that
    with d_invalidate() which detaches every mount on the dentry. That
    affects every mount namespace and includes locked mounts.

    So a mask on /proc/<pid>/net/dev goes away the first time anybody
    looks the name up. The directory is 0555 so anybody really means
    anybody. This is reproducible with a container that masks
    /proc/<pid>/net/dev with a locked bind mount and a stat() by nobody
    in the initial namespaces makes the real file show up again. So it's
    pointless.

    A mount that can't stick is better refused than taken away. I also
    have a version that makes proc_net_d_revalidate() check the network
    namespace instead so mounts there stick. But I think that's
    bullsh*t. It would make mounts on the net entries of other processes
    stick as well and the few users that try to mount there fail today
    anyway (see below).

(2) The links into /proc/self: /proc/self, /proc/thread-self,
    /proc/mounts, /proc/net and the afs and nfs links below /proc/fs/.

    These resolve to a different directory for every process. A mount
    on the link itself is crossed by every lookup through it in that
    mount namespace, so it redirects all processes at once. Mounting
    through the links keeps working and lands on the caller's own entry
    as before.

What I really wanted was to block mounting on anything below
/proc/<pid>/. That was the big hammer I mentioned in [1] years ago. We
can't... I did a pretty exhaustive survey of who mounts on the entries
below /proc/<pid>/ or on the links into it:

- GitHub code search and regex searches on Sourcegraph
- Debian Code Search over unstable
- the systemd units of every binary package in Debian 11, 12, 13 and
  sid, Ubuntu 22.04, 24.04 and 26.04, Fedora 43 and 44, CentOS Stream
  10, openSUSE Leap 16 and Tumbleweed and Arch
- clones of 46 container runtimes and orchestrators, the sandboxes
  (systemd, bubblewrap, flatpak, firejail, snapd, nsjail, minijail,
  Chromium and Firefox), Android and the rooting tools on top of it,
  and the AppArmor and SELinux policies
- detection rules, issue trackers, lore, LWN and the Q&A sites

The legitimate users all mount on entries of the process that does the
mount:

(1) systemd units that list /proc/self/... in ReadOnlyPaths=,
    ReadWritePaths= or ReadWriteDirectories=. systemd resolves
    /proc/self in the child of the service and binds the entry onto
    itself before it execs the service. If the mount fails the unit
    fails with 226/NAMESPACE and the "-" prefix doesn't help. That's
    netdata in Debian 11 and 12 and Ubuntu 22.04 and 24.04, opendht in
    Debian 12 and sid, Ubuntu 24.04 and Arch, accountsservice 22.x in
    Debian 12 and Ubuntu 22.04, OpenSearch 3, wazuh-indexer, TrueNAS
    SCALE, OONI's backend and the JupyterHub systemdspawner example. Of
    the 731 unit files on GitHub that name a /proc path in one of these
    directives 53 name an entry of the service itself.

(2) systemd's make_filesystem() binds /dev/null over /proc/self/mounts
    before it execs mkfs.btrfs so that it doesn't refuse a device whose
    other partitions are mounted. systemd-repart, systemd-homed and
    systemd-makefs use that.

(3) mkosi binds a fake /proc/self/mountinfo for grub-bios-setup.

(4) runc applies maskedPaths such as /proc/1/environ from the
    container's init when a config asks for it, bwrap does the same
    from its pid 1, firejail hides the name of its pid 1 and clampdown
    masks /proc/1/mem.

(5) Shell scripts that mount on /proc/$$/... and then exec, e.g.,
    luci-app-shadowrt on OpenWrt.

(6) Tests in systemd, elogind, LXC, libpathrs and bun.

Fedora's SELinux policy even grants fsadm_t and init_t the permission to
mount over their own entries because of (1) and (2). Sad, very sad...

Everything else I found is malware or offensive tooling:

Hiding a process by mounting over /proc/<pid> is catalogued as MITRE
ATT&CK T1564.013 and used in the wild by TeamTNT, Commando Cat, the KONO
DIO DA miner, KV-botnet, UNC2891 and kbotne. hackshell and emp3r0r ship
it and Elastic's endpoint rule kills any process that does it.

So blocking all of /proc/<pid>/ would break units in released
distributions and systemd itself. A rule that only lets a process
mount on its own entries, and on a directory only a bind of itself,
keeps (1) to (4) working, except for a few bwrap sandboxes that put a
tmpfs over directories like /proc/1/task, and stops all of the attacks.
I have that as well but it's not really a fix so we can delay it.

The remaining entries of a process only fail pid_revalidate() once the
task is gone and proc_flush_pid() invalidates them when it is reaped.
Their mounts go away together with the task and nothing gets revealed.

Nothing I found mounts on the links themselves and crun even has a
test that expects it to fail. The handful of units that mask entries
below /proc/<pid>/net/, e.g., Algo VPN's ReadOnlyPaths=/proc/net/pfkey
for strongswan, already fail today.

systemd's own lookup invalidates the dentry before it can bind onto it.
With this series they fail the same way. Apart from tests in libpathrs
and securejoin that had to be adapted the only fallout from the 6.12
change we found is one small project that handed libmount's new mount
api a /proc/self/fd/N target that move_mount() doesn't follow without
MOVE_MOUNT_T_SYMLINKS. Ubuntu even backported the 6.12 change to their
22.04 and 24.04 kernels this year.

Nuke this crap.

[1]: https://lore.kernel.org/r/20240806-work-procfs-v1-0-fb04e1d09f0c@kernel.org

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
Christian Brauner (7):
      proc: don't let an entry below /proc/<pid>/net be a mountpoint
      proc: don't let /proc/self and /proc/thread-self be mountpoints
      proc: don't let /proc/mounts and /proc/net be mountpoints
      afs: create /proc/fs/afs with proc_self_symlink()
      nfs: create the /proc/fs/nfsfs links with proc_self_symlink()
      proc: explain which entries can't be mountpoints
      selftests/proc: test which entries can't be mountpoints

 fs/afs/main.c                                   |   2 +-
 fs/nfs/client.c                                 |   4 +-
 fs/proc/generic.c                               |  31 ++++-
 fs/proc/internal.h                              |  18 ++-
 fs/proc/proc_net.c                              |   2 +-
 fs/proc/root.c                                  |   2 +-
 fs/proc/self.c                                  |   2 +
 fs/proc/thread_self.c                           |   2 +
 include/linux/proc_fs.h                         |   5 +
 tools/testing/selftests/proc/.gitignore         |   1 +
 tools/testing/selftests/proc/Makefile           |   1 +
 tools/testing/selftests/proc/proc-unmountable.c | 150 ++++++++++++++++++++++++
 12 files changed, 208 insertions(+), 12 deletions(-)
---
base-commit: 7be4bc52fd9e0e55dc9a2e0232b89427d2c4b614
change-id: 20261024-work-procfs-fixes-8b21a6d17448

[PATCH 1/7] proc: don't let an entry below /proc/<pid>/net be a mountpoint

[Christian Brauner]

proc_net_d_revalidate() always returns 0 so that a dentry below
/proc/<pid>/net is never reused after setns(). So lookup_fast() will end
up calling d_invalidate(), which unhashes the dentry and detaches every
mount on it, in every mount namespace; including locked mounts.

Then lookup_slow() creates a new dentry for the same entry. So a mount
on /proc/<pid>/net/dev succeeds and goes away with the first lookup of
the name by anybody with search permission on the directory. Given it's
0555 that's everyone's grandma. A mask on such a name hides nothing as
the repro below shows.

Treat those entries like I did fd/, fdinfo/ and map_files/ and refuse to
mount on them in the first place. It's nonsensical theater otherwise:

  procnet net/dev stat

  // SPDX-License-Identifier: GPL-2.0
  /*
   * procnet: cover an entry of /proc/<pid>/ with a bind mount, lock the mount,
   * then look the name up.
   *
   * The first task stays alive, <pid> is its pid. The "container" task has its
   * own mount namespace. There it binds a mask on /proc/<pid>/<name>. It drops to
   * uid 1000 and enters a new user namespace with a new mount namespace, so that
   * the mask is locked. Then one task looks the name up:
   *
   *   stat    the container calls stat() on the name
   *   umount  the container calls umount2(MNT_DETACH) on the name
   *   nobody  a task with uid 65534 in the first namespaces calls stat()
   *
   * The container lists its mounts before and after (that does not look the
   * name up) and then reads the name. MASK is fine. Anything else means that
   * the lookup uncovered the entry.
   *
   * usage: procnet <name> stat|umount|nobody
   *   name "status" is the control, "net/dev" is a file below net, "net/stat" is
   *   a directory below net
   */
  #define _GNU_SOURCE
  #include <dirent.h>
  #include <errno.h>
  #include <fcntl.h>
  #include <sched.h>
  #include <stdio.h>
  #include <stdlib.h>
  #include <string.h>
  #include <time.h>
  #include <unistd.h>
  #include <sys/mount.h>
  #include <sys/prctl.h>
  #include <sys/stat.h>
  #include <sys/wait.h>

  #define WORK "/run/pn"

  static char target[128];

  static void die(const char *what)
  {
  	fprintf(stderr, "FATAL %s: %s\n", what, strerror(errno));
  	exit(2);
  }

  static void write_str(const char *path, const char *s, int flags)
  {
  	int fd = open(path, O_WRONLY | flags, 0644);

  	if (fd < 0 || write(fd, s, strlen(s)) < 0)
  		die(path);
  	close(fd);
  }

  /* list the mounts below /proc without a lookup of the target */
  static void mounts(const char *when)
  {
  	char line[512];
  	FILE *f = fopen("/proc/self/mountinfo", "r");
  	int n = 0;

  	if (!f)
  		die("mountinfo");
  	while (fgets(line, sizeof(line), f)) {
  		if (!strstr(line, " /proc/"))
  			continue;
  		printf("container: %s: mount: %s", when, line);
  		n++;
  	}
  	fclose(f);
  	if (!n)
  		printf("container: %s: nothing is mounted below /proc\n", when);
  }

  static void show(const char *when, int isdir)
  {
  	char buf[48] = "";
  	int fd;

  	if (isdir) {
  		struct dirent *de;
  		DIR *d = opendir(target);
  		int hit = 0;

  		if (!d) {
  			printf("container: %s: opendir(%s) = %s\n", when, target, strerror(errno));
  			return;
  		}
  		printf("container: %s: readdir(%s) =", when, target);
  		while ((de = readdir(d))) {
  			if (de->d_name[0] == '.')
  				continue;
  			printf(" %s", de->d_name);
  			hit |= strcmp(de->d_name, "MASK");
  		}
  		closedir(d);
  		printf("%s\n", hit ? "   <-- HIT" : "");
  		return;
  	}
  	fd = open(target, O_RDONLY);
  	if (fd < 0) {
  		printf("container: %s: open(%s) = %s\n", when, target, strerror(errno));
  		return;
  	}
  	if (read(fd, buf, sizeof(buf) - 1) < 0)
  		die("read");
  	close(fd);
  	if (strchr(buf, '\n'))
  		*strchr(buf, '\n') = 0;
  	printf("container: %s: read(%s) = \"%s\"%s\n", when, target, buf,
  	       strncmp(buf, "MASK", 4) ? "   <-- HIT" : "");
  }

  int main(int argc, char **argv)
  {
  	const char *name = argc > 1 ? argv[1] : "net/dev";
  	const char *op = argc > 2 ? argv[2] : "stat";
  	int isdir = !strcmp(name, "net/stat");
  	int to_host[2], to_cont[2];
  	struct stat st;
  	pid_t pid;
  	char c;

  	setvbuf(stdout, NULL, _IOLBF, 0);
  	snprintf(target, sizeof(target), "/proc/%d/%s", getpid(), name);
  	if (unshare(CLONE_NEWNS) || mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL))
  		die("unshare");
  	mkdir(WORK, 0755);
  	if (mount("W", WORK, "tmpfs", 0, NULL))
  		die("mount W");
  	write_str(WORK "/mask", "MASK", O_CREAT);
  	mkdir(WORK "/maskdir", 0755);
  	write_str(WORK "/maskdir/MASK", "MASK", O_CREAT);
  	if (pipe(to_host) || pipe(to_cont))
  		die("pipe");

  	pid = fork();
  	if (pid < 0)
  		die("fork");
  	if (!pid) {
  		if (unshare(CLONE_NEWNS))
  			die("container unshare");
  		if (mount(isdir ? WORK "/maskdir" : WORK "/mask", target, NULL, MS_BIND, NULL)) {
  			printf("container: bind the mask on %s: %s\n", target, strerror(errno));
  			exit(0);
  		}
  		printf("container: bind the mask on %s: ok\n", target);
  		if (setgid(1000) || setuid(1000) || prctl(PR_SET_DUMPABLE, 1))
  			die("setuid");
  		/* lock the mount: a new user namespace with a copy of the tree */
  		if (unshare(CLONE_NEWUSER | CLONE_NEWNS))
  			die("container userns");
  		write_str("/proc/self/setgroups", "deny", 0);
  		write_str("/proc/self/uid_map", "0 1000 1", 0);
  		write_str("/proc/self/gid_map", "0 1000 1", 0);
  		mounts("before");
  		errno = 0;
  		if (!strcmp(op, "stat")) {
  			stat(target, &st);
  			printf("container: stat(%s): %s\n", target, strerror(errno));
  		} else if (!strcmp(op, "umount")) {
  			umount2(target, MNT_DETACH);
  			printf("container: umount2(%s, MNT_DETACH): %s\n", target, strerror(errno));
  		} else if (write(to_host[1], "g", 1) != 1 || read(to_cont[0], &c, 1) != 1) {
  			die("sync");
  		}
  		mounts("after");
  		show("after", isdir);
  		exit(0);
  	}
  	close(to_host[1]);
  	if (read(to_host[0], &c, 1) == 1) {
  		pid_t p2 = fork();

  		if (!p2) {
  			if (setgid(65534) || setuid(65534))
  				die("setuid");
  			errno = 0;
  			stat(target, &st);
  			printf("nobody: uid %u: stat(%s): %s\n", getuid(), target, strerror(errno));
  			exit(0);
  		}
  		waitpid(p2, NULL, 0);
  		if (write(to_cont[1], "g", 1) != 1)
  			die("sync");
  	}
  	waitpid(pid, NULL, 0);
  	return 0;
  }

Fixes: 1fde6f21d90f ("proc: fix /proc/net/* after setns(2)")
Cc: <stable@vger.kernel.org>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/proc/generic.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 2b1971da4a85..4c3fcaf68ef4 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -256,9 +256,10 @@ struct dentry *proc_lookup_de(struct inode *dir, struct dentry *dentry,
 		inode = proc_get_inode(dir->i_sb, de);
 		if (!inode)
 			return ERR_PTR(-ENOMEM);
+		/* never reused and so never a mountpoint for long */
 		if (de->flags & PROC_ENTRY_FORCE_LOOKUP)
-			return d_splice_alias_ops(inode, dentry,
-						  &proc_net_dentry_ops);
+			return proc_splice_unmountable(inode, dentry,
+						       &proc_net_dentry_ops);
 		return d_splice_alias_ops(inode, dentry,
 					  &proc_misc_dentry_ops);
 	}

-- 
2.53.0

[PATCH 2/7] proc: don't let /proc/self and /proc/thread-self be mountpoints

[Christian Brauner]

/proc/self and /proc/thread-self resolve to a different directory for
every process that walks through them. A mount on one of the links
itself is crossed by every lookup of /proc/self/... in that mount
namespace, so it redirects all processes at once, not just the one that
created the mount. There's no good reason to ever do that.

Mark both links with dont_mount() before they become visible.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/proc/self.c        | 2 ++
 fs/proc/thread_self.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/fs/proc/self.c b/fs/proc/self.c
index 56adf1c68f7a..63120ab7704c 100644
--- a/fs/proc/self.c
+++ b/fs/proc/self.c
@@ -48,6 +48,8 @@ int proc_setup_self(struct super_block *s)
 			inode->i_uid = GLOBAL_ROOT_UID;
 			inode->i_gid = GLOBAL_ROOT_GID;
 			inode->i_op = &proc_self_inode_operations;
+			/* a mount here would redirect every process */
+			dont_mount(self);
 			d_make_persistent(self, inode);
 			ret = 0;
 		}
diff --git a/fs/proc/thread_self.c b/fs/proc/thread_self.c
index 61ac62c3fd9f..a4f803daf589 100644
--- a/fs/proc/thread_self.c
+++ b/fs/proc/thread_self.c
@@ -48,6 +48,8 @@ int proc_setup_thread_self(struct super_block *s)
 			inode->i_uid = GLOBAL_ROOT_UID;
 			inode->i_gid = GLOBAL_ROOT_GID;
 			inode->i_op = &proc_thread_self_inode_operations;
+			/* a mount here would redirect every thread */
+			dont_mount(thread_self);
 			d_make_persistent(thread_self, inode);
 			ret = 0;
 		}

-- 
2.53.0

[PATCH 3/7] proc: don't let /proc/mounts and /proc/net be mountpoints

[Christian Brauner]

/proc/mounts and /proc/net are links into /proc/self. Like /proc/self
itself they resolve differently for every process. A mount on one of the
links redirects the lookup for all processes in that mount namespace.

Add proc_self_symlink() for links that point into /proc/self. It creates
the link with PROC_ENTRY_CANT_MOUNT and proc_lookup_de() splices such an
entry with proc_splice_unmountable(). Use it for /proc/mounts and
/proc/net.

Mounting through the links keeps working. Such a mount lands on the
caller's own /proc/<pid>/mounts or /proc/<pid>/net, not on the link.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/proc/generic.c       | 26 ++++++++++++++++++++++++--
 fs/proc/proc_net.c      |  2 +-
 fs/proc/root.c          |  2 +-
 include/linux/proc_fs.h |  5 +++++
 4 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/fs/proc/generic.c b/fs/proc/generic.c
index 4c3fcaf68ef4..484d7dc123eb 100644
--- a/fs/proc/generic.c
+++ b/fs/proc/generic.c
@@ -260,6 +260,10 @@ struct dentry *proc_lookup_de(struct inode *dir, struct dentry *dentry,
 		if (de->flags & PROC_ENTRY_FORCE_LOOKUP)
 			return proc_splice_unmountable(inode, dentry,
 						       &proc_net_dentry_ops);
+		/* a mount on a link into /proc/self redirects every process */
+		if (de->flags & PROC_ENTRY_CANT_MOUNT)
+			return proc_splice_unmountable(inode, dentry,
+						       &proc_misc_dentry_ops);
 		return d_splice_alias_ops(inode, dentry,
 					  &proc_misc_dentry_ops);
 	}
@@ -487,8 +491,8 @@ static struct proc_dir_entry *__proc_create(struct proc_dir_entry **parent,
 	return ent;
 }
 
-struct proc_dir_entry *proc_symlink(const char *name,
-		struct proc_dir_entry *parent, const char *dest)
+static struct proc_dir_entry *__proc_symlink(const char *name,
+		struct proc_dir_entry *parent, const char *dest, u8 flags)
 {
 	struct proc_dir_entry *ent;
 
@@ -496,6 +500,7 @@ struct proc_dir_entry *proc_symlink(const char *name,
 			  (S_IFLNK | S_IRUGO | S_IWUGO | S_IXUGO),1);
 
 	if (ent) {
+		ent->flags |= flags;
 		ent->size = strlen(dest);
 		ent->data = kmemdup(dest, ent->size + 1, GFP_KERNEL);
 		if (ent->data) {
@@ -508,8 +513,25 @@ struct proc_dir_entry *proc_symlink(const char *name,
 	}
 	return ent;
 }
+
+struct proc_dir_entry *proc_symlink(const char *name,
+		struct proc_dir_entry *parent, const char *dest)
+{
+	return __proc_symlink(name, parent, dest, 0);
+}
 EXPORT_SYMBOL(proc_symlink);
 
+/*
+ * A link into /proc/self resolves differently for every process, so a
+ * mount on the link itself would redirect all of them.
+ */
+struct proc_dir_entry *proc_self_symlink(const char *name,
+		struct proc_dir_entry *parent, const char *dest)
+{
+	return __proc_symlink(name, parent, dest, PROC_ENTRY_CANT_MOUNT);
+}
+EXPORT_SYMBOL_GPL(proc_self_symlink);
+
 struct proc_dir_entry *_proc_mkdir(const char *name, umode_t mode,
 		struct proc_dir_entry *parent, void *data, bool force_lookup)
 {
diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index b1f5eafb069a..578b1eb081bf 100644
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -420,7 +420,7 @@ static struct pernet_operations __net_initdata proc_net_ns_ops = {
 
 int __init proc_net_init(void)
 {
-	proc_symlink("net", NULL, "self/net");
+	proc_self_symlink("net", NULL, "self/net");
 
 	return register_pernet_subsys(&proc_net_ns_ops);
 }
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 7fbbe92bf73a..42fdc6ec50a3 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -380,7 +380,7 @@ void __init proc_root_init(void)
 	set_proc_pid_nlink();
 	proc_self_init();
 	proc_thread_self_init();
-	proc_symlink("mounts", NULL, "self/mounts");
+	proc_self_symlink("mounts", NULL, "self/mounts");
 
 	proc_net_init();
 	proc_mkdir("fs", NULL);
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 47d7deaeed8f..903fc811efd4 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -29,6 +29,7 @@ enum {
 	PROC_ENTRY_proc_compat_ioctl	= 1U << 2,
 	PROC_ENTRY_proc_lseek		= 1U << 3,
 
+	PROC_ENTRY_CANT_MOUNT		= 1U << 6,
 	PROC_ENTRY_FORCE_LOOKUP		= 1U << 7,
 };
 
@@ -87,6 +88,8 @@ extern void proc_flush_pid(struct pid *);
 
 extern struct proc_dir_entry *proc_symlink(const char *,
 		struct proc_dir_entry *, const char *);
+struct proc_dir_entry *proc_self_symlink(const char *, struct proc_dir_entry *,
+					 const char *);
 struct proc_dir_entry *_proc_mkdir(const char *, umode_t, struct proc_dir_entry *, void *, bool);
 extern struct proc_dir_entry *proc_mkdir(const char *, struct proc_dir_entry *);
 extern struct proc_dir_entry *proc_mkdir_data(const char *, umode_t,
@@ -179,6 +182,8 @@ static inline void proc_flush_pid(struct pid *pid)
 
 static inline struct proc_dir_entry *proc_symlink(const char *name,
 		struct proc_dir_entry *parent,const char *dest) { return NULL;}
+static inline struct proc_dir_entry *proc_self_symlink(const char *name,
+		struct proc_dir_entry *parent, const char *dest) { return NULL; }
 static inline struct proc_dir_entry *proc_mkdir(const char *name,
 	struct proc_dir_entry *parent) {return NULL;}
 static inline struct proc_dir_entry *proc_create_mount_point(const char *name) { return NULL; }

-- 
2.53.0

[PATCH 4/7] afs: create /proc/fs/afs with proc_self_symlink()

[Christian Brauner]

/proc/fs/afs points into /proc/self/net. Create it with
proc_self_symlink() so nothing can be mounted on the link itself.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/afs/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/afs/main.c b/fs/afs/main.c
index 7a883c59976f..b12725f1d3d5 100644
--- a/fs/afs/main.c
+++ b/fs/afs/main.c
@@ -188,7 +188,7 @@ static int __init afs_init(void)
 	if (ret < 0)
 		goto error_fs;
 
-	afs_proc_symlink = proc_symlink("fs/afs", NULL, "../self/net/afs");
+	afs_proc_symlink = proc_self_symlink("fs/afs", NULL, "../self/net/afs");
 	if (!afs_proc_symlink) {
 		ret = -ENOMEM;
 		goto error_proc;

-- 
2.53.0

[PATCH 5/7] nfs: create the /proc/fs/nfsfs links with proc_self_symlink()

[Christian Brauner]

/proc/fs/nfsfs/servers and /proc/fs/nfsfs/volumes resolve through
/proc/net into /proc/self/net. Create them with proc_self_symlink() so
nothing can be mounted on the links themselves.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/nfs/client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 60386330aeec..97e363ac7a3e 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1528,11 +1528,11 @@ int __init nfs_fs_proc_init(void)
 		goto error_0;
 
 	/* a file of servers with which we're dealing */
-	if (!proc_symlink("fs/nfsfs/servers", NULL, "../../net/nfsfs/servers"))
+	if (!proc_self_symlink("fs/nfsfs/servers", NULL, "../../net/nfsfs/servers"))
 		goto error_1;
 
 	/* a file of volumes that we have mounted */
-	if (!proc_symlink("fs/nfsfs/volumes", NULL, "../../net/nfsfs/volumes"))
+	if (!proc_self_symlink("fs/nfsfs/volumes", NULL, "../../net/nfsfs/volumes"))
 		goto error_1;
 
 	return 0;

-- 
2.53.0

[PATCH 6/7] proc: explain which entries can't be mountpoints

[Christian Brauner]

Document how a failing ->d_revalidate() takes the mounts off a procfs
entry, which entries can't be mountpoints because of that and why the
other entries of a process are fine.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/proc/internal.h | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index b9aaac41c283..296ea13bbd49 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -425,9 +425,21 @@ static inline void pde_force_lookup(struct proc_dir_entry *pde)
 }
 
 /*
- * Add a new procfs dentry that can't serve as a mountpoint. That should
- * encompass anything that is ephemeral and can just disappear while the
- * process is still around.
+ * Add a new procfs dentry that can't serve as a mountpoint.
+ *
+ * If ->d_revalidate() fails the VFS calls d_invalidate() on the dentry. That
+ * detaches every mount on it and below it, in every mount namespace, locked
+ * mounts included, and the next lookup creates a new dentry without them. So
+ * nothing whose ->d_revalidate() can fail while the process is still around
+ * may be a mountpoint: the entries below fd/ and fdinfo/ once the fd is
+ * closed, the entries below map_files/ once the mapping is gone and
+ * everything below net/ where proc_net_d_revalidate() fails on every lookup.
+ * The links into /proc/self can't be mountpoints either. A mount on one of
+ * them would redirect every process that walks through it.
+ *
+ * The other entries of a process fail pid_revalidate() only once the task
+ * is gone and proc_flush_pid() invalidates them when it is reaped. Mounts
+ * on them go away together with the task.
  */
 static inline struct dentry *proc_splice_unmountable(struct inode *inode,
 		struct dentry *dentry, const struct dentry_operations *d_ops)

-- 
2.53.0

[PATCH 7/7] selftests/proc: test which entries can't be mountpoints

[Christian Brauner]

Check that nothing can be mounted on:

- /proc/self, /proc/thread-self, /proc/mounts and /proc/net themselves
- the entries below /proc/<pid>/net
- the entries below /proc/<pid>/fd and /proc/<pid>/fdinfo

Mounting through /proc/mounts still works and lands on the entry of the
caller.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 tools/testing/selftests/proc/.gitignore         |   1 +
 tools/testing/selftests/proc/Makefile           |   1 +
 tools/testing/selftests/proc/proc-unmountable.c | 150 ++++++++++++++++++++++++
 3 files changed, 152 insertions(+)

diff --git a/tools/testing/selftests/proc/.gitignore b/tools/testing/selftests/proc/.gitignore
index 9c9735570abf..1dd43799ec45 100644
--- a/tools/testing/selftests/proc/.gitignore
+++ b/tools/testing/selftests/proc/.gitignore
@@ -20,6 +20,7 @@
 /proc-uptime-001
 /proc-uptime-002
 /proc-pidns
+/proc-unmountable
 /read
 /self
 /setns-dcache
diff --git a/tools/testing/selftests/proc/Makefile b/tools/testing/selftests/proc/Makefile
index a7de2bb6d8be..49afe08daebe 100644
--- a/tools/testing/selftests/proc/Makefile
+++ b/tools/testing/selftests/proc/Makefile
@@ -30,5 +30,6 @@ TEST_GEN_PROGS += thread-self
 TEST_GEN_PROGS += proc-multiple-procfs
 TEST_GEN_PROGS += proc-fsconfig-hidepid
 TEST_GEN_PROGS += proc-pidns
+TEST_GEN_PROGS += proc-unmountable
 
 include ../lib.mk
diff --git a/tools/testing/selftests/proc/proc-unmountable.c b/tools/testing/selftests/proc/proc-unmountable.c
new file mode 100644
index 000000000000..a1282927cfff
--- /dev/null
+++ b/tools/testing/selftests/proc/proc-unmountable.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0
+/* The entries in /proc that can't be mountpoints. */
+
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <sched.h>
+#include <stdio.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#include "kselftest_harness.h"
+
+#ifndef OPEN_TREE_CLONE
+#define OPEN_TREE_CLONE 1
+#endif
+
+#ifndef MOVE_MOUNT_F_EMPTY_PATH
+#define MOVE_MOUNT_F_EMPTY_PATH 0x00000004
+#endif
+
+#ifndef MOVE_MOUNT_T_EMPTY_PATH
+#define MOVE_MOUNT_T_EMPTY_PATH 0x00000040
+#endif
+
+#ifndef __NR_open_tree
+#define __NR_open_tree 428
+#endif
+
+#ifndef __NR_move_mount
+#define __NR_move_mount 429
+#endif
+
+static int sys_open_tree(int dfd, const char *path, unsigned int flags)
+{
+	return syscall(__NR_open_tree, dfd, path, flags);
+}
+
+static int sys_move_mount(int from_dfd, const char *from_path, int to_dfd,
+			  const char *to_path, unsigned int flags)
+{
+	return syscall(__NR_move_mount, from_dfd, from_path, to_dfd, to_path,
+		       flags);
+}
+
+/* Mount a bind of /dev/null right on the object @fd refers to. */
+static int move_mount_null(int fd)
+{
+	int fd_tree, ret;
+
+	fd_tree = sys_open_tree(AT_FDCWD, "/dev/null", OPEN_TREE_CLONE);
+	if (fd_tree < 0)
+		return -1;
+	ret = sys_move_mount(fd_tree, "", fd, "",
+			     MOVE_MOUNT_F_EMPTY_PATH | MOVE_MOUNT_T_EMPTY_PATH);
+	close(fd_tree);
+	return ret;
+}
+
+FIXTURE(unmountable) {
+	char path[PATH_MAX];
+};
+
+FIXTURE_SETUP(unmountable)
+{
+	if (geteuid())
+		SKIP(return, "mounting needs root");
+
+	ASSERT_EQ(unshare(CLONE_NEWNS), 0);
+	ASSERT_EQ(mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL), 0);
+}
+
+FIXTURE_TEARDOWN(unmountable)
+{
+}
+
+/* Nothing may be mounted on a link into /proc/self itself. */
+TEST_F(unmountable, self_links)
+{
+	static const char *const links[] = {
+		"/proc/self", "/proc/thread-self", "/proc/mounts", "/proc/net",
+	};
+
+	for (size_t i = 0; i < ARRAY_SIZE(links); i++) {
+		int fd;
+
+		fd = open(links[i], O_PATH | O_NOFOLLOW | O_CLOEXEC);
+		/* /proc/net needs CONFIG_NET */
+		if (fd < 0 && errno == ENOENT)
+			continue;
+		ASSERT_GE(fd, 0);
+		EXPECT_EQ(move_mount_null(fd), -1) {
+			TH_LOG("mounted on %s", links[i]);
+		}
+		EXPECT_EQ(errno, ENOENT);
+		close(fd);
+	}
+}
+
+/* Mounting through a link into /proc/self lands on the entry of the caller. */
+TEST_F(unmountable, through_links)
+{
+	struct stat st;
+
+	ASSERT_EQ(mount("/dev/null", "/proc/mounts", NULL, MS_BIND, NULL), 0);
+	/* the caller's /proc/<pid>/mounts is covered */
+	ASSERT_EQ(stat("/proc/self/mounts", &st), 0);
+	EXPECT_TRUE(S_ISCHR(st.st_mode));
+	/* the link itself is not */
+	ASSERT_EQ(lstat("/proc/mounts", &st), 0);
+	EXPECT_TRUE(S_ISLNK(st.st_mode));
+}
+
+/* The entries below /proc/<pid>/net take no mount. */
+TEST_F(unmountable, net_entries)
+{
+	if (access("/proc/self/net/dev", F_OK))
+		SKIP(return, "needs CONFIG_NET");
+
+	ASSERT_EQ(mount("/dev/null", "/proc/self/net/dev", NULL, MS_BIND, NULL), -1);
+	ASSERT_EQ(errno, ENOENT);
+}
+
+/* Neither do the entries below /proc/<pid>/fd and /proc/<pid>/fdinfo. */
+TEST_F(unmountable, fd_entries)
+{
+	int fd, fd_link;
+
+	fd = open("/dev/null", O_RDONLY | O_CLOEXEC);
+	ASSERT_GE(fd, 0);
+
+	snprintf(self->path, sizeof(self->path), "/proc/self/fd/%d", fd);
+	fd_link = open(self->path, O_PATH | O_NOFOLLOW | O_CLOEXEC);
+	ASSERT_GE(fd_link, 0);
+	EXPECT_EQ(move_mount_null(fd_link), -1);
+	EXPECT_EQ(errno, ENOENT);
+	close(fd_link);
+
+	snprintf(self->path, sizeof(self->path), "/proc/self/fdinfo/%d", fd);
+	EXPECT_EQ(mount("/dev/null", self->path, NULL, MS_BIND, NULL), -1);
+	EXPECT_EQ(errno, ENOENT);
+	close(fd);
+}
+
+TEST_HARNESS_MAIN

-- 
2.53.0