Merge patch series "binfmt_misc: bound the interpreters an entry can pre-open"

Christian Brauner <brauner@kernel.org> says:

An 'F' entry opens its interpreter at registration and every exec runs a
clone of it. A 'B' entry does the same for each interpreter it binds. That
file stays open for as long as the entry lives. So it pins the file, its
inode, the mount it came from and that mount's superblock.

An entry binds at most 100 interpreters, but nothing caps the entries.
binfmt_misc is container mountable so all of this is reachable by
unprivileged users.

While the pins go away when the instance is unmounted, it's still weird
for an unprivileged namespace to be allowed to do this. And the fix is
simple.

Charge each binding to the user namespace and uid that makes it against
a new UCOUNT_BINFMT_MISC_INTERPRETERS and refuse with -ENOSPC when the
limit is hit.

A per-instance cap won't do. Instances are keyed on the user namespace, so
whatever constant I pick gets multiplied by however many namespaces the
caller cares to create. inc_ucount() charges the namespace and every one
of its ancestors, and a namespace can only ever raise its own limit, so
nesting buys nothing.

The knob is /proc/sys/user/max_binfmt_misc_interpreters, per namespace like
every other ucount. I left it at the max_threads/2 default that
fork_init() hands a new ucount type. Nothing anyone runs today comes
anywhere near that.

Selftests for all of it, including that a nested namespace can't buy
itself budget.

* patches from https://patch.msgid.link/20260803-work-binfmt_misc-interplimit-v1-0-4a2435500bd9@kernel.org:
  binfmt_misc: document the pre-opened interpreter limit
  selftests/exec: test the pre-opened interpreter limit
  binfmt_misc: correctly account pre-opened interpreters

Link: https://patch.msgid.link/20260803-work-binfmt_misc-interplimit-v1-0-4a2435500bd9@kernel.org
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

4 files changed, 309 insertions, 0 deletions

diff --git a/tools/testing/selftests/exec/.gitignore b/tools/testing/selftests/exec/.gitignore
index fbbb1600ddb9..e42ecd4c908d 100644
--- a/tools/testing/selftests/exec/.gitignore
+++ b/tools/testing/selftests/exec/.gitignore
@@ -20,6 +20,7 @@ xxxxxxxx*
 pipe
 S_I*.test
 binfmt_misc_bpf
+binfmt_misc_interplimit
 binfmt_bpf_interp
 binfmt_bpf_app
 binfmt_misc_transparent
diff --git a/tools/testing/selftests/exec/Makefile b/tools/testing/selftests/exec/Makefile
index 410c93606a0c..b640af8f02b5 100644
--- a/tools/testing/selftests/exec/Makefile
+++ b/tools/testing/selftests/exec/Makefile
@@ -25,6 +25,10 @@ TEST_GEN_PROGS += check-exec
 # or an 'F' entry can pin the instance that owns it. Unprivileged, no bpf.
 TEST_GEN_PROGS += binfmt_misc_selfpin
 
+# The interpreters an 'F' or 'B' entry pre-opens are charged against
+# UCOUNT_BINFMT_MISC_INTERPRETERS. Unprivileged, no bpf.
+TEST_GEN_PROGS += binfmt_misc_interplimit
+
 # 'D' (register disabled) binfmt_misc test: an entry that exists but does
 # not dispatch until it is enabled. Static magic entry, no bpf toolchain.
 TEST_GEN_PROGS += binfmt_misc_disabled
@@ -104,6 +108,8 @@ $(OUTPUT)/script-noexec.inc: $(CHECK_EXEC_SAMPLES)/script-noexec.inc
 # CFLAGS for every program in this directory.
 $(OUTPUT)/binfmt_misc_selfpin: CFLAGS += $(TOOLS_INCLUDES)
 $(OUTPUT)/binfmt_misc_selfpin: ../filesystems/utils.c
+$(OUTPUT)/binfmt_misc_interplimit: CFLAGS += $(TOOLS_INCLUDES)
+$(OUTPUT)/binfmt_misc_interplimit: ../filesystems/utils.c
 
 # --- binfmt_misc bpf ('B') handler test ---------------------------------
 # The struct_ops bpf objects are compiled against the running kernel's BTF.
diff --git a/tools/testing/selftests/exec/binfmt_misc_bpf.c b/tools/testing/selftests/exec/binfmt_misc_bpf.c
index 2c7b63075f1d..b2a4518901b0 100644
--- a/tools/testing/selftests/exec/binfmt_misc_bpf.c
+++ b/tools/testing/selftests/exec/binfmt_misc_bpf.c
@@ -38,6 +38,7 @@
 #define _GNU_SOURCE
 #include <elf.h>
 #include <limits.h>
+#include <sched.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -65,6 +66,9 @@
 #define BIND_RISCV_PATH	"/tmp/binfmt_bind_riscv"
 #define BIND_EXPECT	"BIND_RAN "
 #define BIND_MAX	100
+#define INTERP_LIMIT	"/proc/sys/user/max_binfmt_misc_interpreters"
+/* Exit status of the binding child when it cannot set up a budget of its own. */
+#define BIND_NO_BUDGET	200
 
 /* A minimal 64-bit little-endian ELF header, padded to the read size. */
 static int create_fake_elf(const char *path, unsigned short machine)
@@ -378,6 +382,57 @@ static int entry_bind(const char *entry, const char *name, const char *path)
 	return entry_command(entry, cmd);
 }
 
+/* Set the interpreter budget of this namespace. */
+static int write_interp_limit(const char *val)
+{
+	ssize_t n;
+	int fd;
+
+	fd = open(INTERP_LIMIT, O_WRONLY | O_CLOEXEC);
+	if (fd < 0)
+		return -1;
+	n = write(fd, val, strlen(val));
+	close(fd);
+	return n < 0 ? -1 : 0;
+}
+
+/*
+ * The errno a bind is refused with when the writer is a child that has spent
+ * the budget of a user namespace of its own, 0 if it succeeded and -1 if the
+ * child could not set itself up. The fd is opened here and inherited, so the
+ * interpreter is still opened with this process's credentials.
+ */
+static int bind_out_of_budget(const char *entry, const char *name,
+			      const char *path)
+{
+	char cmd[PATH_MAX], file[PATH_MAX];
+	int fd, status, retval;
+	pid_t pid;
+
+	snprintf(file, sizeof(file), BINFMT_DIR "/%s", entry);
+	snprintf(cmd, sizeof(cmd), "+%s %s\n", name, path);
+
+	fd = open(file, O_WRONLY | O_CLOEXEC);
+	if (fd < 0)
+		return -1;
+
+	pid = fork();
+	if (pid == 0) {
+		ssize_t n;
+
+		/* A namespace of its own, with nothing left in it to spend. */
+		if (unshare(CLONE_NEWUSER) || write_interp_limit("0"))
+			_exit(BIND_NO_BUDGET);
+		n = write(fd, cmd, strlen(cmd));
+		_exit(n < 0 ? errno : 0);
+	}
+	close(fd);
+	if (pid < 0 || waitpid(pid, &status, 0) != pid || !WIFEXITED(status))
+		return -1;
+	retval = WEXITSTATUS(status);
+	return retval == BIND_NO_BUDGET ? -1 : retval;
+}
+
 FIXTURE(bound_interp) {
 	char obj[PATH_MAX];
 	struct bpf_case c;
@@ -565,4 +620,19 @@ TEST_F(bound_interp, capped_bindings)
 	EXPECT_EQ(entry_bind("test_interp_bind", "over", BIND_FIRST), -ENOSPC);
 }
 
+/* A binding pins a file: it is charged, and refused once the budget is out. */
+TEST_F(bound_interp, bindings_are_charged)
+{
+	int err = bind_out_of_budget("test_interp_bind", "third", BIND_FIRST);
+
+	if (err < 0)
+		SKIP(return, "no user namespaces or no " INTERP_LIMIT);
+
+	/* The charge follows the writer, not the entry file it writes to. */
+	EXPECT_EQ(err, ENOSPC);
+
+	/* The budget was the only thing in the way. */
+	EXPECT_EQ(entry_bind("test_interp_bind", "third", BIND_FIRST), 0);
+}
+
 TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/exec/binfmt_misc_interplimit.c b/tools/testing/selftests/exec/binfmt_misc_interplimit.c
new file mode 100644
index 000000000000..bf611c551784
--- /dev/null
+++ b/tools/testing/selftests/exec/binfmt_misc_interplimit.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * A pre-opened interpreter - what 'F' gives a static entry and what a 'B'
+ * entry binds - keeps a file open for as long as the entry lives, so it pins
+ * the mount it came from. It costs no file descriptor, and binfmt_misc is
+ * FS_USERNS_MOUNT, so an unprivileged user namespace can create them without
+ * bound. Check that UCOUNT_BINFMT_MISC_INTERPRETERS bounds it, that an entry
+ * that pre-opens nothing is not charged, that removing an entry gives the
+ * charge back, and that nesting a user namespace does not evade it.
+ *
+ * Runs unprivileged in a user namespace.
+ */
+#define _GNU_SOURCE
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <unistd.h>
+
+#include "../filesystems/utils.h"
+#include "kselftest_harness.h"
+
+#define MNT		"/tmp/binfmt_interplimit"
+#define NESTED_MNT	"/tmp/binfmt_interplimit_nested"
+#define LIMIT_SYSCTL	"/proc/sys/user/max_binfmt_misc_interpreters"
+
+#define MAGIC		"\\xde\\xad"
+/* Not on the instance, and unlike /bin/true it always exists. */
+#define INTERP		"/proc/self/exe"
+
+/* Small enough to fill by hand, big enough that a refund is visible. */
+#define LIMIT		4
+
+/* What UCOUNT_ENTRY() lets a namespace raise its own limit to. */
+#define LIMIT_MAX	"2147483647"
+
+static int ensure_dir(const char *path)
+{
+	if (mkdir(path, 0755) && errno != EEXIST)
+		return -1;
+	return 0;
+}
+
+/* Write @val to @path, preserving write(2)'s errno for the caller. */
+static int write_keep_errno(const char *path, const char *val)
+{
+	int fd, saved;
+	ssize_t n;
+
+	fd = open(path, O_WRONLY | O_CLOEXEC);
+	if (fd < 0)
+		return -1;
+	n = write(fd, val, strlen(val));
+	saved = errno;
+	close(fd);
+	errno = saved;
+	return n < 0 ? -1 : 0;
+}
+
+static int set_limit(const char *val)
+{
+	return write_keep_errno(LIMIT_SYSCTL, val);
+}
+
+static int register_at(const char *mnt, const char *rule)
+{
+	char path[PATH_MAX];
+
+	snprintf(path, sizeof(path), "%s/register", mnt);
+	return write_keep_errno(path, rule);
+}
+
+/* An 'F' entry: one interpreter pre-opened at registration, one charge. */
+static int register_fixed(const char *mnt, const char *name)
+{
+	char rule[PATH_MAX];
+
+	snprintf(rule, sizeof(rule), ":%s:M::" MAGIC "::" INTERP ":F", name);
+	return register_at(mnt, rule);
+}
+
+/* The same entry without 'F': the interpreter is opened per exec instead. */
+static int register_plain(const char *mnt, const char *name)
+{
+	char rule[PATH_MAX];
+
+	snprintf(rule, sizeof(rule), ":%s:M::" MAGIC "::" INTERP ":", name);
+	return register_at(mnt, rule);
+}
+
+static int remove_entry(const char *mnt, const char *name)
+{
+	char path[PATH_MAX];
+
+	snprintf(path, sizeof(path), "%s/%s", mnt, name);
+	return write_keep_errno(path, "-1\n");
+}
+
+static bool entry_exists(const char *mnt, const char *name)
+{
+	char path[PATH_MAX];
+
+	snprintf(path, sizeof(path), "%s/%s", mnt, name);
+	return access(path, F_OK) == 0;
+}
+
+/* Register @n 'F' entries, each with a name of its own. */
+static int fill_budget(const char *mnt, unsigned int n)
+{
+	char name[32];
+	unsigned int i;
+
+	for (i = 0; i < n; i++) {
+		snprintf(name, sizeof(name), "fixed%u", i);
+		if (register_fixed(mnt, name))
+			return -1;
+	}
+	return 0;
+}
+
+FIXTURE(interp_limit) {
+};
+
+FIXTURE_SETUP(interp_limit)
+{
+	/* setup_userns() exits rather than returns if this is not there. */
+	if (access("/proc/self/ns/user", F_OK))
+		SKIP(return, "kernel without user namespaces");
+	ASSERT_EQ(setup_userns(), 0);
+
+	/* CAP_SYS_RESOURCE in this namespace is what makes it writable. */
+	if (set_limit(LIMIT_MAX)) {
+		if (errno == ENOENT)
+			SKIP(return, "kernel without " LIMIT_SYSCTL);
+		SKIP(return, "cannot set the limit: %s", strerror(errno));
+	}
+
+	ASSERT_EQ(ensure_dir(MNT), 0);
+	if (mount("binfmt_misc", MNT, "binfmt_misc", 0, NULL)) {
+		int saved = errno;
+
+		/* Teardown doesn't run when setup skips, so clean up here. */
+		rmdir(MNT);
+		SKIP(return, "no binfmt_misc: %s", strerror(saved));
+	}
+}
+
+FIXTURE_TEARDOWN(interp_limit)
+{
+	/* The namespaces go with the process; just don't litter /tmp. */
+	umount2(NESTED_MNT, MNT_DETACH);
+	umount2(MNT, MNT_DETACH);
+	rmdir(NESTED_MNT);
+	rmdir(MNT);
+}
+
+/* Every pre-opened interpreter is charged, and the budget is a hard stop. */
+TEST_F(interp_limit, fixed_interpreters_are_charged)
+{
+	char buf[32];
+
+	snprintf(buf, sizeof(buf), "%u", LIMIT);
+	ASSERT_EQ(set_limit(buf), 0);
+
+	ASSERT_EQ(fill_budget(MNT, LIMIT), 0);
+
+	EXPECT_NE(register_fixed(MNT, "over"), 0);
+	EXPECT_EQ(errno, ENOSPC);
+
+	/* A refused registration leaves nothing behind. */
+	EXPECT_FALSE(entry_exists(MNT, "over"));
+}
+
+/* An entry that pre-opens nothing pins nothing, so it is not charged. */
+TEST_F(interp_limit, plain_entries_are_not_charged)
+{
+	ASSERT_EQ(set_limit("0"), 0);
+
+	EXPECT_EQ(register_plain(MNT, "plain"), 0);
+	EXPECT_TRUE(entry_exists(MNT, "plain"));
+
+	/* ... while the same entry with 'F' has nothing to spend. */
+	EXPECT_NE(register_fixed(MNT, "fixed"), 0);
+	EXPECT_EQ(errno, ENOSPC);
+}
+
+/* Removing an entry closes its interpreters and gives the charge back. */
+TEST_F(interp_limit, removal_refunds_the_charge)
+{
+	char buf[32];
+
+	snprintf(buf, sizeof(buf), "%u", LIMIT);
+	ASSERT_EQ(set_limit(buf), 0);
+
+	ASSERT_EQ(fill_budget(MNT, LIMIT), 0);
+	ASSERT_NE(register_fixed(MNT, "over"), 0);
+
+	ASSERT_EQ(remove_entry(MNT, "fixed0"), 0);
+	EXPECT_EQ(register_fixed(MNT, "over"), 0);
+}
+
+/*
+ * The charge walks the ancestors, so a namespace cannot buy itself budget by
+ * nesting: it may raise only its own limit, and the parent it was created
+ * from is charged for every binding made below it.
+ */
+TEST_F(interp_limit, nesting_does_not_evade_it)
+{
+	char buf[32];
+
+	snprintf(buf, sizeof(buf), "%u", LIMIT);
+	ASSERT_EQ(set_limit(buf), 0);
+	ASSERT_EQ(fill_budget(MNT, LIMIT), 0);
+
+	ASSERT_EQ(setup_userns(), 0);
+	ASSERT_EQ(set_limit(LIMIT_MAX), 0);
+
+	ASSERT_EQ(ensure_dir(NESTED_MNT), 0);
+	ASSERT_EQ(mount("binfmt_misc", NESTED_MNT, "binfmt_misc", 0, NULL), 0);
+
+	/* A fresh instance with an unlimited budget of its own, and yet: */
+	EXPECT_NE(register_fixed(NESTED_MNT, "nested"), 0);
+	EXPECT_EQ(errno, ENOSPC);
+
+	/* The nested instance works for anything that pins no file. */
+	EXPECT_EQ(register_plain(NESTED_MNT, "nested_plain"), 0);
+}
+
+TEST_HARNESS_MAIN