[PATCH v2 0/2] binfmt_elf: return the real error when program headers cannot be loaded

[PATCH v2 0/2] binfmt_elf: return the real error when program headers cannot be loaded

[Konstantin Khorenko]

load_elf_phdrs() returns NULL on any failure, and load_elf_binary()
turns that into -ENOEXEC for the executable and -ELIBBAD for its
interpreter. A failed allocation or read of the program header table
then looks like a bad file, and glibc's execvp() runs the ELF with
/bin/sh.

v2 changes the errno of those two failures and nothing else. Patch 2
adds a selftest for both what stays and what changes.

What changes
============

  +--------------------------------------+----------+------------+
  | the program header table             | before   | after      |
  +--------------------------------------+----------+------------+
  | has a bad e_phentsize or e_phnum     | ENOEXEC  | ENOEXEC    |
  | has an e_phoff past LLONG_MAX - size | ENOEXEC  | ENOEXEC    |
  | starts past the end of the file      | ENOEXEC  | ENOEXEC    |
  | gives a short read                   | ENOEXEC  | ENOEXEC    |
  +--------------------------------------+----------+------------+
  | cannot be allocated                  | ENOEXEC  | ENOMEM     |
  | cannot be read: kernel_read() error  | ENOEXEC  | that error |
  +--------------------------------------+----------+------------+

For the interpreter's table, ELIBBAD stands in place of every ENOEXEC
above; ENOMEM and the read error come back as they are. A read that
fails after some bytes were copied comes back from the page cache as a
short count, so it still gives ENOEXEC.

In the kernel, search_binary_handler() now stops at the ELF handler for
these two errors instead of trying the formats after it. All of those
decline a file with the ELF magic of this machine, so the loop ended
with -ENOEXEC before anyway.

Userspace
=========

The patch adds no errno that execve() cannot return today. Other reads
of the same files already pass the kernel_read() error through:
prepare_binprm() for the first BINPRM_BUF_SIZE bytes of the binary,
and elf_read() for the PT_INTERP path and the interpreter's ELF
header, while parse_elf_properties() reports a failed read as -EIO.
The program header table was the only read of an ELF exec whose error
reached userspace as ENOEXEC.

The execvp() family acts on ENOEXEC: glibc runs the file with /bin/sh,
that exec succeeds, and the shell replaces the caller and exits with an
error such as bash's "cannot execute binary file". glibc's
posix_spawnp() does so only through its compat symbol from before 2.15,
and musl's execvp() never does.

Shells check ENOEXEC themselves when they exec a command: bash reports
"cannot execute binary file", dash runs the file as a script. With the
patch they report the ENOMEM or the read error instead.

QEMU's cpr-exec live update re-execs itself with execvp() and resumes
the guest when that fails; on ENOEXEC the guest is killed instead. With
the patch these callers get the ENOMEM or the read error back.

Testing
=======

x86_64 KVM guest, mainline f66469d9905b with CONFIG_FAILSLAB and
CONFIG_FAULT_INJECTION_DEBUG_FS, selftests/exec:

  +------------------------------+---------------------+---------------+
  |                              | without patch 1     | with patch 1  |
  +------------------------------+---------------------+---------------+
  | load_elf_phdrs, bad tables   | pass                | pass          |
  +------------------------------+---------------------+---------------+
  | load_elf_phdrs, allocation   | fails: ENOEXEC for  | passes        |
  | failures                     | the executable,     |               |
  |                              | ELIBBAD for ld.so   |               |
  +------------------------------+---------------------+---------------+
  | the other exec selftests     | pass                | pass          |
  +------------------------------+---------------------+---------------+

Unprivileged, the allocation case is skipped. A kernel_read() error is
not covered by the selftest, as it needs a filesystem that fails the
read.

Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
Changes in v2:
- Dropped the read loop: a short read gives -ENOEXEC as before, and
  only the errno changes (Kees).
- Patch 2: a selftest for the errors of the table.
- Dropped the Fixes: tags on 6a8d38945cf4 and a9d9ef133f44 (v3.19):
  the errno has been this way since then, and the change should not go
  to stable.
- Rebased on current mainline, f66469d9905b (after v7.4-rc1).
- The Assisted-by line moved from the patch to this letter.
- Link to v1: https://patch.msgid.link/20261014-b4-prep-vstor-149847-binfmt_elf-enomem-load-elf-phdrs-v1-1-f39c74d55e42@virtuozzo.com

---
Konstantin Khorenko (2):
      binfmt_elf: return the real error when program headers cannot be loaded
      selftests/exec: check the errors of an unloadable program header table

 fs/binfmt_elf.c                               |  38 ++-
 tools/testing/selftests/exec/.gitignore       |   2 +
 tools/testing/selftests/exec/Makefile         |   4 +
 tools/testing/selftests/exec/load_elf_phdrs.c | 399 ++++++++++++++++++++++++++
 4 files changed, 428 insertions(+), 15 deletions(-)
---
base-commit: f66469d9905b39d2c260a15d7d6db450f4db67c1
change-id: 20261014-b4-prep-vstor-149847-binfmt_elf-enomem-load-elf-phdrs-395ffbfc33e2

Best regards,
--  
Konstantin Khorenko,
Virtuozzo Linux Kernel Team

[PATCH v2 1/2] binfmt_elf: return the real error when program headers cannot be loaded

[Konstantin Khorenko]

load_elf_phdrs() returns NULL on any failure, and load_elf_binary()
turns that into -ENOEXEC for the executable and -ELIBBAD for its
interpreter. So a failed allocation or read of the program headers
makes glibc's execvp() run the ELF file with /bin/sh, and for the
interpreter reports a healthy ld.so as a corrupted shared library.

Return an ERR_PTR instead:

  +------------+------------------------------------------------------+
  | error      | when                                                 |
  +------------+------------------------------------------------------+
  | -ENOEXEC   | bad e_phentsize, e_phnum or e_phoff, or a short read |
  | -ENOMEM    | the allocation fails                                 |
  | read error | kernel_read() fails                                  |
  +------------+------------------------------------------------------+

Malformed files and short reads keep getting -ENOEXEC and -ELIBBAD.
The table is read with kernel_read() directly, as elf_read() turns a
short read into -EIO. An e_phoff past LLONG_MAX - size is rejected up
front, so that rw_verify_area() does not turn it into -EINVAL.

Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/binfmt_elf.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index bf7f8f47548d3..ee44a354c7243 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -512,39 +512,41 @@ static unsigned long maximum_alignment(struct elf_phdr *cmds, int nr)
  *
  * Loads ELF program headers from the binary file elf_file, which has the ELF
  * header pointed to by elf_ex, into a newly allocated array. The caller is
- * responsible for freeing the allocated data. Returns NULL upon failure.
+ * responsible for freeing the allocated data. Returns an ERR_PTR upon failure:
+ * -ENOEXEC for a malformed header or a short read, the allocation or read
+ * error otherwise.
  */
 static struct elf_phdr *load_elf_phdrs(const struct elfhdr *elf_ex,
 				       struct file *elf_file)
 {
-	struct elf_phdr *elf_phdata = NULL;
-	int retval = -1;
+	struct elf_phdr *elf_phdata;
 	unsigned int size;
+	loff_t pos;
+	ssize_t rv;
 
 	/*
 	 * If the size of this structure has changed, then punt, since
 	 * we will be doing the wrong thing.
 	 */
 	if (elf_ex->e_phentsize != sizeof(struct elf_phdr))
-		goto out;
+		return ERR_PTR(-ENOEXEC);
 
 	/* Sanity check the number of program headers... */
-	/* ...and their total size. */
+	/* ...their total size and their offset. */
 	size = sizeof(struct elf_phdr) * elf_ex->e_phnum;
-	if (size == 0 || size > 65536)
-		goto out;
+	if (size == 0 || size > 65536 || elf_ex->e_phoff > LLONG_MAX - size)
+		return ERR_PTR(-ENOEXEC);
 
 	elf_phdata = kmalloc(size, GFP_KERNEL);
 	if (!elf_phdata)
-		goto out;
+		return ERR_PTR(-ENOMEM);
 
 	/* Read in the program headers */
-	retval = elf_read(elf_file, elf_phdata, size, elf_ex->e_phoff);
-
-out:
-	if (retval) {
+	pos = elf_ex->e_phoff;
+	rv = kernel_read(elf_file, elf_phdata, size, &pos);
+	if (rv != size) {
 		kfree(elf_phdata);
-		elf_phdata = NULL;
+		return ERR_PTR(rv < 0 ? rv : -ENOEXEC);
 	}
 	return elf_phdata;
 }
@@ -864,8 +866,10 @@ static int load_elf_binary(struct linux_binprm *bprm)
 		goto out;
 
 	elf_phdata = load_elf_phdrs(elf_ex, bprm->file);
-	if (!elf_phdata)
+	if (IS_ERR(elf_phdata)) {
+		retval = PTR_ERR(elf_phdata);
 		goto out;
+	}
 
 	elf_ppnt = elf_phdata;
 	for (i = 0; i < elf_ex->e_phnum; i++, elf_ppnt++) {
@@ -968,8 +972,12 @@ static int load_elf_binary(struct linux_binprm *bprm)
 		/* Load the interpreter program headers */
 		interp_elf_phdata = load_elf_phdrs(interp_elf_ex,
 						   interpreter);
-		if (!interp_elf_phdata)
+		if (IS_ERR(interp_elf_phdata)) {
+			if (PTR_ERR(interp_elf_phdata) != -ENOEXEC)
+				retval = PTR_ERR(interp_elf_phdata);
+			interp_elf_phdata = NULL;
 			goto out_free_dentry;
+		}
 
 		/* Pass PT_LOPROC..PT_HIPROC headers to arch code */
 		elf_property_phdata = NULL;

-- 
2.43.0

[PATCH v2 2/2] selftests/exec: check the errors of an unloadable program header table

[Konstantin Khorenko]

load_elf_binary() reports a program header table that it cannot use as
-ENOEXEC, and as -ELIBBAD when the table is the interpreter's. A failed
allocation of the table says nothing about the file and has to come
back as -ENOMEM.

The test builds ELF files whose table is malformed, placed past the end
of the file or cut short, both as the executable and as the PT_INTERP of
one, and checks for those two errors. As root, on a kernel with fault
injection, it then fails each fault injection point of an execve() in
turn and checks that none of them comes back as -ENOEXEC or -ELIBBAD;
without root or fault injection that case is skipped.

Fault injection here means CONFIG_FAULT_INJECTION, CONFIG_FAILSLAB and
CONFIG_FAULT_INJECTION_DEBUG_FS.

Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 tools/testing/selftests/exec/.gitignore       |   2 +
 tools/testing/selftests/exec/Makefile         |   4 +
 tools/testing/selftests/exec/load_elf_phdrs.c | 399 ++++++++++++++++++++++++++
 3 files changed, 405 insertions(+)

diff --git a/tools/testing/selftests/exec/.gitignore b/tools/testing/selftests/exec/.gitignore
index e42ecd4c908d1..3736834117f7c 100644
--- a/tools/testing/selftests/exec/.gitignore
+++ b/tools/testing/selftests/exec/.gitignore
@@ -11,9 +11,11 @@ non-regular
 null-argv
 /check-exec
 /false
+/false-dynamic
 /inc
 /load_address.*
 !load_address.c
+/load_elf_phdrs
 /recursion-depth
 /set-exec
 xxxxxxxx*
diff --git a/tools/testing/selftests/exec/Makefile b/tools/testing/selftests/exec/Makefile
index 91decfa8ff2c1..607f5ecd9d76c 100644
--- a/tools/testing/selftests/exec/Makefile
+++ b/tools/testing/selftests/exec/Makefile
@@ -20,6 +20,8 @@ TEST_FILES := Makefile
 TEST_GEN_PROGS += recursion-depth
 TEST_GEN_PROGS += null-argv
 TEST_GEN_PROGS += check-exec
+TEST_GEN_PROGS += load_elf_phdrs
+TEST_GEN_PROGS_EXTENDED += false-dynamic
 
 # 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.
@@ -98,6 +100,8 @@ $(OUTPUT)/load_address.static.0x%: load_address.c
 		-fPIE -static-pie $< -o $@
 $(OUTPUT)/false: false.c
 	$(CC) $(CFLAGS) $(LDFLAGS) -static $< -o $@
+$(OUTPUT)/false-dynamic: false.c
+	$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
 $(OUTPUT)/inc: $(CHECK_EXEC_SAMPLES)/inc.c
 	$(CC) $(CFLAGS) $(LDFLAGS) $< -o $@
 $(OUTPUT)/set-exec: $(CHECK_EXEC_SAMPLES)/set-exec.c
diff --git a/tools/testing/selftests/exec/load_elf_phdrs.c b/tools/testing/selftests/exec/load_elf_phdrs.c
new file mode 100644
index 0000000000000..7d888a3e9d7f4
--- /dev/null
+++ b/tools/testing/selftests/exec/load_elf_phdrs.c
@@ -0,0 +1,399 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * execve() of an ELF file whose program header table cannot be loaded.
+ *
+ * A malformed, misplaced or truncated table fails with ENOEXEC, and with
+ * ELIBBAD when it is the interpreter's. A failed allocation of the table is
+ * reported as ENOMEM, never as one of those two: checked as root with
+ * CONFIG_FAULT_INJECTION, CONFIG_FAILSLAB and CONFIG_FAULT_INJECTION_DEBUG_FS
+ * (so CONFIG_DEBUG_FS), skipped without them.
+ */
+#define _GNU_SOURCE
+#include <elf.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <link.h>
+#include <linux/magic.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mount.h>
+#include <sys/resource.h>
+#include <sys/statvfs.h>
+#include <sys/vfs.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "kselftest_harness.h"
+
+/* The test itself could not fork or arm the fault, as opposed to an errno. */
+#define SETUP_FAILED	(-2)
+
+enum table {
+	BAD_PHENTSIZE,
+	NO_PHDRS,
+	TOO_BIG,
+	HUGE_PHOFF,	/* on 32-bit only past the end of the file */
+	PAST_EOF,
+	TRUNCATED,
+};
+
+static int write_file(const char *path, const void *buf, size_t len)
+{
+	int fd = open(path, O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, 0755);
+	ssize_t ret;
+
+	if (fd < 0)
+		return -1;
+	ret = write(fd, buf, len);
+	close(fd);
+	return ret == (ssize_t)len ? 0 : -1;
+}
+
+static int read_ehdr(ElfW(Ehdr) *eh)
+{
+	int fd = open("/proc/self/exe", O_RDONLY | O_CLOEXEC);
+	ssize_t ret;
+
+	if (fd < 0)
+		return -1;
+	ret = read(fd, eh, sizeof(*eh));
+	close(fd);
+	return ret == sizeof(*eh) ? 0 : -1;
+}
+
+/* An ELF header for this machine with one program header, then that header. */
+static size_t one_phdr_image(char *buf, const ElfW(Phdr) *ph)
+{
+	ElfW(Ehdr) eh;
+
+	if (read_ehdr(&eh))
+		return 0;
+	eh.e_phoff = sizeof(eh);
+	eh.e_phentsize = sizeof(*ph);
+	eh.e_phnum = 1;
+	eh.e_shoff = 0;
+	eh.e_shnum = 0;
+	eh.e_shstrndx = 0;
+	memcpy(buf, &eh, sizeof(eh));
+	memcpy(buf + sizeof(eh), ph, sizeof(*ph));
+	return sizeof(eh) + sizeof(*ph);
+}
+
+static size_t bad_table_image(char *buf, enum table table)
+{
+	ElfW(Phdr) ph = { .p_type = PT_NULL };
+	ElfW(Ehdr) *eh = (ElfW(Ehdr) *)buf;
+	size_t len = one_phdr_image(buf, &ph);
+
+	if (!len)
+		return 0;
+	switch (table) {
+	case BAD_PHENTSIZE:
+		eh->e_phentsize--;
+		break;
+	case NO_PHDRS:
+		eh->e_phnum = 0;
+		break;
+	case TOO_BIG:
+		eh->e_phnum = 65536 / sizeof(ph) + 1;
+		break;
+	case HUGE_PHOFF:
+		eh->e_phoff = (ElfW(Off))1 << (8 * sizeof(eh->e_phoff) - 1);
+		break;
+	case PAST_EOF:
+		eh->e_phoff = 1 << 20;
+		break;
+	case TRUNCATED:
+		eh->e_phnum = 2;
+		break;
+	}
+	return len;
+}
+
+/* An executable whose PT_INTERP names @interp. */
+static size_t interp_image(char *buf, const char *interp)
+{
+	ElfW(Phdr) ph = {
+		.p_type = PT_INTERP,
+		.p_offset = sizeof(ElfW(Ehdr)) + sizeof(ElfW(Phdr)),
+		.p_filesz = strlen(interp) + 1,
+	};
+	size_t len = one_phdr_image(buf, &ph);
+
+	if (!len)
+		return 0;
+	memcpy(buf + len, interp, ph.p_filesz);
+	return len + ph.p_filesz;
+}
+
+/* Returns the errno of a failed execve(), 0 if it did not return. */
+static int exec_errno(const char *path, char *const argv[])
+{
+	char *const envp[] = { NULL };
+	int pipefd[2], err = 0, status;
+	pid_t pid;
+
+	if (pipe2(pipefd, O_CLOEXEC))
+		return SETUP_FAILED;
+	pid = fork();
+	if (pid < 0) {
+		close(pipefd[0]);
+		close(pipefd[1]);
+		return SETUP_FAILED;
+	}
+	if (!pid) {
+		execve(path, argv, envp);
+		err = errno;
+		write(pipefd[1], &err, sizeof(err));
+		_exit(1);
+	}
+	close(pipefd[1]);
+	if (read(pipefd[0], &err, sizeof(err)) != sizeof(err))
+		err = 0;
+	close(pipefd[0]);
+	waitpid(pid, &status, 0);
+	return err;
+}
+
+FIXTURE(table) {
+	char dir[64];
+	char exe[96];
+	char interp[96];
+};
+
+FIXTURE_VARIANT(table) {
+	enum table table;
+	bool interp;
+};
+
+FIXTURE_VARIANT_ADD(table, bad_phentsize) { .table = BAD_PHENTSIZE };
+FIXTURE_VARIANT_ADD(table, no_phdrs) { .table = NO_PHDRS };
+FIXTURE_VARIANT_ADD(table, too_big) { .table = TOO_BIG };
+FIXTURE_VARIANT_ADD(table, huge_phoff) { .table = HUGE_PHOFF };
+FIXTURE_VARIANT_ADD(table, past_eof) { .table = PAST_EOF };
+FIXTURE_VARIANT_ADD(table, truncated) { .table = TRUNCATED };
+FIXTURE_VARIANT_ADD(table, interp_bad_phentsize) { .table = BAD_PHENTSIZE, .interp = true };
+FIXTURE_VARIANT_ADD(table, interp_no_phdrs) { .table = NO_PHDRS, .interp = true };
+FIXTURE_VARIANT_ADD(table, interp_too_big) { .table = TOO_BIG, .interp = true };
+FIXTURE_VARIANT_ADD(table, interp_huge_phoff) { .table = HUGE_PHOFF, .interp = true };
+FIXTURE_VARIANT_ADD(table, interp_past_eof) { .table = PAST_EOF, .interp = true };
+FIXTURE_VARIANT_ADD(table, interp_truncated) { .table = TRUNCATED, .interp = true };
+
+static void remove_files(FIXTURE_DATA(table) *self)
+{
+	unlink(self->exe);
+	unlink(self->interp);
+	rmdir(self->dir);
+}
+
+FIXTURE_SETUP(table)
+{
+	char bad[sizeof(ElfW(Ehdr)) + sizeof(ElfW(Phdr))];
+	char exe[sizeof(bad) + sizeof(self->interp)];
+	size_t bad_len, exe_len;
+	struct statvfs st;
+	int ret;
+
+	ASSERT_EQ(0, statvfs("/tmp", &st));
+	if (st.f_flag & ST_NOEXEC)
+		SKIP(return, "/tmp is on a noexec mount");
+	bad_len = bad_table_image(bad, variant->table);
+	ASSERT_NE(0, bad_len);
+	strcpy(self->dir, "/tmp/load_elf_phdrs.XXXXXX");
+	ASSERT_NE(NULL, mkdtemp(self->dir));
+	snprintf(self->exe, sizeof(self->exe), "%s/exe", self->dir);
+	snprintf(self->interp, sizeof(self->interp), "%s/interp", self->dir);
+
+	if (!variant->interp) {
+		ret = write_file(self->exe, bad, bad_len);
+	} else {
+		exe_len = interp_image(exe, self->interp);
+		ret = !exe_len || write_file(self->interp, bad, bad_len) ||
+		      write_file(self->exe, exe, exe_len);
+	}
+	if (ret)
+		remove_files(self);
+	ASSERT_EQ(0, ret);
+}
+
+FIXTURE_TEARDOWN(table)
+{
+	remove_files(self);
+}
+
+TEST_F(table, execve)
+{
+	char *const argv[] = { self->exe, NULL };
+	int expect = variant->interp ? ELIBBAD : ENOEXEC;
+	int err = exec_errno(self->exe, argv);
+
+	ASSERT_NE(SETUP_FAILED, err);
+	EXPECT_EQ(expect, err) {
+		TH_LOG("execve(): %s, expected %s", strerror(err), strerror(expect));
+	}
+}
+
+#define DEBUGFS		"/sys/kernel/debug"
+#define IGNORE_GFP_WAIT	DEBUGFS "/failslab/ignore-gfp-wait"
+#define VERBOSE		DEBUGFS "/failslab/verbose"
+
+static int read_str(const char *path, char *buf, size_t size)
+{
+	int fd = open(path, O_RDONLY | O_CLOEXEC);
+	ssize_t ret;
+
+	if (fd < 0)
+		return -1;
+	ret = read(fd, buf, size - 1);
+	close(fd);
+	if (ret <= 0)
+		return -1;
+	buf[ret] = '\0';
+	return 0;
+}
+
+static int write_str(const char *path, const char *str)
+{
+	return write_file(path, str, strlen(str));
+}
+
+/* errno of execve() with fault @nth injected; 0: the new image exited, -1: it died */
+static int exec_fail_nth(const char *path, unsigned int nth)
+{
+	char *const argv[] = { (char *)path, NULL };
+	char *const envp[] = { NULL };
+	int pipefd[2], err, status;
+	char buf[16];
+	pid_t pid;
+
+	if (pipe2(pipefd, O_CLOEXEC))
+		return SETUP_FAILED;
+	pid = fork();
+	if (pid < 0) {
+		close(pipefd[0]);
+		close(pipefd[1]);
+		return SETUP_FAILED;
+	}
+	if (!pid) {
+		struct rlimit no_core = { 0, 0 };
+		int null = open("/dev/null", O_WRONLY | O_CLOEXEC);
+		int fd;
+
+		/* Faults past the point of no return kill the new image. */
+		setrlimit(RLIMIT_CORE, &no_core);
+		if (null >= 0) {
+			dup2(null, STDOUT_FILENO);
+			dup2(null, STDERR_FILENO);
+		}
+		fd = open("/proc/thread-self/fail-nth", O_WRONLY | O_CLOEXEC);
+		if (fd < 0 || write(fd, buf, sprintf(buf, "%u", nth)) < 0)
+			_exit(2);
+		execve(path, argv, envp);
+		err = errno;
+		write(fd, "0", 1);
+		write(pipefd[1], &err, sizeof(err));
+		_exit(1);
+	}
+	close(pipefd[1]);
+	if (read(pipefd[0], &err, sizeof(err)) != sizeof(err))
+		err = 0;
+	close(pipefd[0]);
+	if (waitpid(pid, &status, 0) != pid ||
+	    (WIFEXITED(status) && WEXITSTATUS(status) == 2))
+		return SETUP_FAILED;
+	if (err)
+		return err;
+	return WIFEXITED(status) && WEXITSTATUS(status) == 1 ? 0 : -1;
+}
+
+FIXTURE(alloc) {
+	char ignore_gfp_wait[8];
+	char verbose[8];
+	bool mounted;
+};
+
+static void failslab_restore(FIXTURE_DATA(alloc) *self)
+{
+	write_str(IGNORE_GFP_WAIT, self->ignore_gfp_wait);
+	write_str(VERBOSE, self->verbose);
+	if (self->mounted)
+		umount(DEBUGFS);
+}
+
+FIXTURE_SETUP(alloc)
+{
+	struct statfs st;
+	int ret;
+
+	self->mounted = false;
+	if (geteuid())
+		SKIP(return, "needs root for the failslab settings");
+	if (access("/proc/thread-self/fail-nth", W_OK))
+		SKIP(return, "no fault injection (CONFIG_FAULT_INJECTION)");
+	if (statfs(DEBUGFS, &st) || st.f_type != DEBUGFS_MAGIC)
+		self->mounted = !mount("debugfs", DEBUGFS, "debugfs", 0, NULL);
+	if (read_str(IGNORE_GFP_WAIT, self->ignore_gfp_wait, sizeof(self->ignore_gfp_wait)) ||
+	    read_str(VERBOSE, self->verbose, sizeof(self->verbose))) {
+		if (self->mounted)
+			umount(DEBUGFS);
+		SKIP(return, "no debugfs or no failslab (CONFIG_FAULT_INJECTION_DEBUG_FS)");
+	}
+	/* The allocations of an execve() may sleep, and a splat for each is noise. */
+	ret = write_str(IGNORE_GFP_WAIT, "N") ?: write_str(VERBOSE, "0");
+	if (ret) {
+		ret = errno;
+		failslab_restore(self);
+		SKIP(return, "cannot set up failslab: %s", strerror(ret));
+	}
+}
+
+FIXTURE_TEARDOWN(alloc)
+{
+	failslab_restore(self);
+}
+
+TEST_F(alloc, no_enoexec)
+{
+	unsigned int nth, clean = 0, failed = 0, wrong = 0;
+	char path[PATH_MAX];
+	ssize_t len;
+	int err;
+
+	/* Dynamically linked, so that the interpreter's table is allocated too. */
+	len = readlink("/proc/self/exe", path, sizeof(path) - sizeof("false-dynamic"));
+	ASSERT_LT(0, len);
+	path[len] = '\0';
+	strcpy(strrchr(path, '/') + 1, "false-dynamic");
+	if (access(path, X_OK))
+		SKIP(return, "false-dynamic not found next to the test");
+
+	/* Done once the faults to inject lie past the whole exec and exit. */
+	for (nth = 1; clean < 50 && nth < 100000; nth++) {
+		err = exec_fail_nth(path, nth);
+		ASSERT_NE(SETUP_FAILED, err) {
+			TH_LOG("fail-nth %u: fork or fail-nth write failed", nth);
+		}
+		if (!err) {
+			clean++;
+			continue;
+		}
+		clean = 0;
+		if (err < 0)
+			continue;
+		failed++;
+		if (err == ENOEXEC || err == ELIBBAD) {
+			wrong++;
+			TH_LOG("fail-nth %u: execve(): %s", nth, strerror(err));
+		}
+	}
+	ASSERT_EQ(50, clean) {
+		TH_LOG("no clean exec after fail-nth %u", nth);
+	}
+	if (!failed)
+		SKIP(return, "no execve() failed: failslab did not inject");
+	EXPECT_EQ(0, wrong);
+}
+
+TEST_HARNESS_MAIN

-- 
2.43.0

Re: [PATCH v2 1/2] binfmt_elf: return the real error when program headers cannot be loaded

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] binfmt_elf: missing Fixes trailer for load_elf_phdrs() error
  return
--

commit 46b1e6b996d116acfe43305abc30c04627bdcd95
Author: Konstantin Khorenko <khorenko@virtuozzo.com>

> binfmt_elf: return the real error when program headers cannot be loaded
>
> load_elf_phdrs() returns NULL on any failure, and load_elf_binary()
> turns that into -ENOEXEC for the executable and -ELIBBAD for its
> interpreter. So a failed allocation or read of the program headers
> makes glibc's execvp() run the ELF file with /bin/sh, and for the
> interpreter reports a healthy ld.so as a corrupted shared library.
>
> Return an ERR_PTR instead:
>
>   +------------+------------------------------------------------------+
>   | error      | when                                                 |
>   +------------+------------------------------------------------------+
>   | -ENOEXEC   | bad e_phentsize, e_phnum or e_phoff, or a short read |
>   | -ENOMEM    | the allocation fails                                 |
>   | read error | kernel_read() fails                                  |
>   +------------+------------------------------------------------------+
>
> Malformed files and short reads keep getting -ENOEXEC and -ELIBBAD.
> The table is read with kernel_read() directly, as elf_read() turns a
> short read into -EIO. An e_phoff past LLONG_MAX - size is rejected up
> front, so that rw_verify_area() does not turn it into -EINVAL.
>
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>

[Severity: Low]
Prior to 6a8d38945cf4 ("binfmt_elf: Hoist ELF program header loading to a
function"), load_elf_binary() propagated -ENOMEM and negative read errors
when loading program headers.  Should this include:

  Fixes: 6a8d38945cf4 ("binfmt_elf: Hoist ELF program header loading to a function")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-b4-prep-vstor-149847-binfmt_elf-enomem-load-elf-phdrs-v2-0-1c8b7b1dae5c@virtuozzo.com?part=1