[PATCH 0/2] exec: randomize the argument block's in-page offset

[PATCH 0/2] exec: randomize the argument block's in-page offset

[Brian Witte]

Patch 1 randomizes where the argv, envp and execfn strings sit within
the first stack page, as KSPP issue 283 asks; today their low address
bits never change. Patch 2 adds a selftest.

Patch 1 repeats bprm_fill_uid()'s set-id checks; I can send a shared
helper as a preparatory patch if preferred.

Tested in QEMU, compared with unpatched for-next/execve (b36b990c50b1),
on x86_64 with ia32, arm64 (4K pages with AArch32, and 64K pages), and
parisc 32-bit and 64-bit (STACK_GROWSUP):

 - The selftest passes; unpatched, it fails.
 - 10000 execs give 3709-3764 distinct offsets with 4K pages and 9259
   with 64K, as a uniform draw predicts.
 - Cost: about 0.5 more minor page faults per exec on average, and up
   to a page of RLIMIT_STACK; exec time is within noise.
 - No change in LTP execve and execveat, CRIU dump and restore, or
   programs that rewrite argv[0] (setproctitle(), sshd, postgres), and
   no KASAN, UBSAN or lockdep reports under exec and chmod races.

The series is based on mainline (864c07de0c7d), which now includes
for-next/execve and the const mnt_idmap conversion. On that base it adds
no W=1 warnings with GCC 14 or clang 19 on x86_64 and arm64, and the
selftest passes on x86_64.

Brian Witte (2):
  exec: randomize the argument block's offset within its page
  selftests/exec: test the argument block's in-page offset

 fs/exec.c                                     |  32 +++
 tools/testing/selftests/exec/.gitignore       |   3 +
 tools/testing/selftests/exec/Makefile         |   5 +
 .../testing/selftests/exec/argblock-offset.c  | 214 ++++++++++++++++++
 .../testing/selftests/exec/argblock-report.c  |  24 ++
 5 files changed, 278 insertions(+)
 create mode 100644 tools/testing/selftests/exec/argblock-offset.c
 create mode 100644 tools/testing/selftests/exec/argblock-report.c


base-commit: 864c07de0c7dcbca58b61625dfa2e461ae63b242
-- 
2.47.3

[PATCH 1/2] exec: randomize the argument block's offset within its page

[Brian Witte]

bprm->p starts one word below STACK_TOP_MAX, and setup_arg_pages()
moves the stack only by whole pages, so the argv, envp and execfn
strings sit at the same offset within their page on every exec. Three
execs on x86_64 put argv[0] at 0x7ffeee30af9f, 0x7ffdf3709f9f and
0x7ffc871e0f9f.

Lower bprm->p by a random amount under a page before the strings are
copied: about 12 bits of entropy with 4K pages and 16 with 64K.
bprm->argmin is derived from bprm->p, so the argument size limit is
unchanged, but up to a page of RLIMIT_STACK is used.

ADDR_NO_RANDOMIZE disables the offset; PF_RANDOMIZE cannot be used,
since load_elf_binary() sets it after the strings are copied.
PER_CLEAR_ON_SETID also clears the flag for set-id execs only after the
copy, and moving the strings then would mean copying them all. So
bprm_fill_uid()'s set-id checks are repeated early, without its
inode_lock(), and a set-id exec gets the offset regardless of the flag.
A racing chmod or sysctl write can make the offset disagree with the ELF
loader but never affects the credentials. Only the file passed to
execve() is checked: set-id interpreters, file capabilities and LSM
transitions keep today's fixed offset under the flag. The checks need
bprm->file, so they run in bprm_mm_init(), not create_init_stack_vma().

The idea comes from PaX RANDUSTACK; linux-hardened carries a version by
Daniel Micay and Levente Polyak.

Suggested-by: Daniel Micay <danielmicay@gmail.com>
Link: https://github.com/KSPP/linux/issues/283
Signed-off-by: Brian Witte <brianwitte@mailfence.com>
---
 fs/exec.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/fs/exec.c b/fs/exec.c
index 5d7b0fe32dbf..1070f0799b27 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -70,6 +70,7 @@
 #include <linux/user_events.h>
 #include <linux/rseq.h>
 #include <linux/ksm.h>
+#include <linux/random.h>
 
 #include <linux/uaccess.h>
 #include <asm/mmu_context.h>
@@ -195,6 +196,33 @@ static bool valid_arg_len(struct linux_binprm *bprm, long len)
 	return len <= MAX_ARG_STRLEN;
 }
 
+/*
+ * Called before credentials exist: treat ADDR_NO_RANDOMIZE as cleared for
+ * files bprm_fill_uid() will treat as set-id.
+ */
+static bool bprm_randomize_args(struct linux_binprm *bprm)
+{
+	struct file *file = bprm->file;
+	const struct mnt_idmap *idmap = file_mnt_idmap(file);
+	struct inode *inode = file_inode(file);
+	/* bprm->cred, not yet set, will be in the caller's namespace. */
+	struct user_namespace *ns = current_user_ns();
+	umode_t mode;
+
+	if (!READ_ONCE(randomize_va_space))
+		return false;
+	if (!(current->personality & ADDR_NO_RANDOMIZE))
+		return true;
+	if (!mnt_may_suid(file->f_path.mnt) || task_no_new_privs(current))
+		return false;
+	mode = READ_ONCE(inode->i_mode);
+	if (!(mode & S_ISUID) &&
+	    (mode & (S_ISGID | S_IXGRP)) != (S_ISGID | S_IXGRP))
+		return false;
+	return vfsuid_has_mapping(ns, i_uid_into_vfsuid(idmap, inode)) &&
+	       vfsgid_has_mapping(ns, i_gid_into_vfsgid(idmap, inode));
+}
+
 #else
 
 static inline void acct_arg_size(struct linux_binprm *bprm, unsigned long pages)
@@ -279,6 +307,10 @@ static int bprm_mm_init(struct linux_binprm *bprm)
 	err = create_init_stack_vma(bprm->mm, &bprm->vma, &bprm->p);
 	if (err)
 		goto err;
+
+	/* Randomize the strings' offset within their page; keep the top word. */
+	if (bprm_randomize_args(bprm))
+		bprm->p -= get_random_u32_below(PAGE_SIZE - sizeof(void *));
 #endif
 
 	return 0;
-- 
2.47.3

[PATCH 2/2] selftests/exec: test the argument block's in-page offset

[Brian Witte]

Exec a helper, argblock-report, 64 times per case. Each case checks that
the in-page offset of argv[0] varies exactly when the stack top chosen
by the ELF loader does:
 - under ADDR_NO_RANDOMIZE: fixed;
 - by default: random;
 - a set-user-ID copy under ADDR_NO_RANDOMIZE: random;
 - that copy on a nosuid mount, under no_new_privs, or in a new user
   namespace where its owner has no mapping: fixed.

The tests use the kselftest harness. The fixture keeps the copy in a
private temporary directory and removes it on teardown. The copy is
owned by the caller, so no privilege is needed.

Link: https://github.com/KSPP/linux/issues/283
Signed-off-by: Brian Witte <brianwitte@mailfence.com>
---
 tools/testing/selftests/exec/.gitignore       |   3 +
 tools/testing/selftests/exec/Makefile         |   5 +
 .../testing/selftests/exec/argblock-offset.c  | 214 ++++++++++++++++++
 .../testing/selftests/exec/argblock-report.c  |  24 ++
 4 files changed, 246 insertions(+)
 create mode 100644 tools/testing/selftests/exec/argblock-offset.c
 create mode 100644 tools/testing/selftests/exec/argblock-report.c

diff --git a/tools/testing/selftests/exec/.gitignore b/tools/testing/selftests/exec/.gitignore
index e42ecd4c908d..991097f7fc43 100644
--- a/tools/testing/selftests/exec/.gitignore
+++ b/tools/testing/selftests/exec/.gitignore
@@ -9,6 +9,9 @@ execveat.ephemeral
 execveat.denatured
 non-regular
 null-argv
+/argblock-offset
+/argblock-offset.*
+/argblock-report
 /check-exec
 /false
 /inc
diff --git a/tools/testing/selftests/exec/Makefile b/tools/testing/selftests/exec/Makefile
index 91decfa8ff2c..4835c7ca4a20 100644
--- a/tools/testing/selftests/exec/Makefile
+++ b/tools/testing/selftests/exec/Makefile
@@ -21,6 +21,11 @@ TEST_GEN_PROGS += recursion-depth
 TEST_GEN_PROGS += null-argv
 TEST_GEN_PROGS += check-exec
 
+# The argument block's in-page offset must be random exactly when the
+# ELF loader randomizes the stack. Unprivileged.
+TEST_GEN_PROGS += argblock-offset
+TEST_GEN_FILES += argblock-report
+
 # binfmt_misc must not be reachable as an exec source or as a stacking layer,
 # or an 'F' entry can pin the instance that owns it. Unprivileged, no bpf.
 TEST_GEN_PROGS += binfmt_misc_selfpin
diff --git a/tools/testing/selftests/exec/argblock-offset.c b/tools/testing/selftests/exec/argblock-offset.c
new file mode 100644
index 000000000000..6edf9e6aadaf
--- /dev/null
+++ b/tools/testing/selftests/exec/argblock-offset.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * The argument block should land at a random offset within its page
+ * exactly when the ELF loader randomizes the stack: always, unless
+ * ADDR_NO_RANDOMIZE is set and the exec is not set-id. Each case
+ * execs argblock-report repeatedly and compares both.
+ *
+ * Runs unprivileged: the set-user-ID copy is owned by the caller and
+ * kept in a private directory.
+ */
+#define _GNU_SOURCE
+#include <sched.h>
+#include <sys/personality.h>
+#include <sys/prctl.h>
+#include <sys/stat.h>
+#include <sys/statvfs.h>
+
+#include "binfmt_misc_common.h"	/* artifact_path(), copy_file() */
+#include "kselftest_harness.h"
+
+/*
+ * A page holds at least 4088 possible offsets, so 64 samples all landing
+ * on one value happen by chance with probability at most (1/4088)^63.
+ */
+#define SAMPLES 64
+
+/* How the helper is exec'd. */
+#define NO_RAND	0x1	/* under ADDR_NO_RANDOMIZE */
+#define NNP	0x2	/* under no_new_privs */
+#define USERNS	0x4	/* in a new user namespace with no mappings */
+
+FIXTURE(argblock) {
+	char report[PATH_MAX];	/* the helper */
+	char dir[32];		/* private directory for the set-id copy */
+	char setid[PATH_MAX];	/* set-user-ID copy of the helper */
+	bool have_dir, have_setid, nosuid;
+	int randomize_va_space;
+};
+
+static int read_randomize_va_space(void)
+{
+	FILE *f = fopen("/proc/sys/kernel/randomize_va_space", "r");
+	int v = -1;
+
+	if (f) {
+		if (fscanf(f, "%d", &v) != 1)
+			v = -1;
+		fclose(f);
+	}
+	return v;
+}
+
+FIXTURE_SETUP(argblock)
+{
+	struct statvfs sv;
+
+	ASSERT_EQ(artifact_path(self->report, sizeof(self->report),
+				"argblock-report"), 0);
+	self->randomize_va_space = read_randomize_va_space();
+
+	/* A private directory keeps the set-user-ID copy out of reach. */
+	strcpy(self->dir, "argblock-offset.XXXXXX");
+	self->have_dir = mkdtemp(self->dir);
+	if (!self->have_dir)
+		return;
+	snprintf(self->setid, sizeof(self->setid), "%s/setid", self->dir);
+	self->have_setid = !copy_file(self->report, self->setid) &&
+			   !chmod(self->setid, 04755);
+	self->nosuid = !statvfs(self->dir, &sv) && (sv.f_flag & ST_NOSUID);
+}
+
+FIXTURE_TEARDOWN(argblock)
+{
+	if (!self->have_dir)
+		return;
+	unlink(self->setid);
+	rmdir(self->dir);
+}
+
+struct sample {
+	unsigned long offset;		/* argv[0] within its page */
+	unsigned long stack_top;	/* end of [stack], chosen by the loader */
+};
+
+static int sample(const char *path, int how, struct sample *s)
+{
+	int pfd[2], status;
+	char buf[64];
+	ssize_t n;
+	pid_t pid;
+
+	if (pipe(pfd))
+		return -1;
+	pid = fork();
+	if (pid < 0)
+		return -1;
+	if (pid == 0) {
+		char *argv[] = { (char *)path, NULL };
+		char *envp[] = { NULL };
+
+		if ((how & NO_RAND) && personality(ADDR_NO_RANDOMIZE) < 0)
+			_exit(2);
+		if ((how & NNP) && prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0))
+			_exit(2);
+		if ((how & USERNS) && unshare(CLONE_NEWUSER))
+			_exit(2);
+		dup2(pfd[1], STDOUT_FILENO);
+		execve(path, argv, envp);
+		_exit(3);
+	}
+	close(pfd[1]);
+	n = read(pfd[0], buf, sizeof(buf) - 1);
+	close(pfd[0]);
+	waitpid(pid, &status, 0);
+	if (n <= 0 || !WIFEXITED(status) || WEXITSTATUS(status))
+		return -1;
+	buf[n] = '\0';
+	return sscanf(buf, "%lx %lx", &s->offset, &s->stack_top) == 2 ? 0 : -1;
+}
+
+/*
+ * Exec @path SAMPLES times. The in-page offset must vary as @expect says,
+ * and vary exactly when the loader's stack top does.
+ */
+static void check(struct __test_metadata *_metadata, const char *path,
+		  int how, bool expect)
+{
+	bool offset_varies = false, stack_varies = false;
+	struct sample first, s;
+	int i;
+
+	for (i = 0; i < SAMPLES; i++) {
+		ASSERT_EQ(sample(path, how, &s), 0)
+			TH_LOG("exec of %s failed", path);
+		if (!i)
+			first = s;
+		offset_varies |= s.offset != first.offset;
+		stack_varies |= s.stack_top != first.stack_top;
+	}
+	EXPECT_EQ(stack_varies, expect)
+		TH_LOG("the loader %s the stack",
+		       stack_varies ? "randomized" : "did not randomize");
+	EXPECT_EQ(offset_varies, expect)
+		TH_LOG("the argument block offset is %s",
+		       offset_varies ? "random" : "fixed");
+}
+
+/* Can this process create a user namespace? */
+static bool have_userns(void)
+{
+	int status;
+	pid_t pid = fork();
+
+	if (pid == 0)
+		_exit(unshare(CLONE_NEWUSER) ? 1 : 0);
+	return pid > 0 && waitpid(pid, &status, 0) == pid &&
+	       WIFEXITED(status) && !WEXITSTATUS(status);
+}
+
+/* Without privilege, ADDR_NO_RANDOMIZE keeps the offset fixed. */
+TEST_F(argblock, addr_no_randomize_keeps_offset_fixed)
+{
+	check(_metadata, self->report, NO_RAND, false);
+}
+
+/* By default the offset is random. */
+TEST_F(argblock, offset_is_random)
+{
+	if (self->randomize_va_space <= 0)
+		SKIP(return, "randomize_va_space is 0");
+	check(_metadata, self->report, 0, true);
+}
+
+/* A set-id exec clears ADDR_NO_RANDOMIZE, so the offset is random again. */
+TEST_F(argblock, setid_ignores_addr_no_randomize)
+{
+	if (self->randomize_va_space <= 0)
+		SKIP(return, "randomize_va_space is 0");
+	if (!self->have_setid)
+		SKIP(return, "cannot create a set-user-ID copy");
+	if (self->nosuid)
+		SKIP(return, "set-user-ID copy is on a nosuid mount");
+	check(_metadata, self->setid, NO_RAND, true);
+}
+
+/* On a nosuid mount the exec gains nothing, so the flag holds. */
+TEST_F(argblock, setid_on_nosuid_keeps_offset_fixed)
+{
+	if (!self->have_setid)
+		SKIP(return, "cannot create a set-user-ID copy");
+	if (!self->nosuid)
+		SKIP(return, "set-user-ID copy is not on a nosuid mount");
+	check(_metadata, self->setid, NO_RAND, false);
+}
+
+/* no_new_privs makes the exec ignore the set-id bit. */
+TEST_F(argblock, setid_under_no_new_privs_keeps_offset_fixed)
+{
+	if (!self->have_setid)
+		SKIP(return, "cannot create a set-user-ID copy");
+	check(_metadata, self->setid, NO_RAND | NNP, false);
+}
+
+/* The copy's owner has no mapping there, so the set-id bit is ignored. */
+TEST_F(argblock, setid_in_user_namespace_keeps_offset_fixed)
+{
+	if (!self->have_setid)
+		SKIP(return, "cannot create a set-user-ID copy");
+	if (!have_userns())
+		SKIP(return, "cannot create a user namespace");
+	check(_metadata, self->setid, NO_RAND | USERNS, false);
+}
+
+TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/exec/argblock-report.c b/tools/testing/selftests/exec/argblock-report.c
new file mode 100644
index 000000000000..45744ec9bda9
--- /dev/null
+++ b/tools/testing/selftests/exec/argblock-report.c
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Helper for argblock-offset: print the in-page offset of argv[0] and the
+ * end of the [stack] VMA, which the ELF loader chooses.
+ */
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+
+int main(int argc, char **argv)
+{
+	unsigned long end = 0;
+	char line[256];
+	FILE *maps = fopen("/proc/self/maps", "r");
+
+	while (maps && fgets(line, sizeof(line), maps))
+		if (strstr(line, "[stack]"))
+			sscanf(line, "%*[0-9a-f]-%lx", &end);
+	if (argc < 1 || !end)
+		return 1;
+	printf("%lx %lx\n", (unsigned long)argv[0] & (sysconf(_SC_PAGESIZE) - 1),
+	       end);
+	return 0;
+}
-- 
2.47.3