[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@xxxxxxxxxxxxx>
---
 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