[Devel] [PATCH hci-8.1 2/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 reports a
healthy ld.so as a corrupted shared library.

Return an ERR_PTR instead, as binfmt_elf_fdpic does:

  -ENOEXEC     bad e_phentsize, e_phnum or e_phoff, or the file ends
               before the header table
  -ENOMEM      the allocation fails
  read error   kernel_read() fails

Malformed files keep getting -ENOEXEC and -ELIBBAD. kernel_read()
returns a short count when a later page fails, so read on until the
error or the end of the file shows. An e_phoff past LLONG_MAX - size
is rejected up front, as rw_verify_area() would turn it into -EINVAL.

Backport note: the same patch is prepared for mainline. The v6.17 change
"binfmt_elf: remove the 4k limitation of program header size" is not in
this tree, so the header table is still limited to ELF_MIN_ALIGN and the
allocation in practice fails only under fault injection; the read errors
and the e_phoff check apply as they are.

Fixes: 6a8d38945cf4 ("binfmt_elf: Hoist ELF program header loading to a function")
Fixes: a9d9ef133f44 ("binfmt_elf: load interpreter program headers earlier")
https://virtuozzo.atlassian.net/browse/VSTOR-150144

Feature: fix ms/exec
Assisted-by: LLM
Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
---
 fs/binfmt_elf.c | 51 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 9190e5204152a..a5689a2844d43 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -512,39 +512,48 @@ 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 when the read reaches the end of the
+ * file, 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;
-	unsigned int size;
+	struct elf_phdr *elf_phdata;
+	unsigned int size, done;
+	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 || size > ELF_MIN_ALIGN)
-		goto out;
+	if (size == 0 || size > 65536 || size > ELF_MIN_ALIGN ||
+	    elf_ex->e_phoff > LLONG_MAX - size)
+		return ERR_PTR(-ENOEXEC);
 
 	elf_phdata = kmalloc(size, GFP_KERNEL);
 	if (!elf_phdata)
-		goto out;
-
-	/* Read in the program headers */
-	retval = elf_read(elf_file, elf_phdata, size, elf_ex->e_phoff);
+		return ERR_PTR(-ENOMEM);
 
-out:
-	if (retval) {
-		kfree(elf_phdata);
-		elf_phdata = NULL;
+	/*
+	 * Read in the program headers. A read that fails part way returns
+	 * what it got, so read on to see the error or the end of the file.
+	 */
+	pos = elf_ex->e_phoff;
+	for (done = 0; done < size; done += rv) {
+		rv = kernel_read(elf_file, (char *)elf_phdata + done,
+				 size - done, &pos);
+		if (rv <= 0) {
+			kfree(elf_phdata);
+			return ERR_PTR(rv ? rv : -ENOEXEC);
+		}
 	}
 	return elf_phdata;
 }
@@ -939,8 +948,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++) {
@@ -1044,8 +1055,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