binfmt_misc: correctly account pre-opened interpreters

An 'F' entry, and every interpreter a 'B' entry binds, holds a file open
from registration until the entry goes away, pinning the file, its inode,
the mount it came from and that mount's superblock. Nothing bounds how
many of those a user namespace can hold. An entry binds at most
BINFMT_MISC_INTERP_MAX interpreters, but nothing caps the entries.

Charge each binding to the user namespace and uid that makes it against a
new UCOUNT_BINFMT_MISC_INTERPRETERS. Going over budget causes -ENOSPC.

A per-instance cap would suck. Instances are keyed on the user
namespace. So any constant is multiplied by the number of namespaces the
caller creates. Creating those is virtually free. A ucount charges the
namespace and every one of its ancestors. And a namespace can raise only
its own limit. So nesting buys nothing.

The knob is /proc/sys/user/max_binfmt_misc_interpreters. Leave it at the
max_threads/2 default fork_init() gives a new type. No existing
configuration comes close to that.

binfmt_misc is tristate, which makes it the first ucount user that can be
built as a module. Export inc_ucount() and dec_ucount(); without them
CONFIG_BINFMT_MISC=m fails to link. Export them to binfmt_misc alone:
charging a ucount type is not something a module has any business doing
in general, and the list is trivial to extend if a second user shows up.
init_user_ns and init_binfmt_misc are already exported for the same
module.

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

diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index ad8c4f64bf102de27bb00d2a72f7ab68721dfd0b..a3aa42fd57614c5fe5d30d77f070e25dafb2a1ec 100644 (file)
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -289,6 +289,7 @@ static void entry_put_interpreters(struct binfmt_misc_entry *e)
        list_for_each_entry_safe(interp, tmp, &e->interps, list) {
                list_del(&interp->list);
                close_interp_file(interp->file);
+               dec_ucount(interp->ucounts, UCOUNT_BINFMT_MISC_INTERPRETERS);
                kfree(interp);
        }
 }
@@ -307,7 +308,8 @@ static void entry_put_interpreters(struct binfmt_misc_entry *e)
  * The caller has to have validated @name and @path, established that @e
  * cannot be matched yet, and owns @f until this succeeds.
  *
- * Return: 0 on success, a negative errno on failure
+ * Return: 0 on success, -ENOSPC if the entry is full or the binder is out of
+ *         UCOUNT_BINFMT_MISC_INTERPRETERS budget, a negative errno on failure
  */
 static int entry_attach_interpreter(struct binfmt_misc_entry *e,
                                    const char *name, const char *path,
@@ -315,22 +317,32 @@ static int entry_attach_interpreter(struct binfmt_misc_entry *e,
 {
        size_t nlen = strlen(name), plen = strlen(path);
        struct binfmt_misc_interp *interp;
+       struct ucounts *ucounts;
 
        if (binfmt_misc_find_interp(&e->interps, name))
                return -EEXIST;
        if (list_count_nodes(&e->interps) >= BINFMT_MISC_INTERP_MAX)
                return -ENOSPC;
 
+       /* The binding keeps a file open, so charge it to whoever binds it. */
+       ucounts = inc_ucount(current_user_ns(), current_euid(),
+                            UCOUNT_BINFMT_MISC_INTERPRETERS);
+       if (!ucounts)
+               return -ENOSPC;
+
        /* One allocation, both strings in it, like the entry's own buffer. */
        interp = kmalloc(struct_size(interp, name, nlen + plen + 2),
                         GFP_KERNEL_ACCOUNT);
-       if (!interp)
+       if (!interp) {
+               dec_ucount(ucounts, UCOUNT_BINFMT_MISC_INTERPRETERS);
                return -ENOMEM;
+       }
 
        interp->path = interp->name + nlen + 1;
        strscpy(interp->name, name, nlen + 1);
        strscpy(interp->name + nlen + 1, path, plen + 1);
        interp->file = f;
+       interp->ucounts = ucounts;
        /* Publish the node: a lockless cat may be walking the list. */
        list_add_tail_rcu(&interp->list, &e->interps);
        pr_debug("register: interpreter: %s {%s}\n", name, path);

diff --git a/include/linux/binfmt_misc.h b/include/linux/binfmt_misc.h
index 072e4b3dd78d2f8a8010c86a3730e455511ec219..8045b10dd3e5c59013afae2d053ffd0b535561c6 100644 (file)
--- a/include/linux/binfmt_misc.h
+++ b/include/linux/binfmt_misc.h
@@ -7,6 +7,7 @@
 struct bpf_prog;
 struct file;
 struct linux_binprm;
+struct ucounts;
 struct user_namespace;
 
 #define BINFMT_MISC_OPS_NAME_MAX 16
@@ -21,6 +22,7 @@ struct user_namespace;
  * struct binfmt_misc_interp - an interpreter an entry was registered with
  * @list: link in the entry's list, in registration order
  * @file: the file, opened at registration and never resolved again
+ * @ucounts: the UCOUNT_BINFMT_MISC_INTERPRETERS charge the binding took
  * @path: the path it was registered under, used as the name the interpreter
  *        runs under; stored after @name in the same allocation
  * @name: the name the load program selects it by; empty for the fixed
@@ -33,6 +35,7 @@ struct user_namespace;
 struct binfmt_misc_interp {
        struct list_head        list;
        struct file             *file;
+       struct ucounts          *ucounts;
        const char              *path;
        char                    name[];
 };

diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 9c3be157397e021e4d6422b578d502216dfe9697..e38d9e60569f0067f3f0a699d2d0ac8c0d9d4a89 100644 (file)
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -57,6 +57,9 @@ enum ucount_type {
 #ifdef CONFIG_FANOTIFY
        UCOUNT_FANOTIFY_GROUPS,
        UCOUNT_FANOTIFY_MARKS,
+#endif
+#if IS_ENABLED(CONFIG_BINFMT_MISC)
+       UCOUNT_BINFMT_MISC_INTERPRETERS,
 #endif
        UCOUNT_COUNTS,
 };

diff --git a/kernel/ucount.c b/kernel/ucount.c
index d6dc3e859f129605b3b3a998e3b1a7deb24fef2c..ec8b1445e287712b006c2de7931605c3688639e0 100644 (file)
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -4,6 +4,7 @@
 #include <linux/sysctl.h>
 #include <linux/slab.h>
 #include <linux/cred.h>
+#include <linux/export.h>
 #include <linux/hash.h>
 #include <linux/kmemleak.h>
 #include <linux/user_namespace.h>
@@ -89,6 +90,9 @@ static const struct ctl_table user_table[] = {
        UCOUNT_ENTRY("max_fanotify_groups"),
        UCOUNT_ENTRY("max_fanotify_marks"),
 #endif
+#if IS_ENABLED(CONFIG_BINFMT_MISC)
+       UCOUNT_ENTRY("max_binfmt_misc_interpreters"),
+#endif
 };
 #endif /* CONFIG_SYSCTL */
 
@@ -233,6 +237,7 @@ fail:
        put_ucounts(ucounts);
        return NULL;
 }
+EXPORT_SYMBOL_FOR_MODULES(inc_ucount, "binfmt_misc");
 
 void dec_ucount(struct ucounts *ucounts, enum ucount_type type)
 {
@@ -243,6 +248,7 @@ void dec_ucount(struct ucounts *ucounts, enum ucount_type type)
        }
        put_ucounts(ucounts);
 }
+EXPORT_SYMBOL_FOR_MODULES(dec_ucount, "binfmt_misc");
 
 long inc_rlimit_ucounts(struct ucounts *ucounts, enum rlimit_type type, long v)
 {