* [PATCH v2 1/2] binfmt_elf: return the real error when program headers cannot be loaded
2026-11-06 20:57 [PATCH v2 0/2] binfmt_elf: return the real error when program headers cannot be loaded Konstantin Khorenko
@ 2026-11-06 20:57 ` Konstantin Khorenko
2026-11-07 20:58 ` sashiko-bot
2026-11-06 20:57 ` [PATCH v2 2/2] selftests/exec: check the errors of an unloadable program header table Konstantin Khorenko
1 sibling, 1 reply; 4+ messages in thread
From: Konstantin Khorenko @ 2026-11-06 20:57 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Kees Cook,
Paul Burton, Shuah Khan
Cc: linux-fsdevel, linux-mm, linux-kselftest, linux-kernel,
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH v2 2/2] selftests/exec: check the errors of an unloadable program header table
2026-11-06 20:57 [PATCH v2 0/2] binfmt_elf: return the real error when program headers cannot be loaded Konstantin Khorenko
2026-11-06 20:57 ` [PATCH v2 1/2] " Konstantin Khorenko
@ 2026-11-06 20:57 ` Konstantin Khorenko
1 sibling, 0 replies; 4+ messages in thread
From: Konstantin Khorenko @ 2026-11-06 20:57 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Jan Kara, Kees Cook,
Paul Burton, Shuah Khan
Cc: linux-fsdevel, linux-mm, linux-kselftest, linux-kernel,
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
^ permalink raw reply related [flat|nested] 4+ messages in thread