vfs-7.4-rc1.binfmt

Please consider pulling these changes from the signed vfs-7.4-rc1.binfmt tag.
 
 Thanks!
 Christian
 -----BEGIN PGP SIGNATURE-----
 
 iHUEABYKAB0WIQRAhzRXHqcMeLMyaSiRxhvAZXjcogUCatHrlgAKCRCRxhvAZXjc
 ohI7AP95N3Ty6xsiT14eGKl2jT6x265bpC4fodnQB098QdVNAQEA40ttNCy6j+i/
 ZR4Hy5e8OAqWdj87h7m4EW9ED5yUog8=
 =2LN3
 -----END PGP SIGNATURE-----

Merge tag 'vfs-7.4-rc1.binfmt' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs

Pull binfmt updates from Christian Brauner:
 "This only lets punctuation delimit a binfmt_misc register string.

  Arbitrary delimiters never made sense and nothing uses them. Every new
  flag shrank the set of possible delimiters and made extensions more
  hazardous than they need to be. Letters, digits, whitespace, control
  characters, bytes above 0x7f and the backslash now get EINVAL.

  Everything deployed uses ':' anyway. This extends the fix that
  rejected a flag character as the delimiter.

  Selftests and documentation are included"

* tag 'vfs-7.4-rc1.binfmt' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs:
  binfmt_misc: document the field delimiter
  selftests/exec: test delimiter restrictions
  binfmt_misc: only let punctuation delimit a register string

Documentation/admin-guide/binfmt-misc.rst

@@ -19,6 +19,9 @@ To actually register a new binary type, you have to set up a string looking like
 ``:name:type:offset:magic:mask:interpreter:flags`` (where you can choose the
 ``:`` upon your needs) and echo it to ``/proc/sys/fs/binfmt_misc/register``.
 
+The first character of the string is its field delimiter and can be any
+ASCII punctuation character other than the backslash ``\``.
+
 Here is what the fields mean:
 
 - ``name``

fs/binfmt_misc.c

@@ -100,6 +100,13 @@ static const struct binfmt_misc_flag *misc_flag_by_char(const char c)
 	return NULL;
 }
 
+static bool misc_valid_delim(const char c)
+{
+	if (!isascii(c) || !ispunct(c))
+		return false;
+	return c != '\\';
+}
+
 struct binfmt_misc_entry {
 	struct hlist_node node;
 	unsigned long flags;		/* type, status, etc. */
@@ -871,10 +878,9 @@ static struct binfmt_misc_entry *create_entry(const char __user *buffer,
 
 	del = *p++;	/* delimiter */
 
-	pr_debug("register: delim: %#x {%c}\n", del, del);
+	pr_debug("register: delim: %#x\n", del);
 
-	/* A flag-char delimiter runs the flag scan off the buffer. */
-	if (misc_flag_by_char(del))
+	if (!misc_valid_delim(del))
 		return ERR_PTR(-EINVAL);
 
 	/* Pad the buffer with the delim to simplify parsing below. */

tools/testing/selftests/exec/Makefile

@@ -45,6 +45,10 @@ TEST_GEN_FILES += binfmt_transparent_interp
 TEST_GEN_PROGS += binfmt_misc_loader
 TEST_GEN_FILES += binfmt_loader_payload binfmt_loader_payload_static
 
+# Only ASCII punctuation delimits the fields of a register string, so a new
+# flag character cannot change which strings register. No bpf toolchain.
+TEST_GEN_PROGS += binfmt_misc_delim
+
 # binfmt_misc bpf-backed ('B') handler test: a libbpf harness plus its
 # struct_ops objects and the test interpreter/app it routes between. Only
 # built when clang, bpftool, the vmlinux BTF and libbpf are all present

tools/testing/selftests/exec/binfmt_misc_delim.c

@@ -0,0 +1,127 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Test which characters may delimit the fields of a register string.
+ */
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "binfmt_misc_common.h"
+#include "kselftest_harness.h"
+
+#define ENTRY	"bmdelim"
+/* Shares no character with the sets below, or a refusal proves nothing. */
+#define MAGIC	"bmmagic"
+#define INTERP	"/bin/true"
+
+/*
+ * ASCII punctuation without '\' and '/'. The backslash is refused because
+ * it would cut a magic that uses \x to escape short. '/' is accepted
+ * but cannot delimit a rule that names an absolute interpreter.
+ */
+#define PUNCTUATION	"!\"#$%&'()*+,-.:;<=>?@[]^_`{|}~"
+
+/* 'M', 'E' and 'B' name types, 'P' through 'D' are the flags. */
+#define LETTERS		"MEBPOCFTLDqz"
+#define DIGITS		"0157"
+#define WHITESPACE	" \t\n"
+#define CONTROL		"\001\033\177"
+#define NON_ASCII	"\200\244\377"
+
+/* ':bmdelim:E::bmmagic::/bin/true:' with @del in place of every ':'. */
+static int register_with(char del)
+{
+	char rule[128];
+
+	snprintf(rule, sizeof(rule), "%c%s%cE%c%c%s%c%c%s%c", del, ENTRY, del,
+		 del, del, MAGIC, del, del, INTERP, del);
+	return write_reg(rule);
+}
+
+/* No character of @set may delimit a register string. */
+static void expect_refused(struct __test_metadata *_metadata, const char *set)
+{
+	const char *d;
+
+	for (d = set; *d; d++) {
+		int rc = register_with(*d);
+
+		EXPECT_EQ(rc, -1)
+			TH_LOG("%#x delimited a register string",
+			       (unsigned char)*d);
+		if (rc == 0) {
+			unregister(ENTRY);
+			continue;
+		}
+		EXPECT_EQ(errno, EINVAL);
+	}
+}
+
+FIXTURE(delim) {
+};
+
+FIXTURE_SETUP(delim)
+{
+	if (getuid() != 0)
+		SKIP(return, "test must be run as root");
+	if (!binfmt_misc_available())
+		SKIP(return, "no binfmt_misc");
+
+	/* A kernel without the allow-list takes any character but a flag. */
+	if (register_with('q') == 0) {
+		unregister(ENTRY);
+		SKIP(return, "kernel without the delimiter allow-list");
+	}
+}
+
+FIXTURE_TEARDOWN(delim)
+{
+	unregister(ENTRY);
+}
+
+/* Punctuation delimits, which is all anything deployed ever uses. */
+TEST_F(delim, punctuation_accepted)
+{
+	const char *d;
+
+	for (d = PUNCTUATION; *d; d++) {
+		EXPECT_EQ(register_with(*d), 0)
+			TH_LOG("'%c' refused with errno %d", *d, errno);
+		unregister(ENTRY);
+	}
+}
+
+/* Letters name the types and the flags, so none of them can delimit. */
+TEST_F(delim, letters_refused)
+{
+	expect_refused(_metadata, LETTERS);
+}
+
+/* The offset field is written in digits. */
+TEST_F(delim, digits_refused)
+{
+	expect_refused(_metadata, DIGITS);
+}
+
+TEST_F(delim, whitespace_refused)
+{
+	expect_refused(_metadata, WHITESPACE);
+}
+
+TEST_F(delim, control_refused)
+{
+	expect_refused(_metadata, CONTROL);
+}
+
+TEST_F(delim, non_ascii_refused)
+{
+	expect_refused(_metadata, NON_ASCII);
+}
+
+/* The escape character would cut every magic that uses one short. */
+TEST_F(delim, backslash_refused)
+{
+	expect_refused(_metadata, "\\");
+}
+
+TEST_HARNESS_MAIN