Re: [PATCH v2] binfmt_elf: Allow .bss in any interp PT_LOAD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



(+ sam@xxxxxxxxxx from Pedro Falcato's patch)

On 2022-11-10, Kees Cook wrote:
Traditionally, only the final PT_LOAD for load_elf_interp() supported
having p_memsz > p_filesz. Recently, lld's construction of musl's
libc.so on PowerPC64 started having two PT_LOAD program headers with
p_memsz > p_filesz.

As the least invasive change possible, check for p_memsz > p_filesz for
each PT_LOAD in load_elf_interp.

Reported-by: Rich Felker <dalias@xxxxxxxx>
Link: https://maskray.me/blog/2022-11-05-lld-musl-powerpc64
Cc: Pedro Falcato <pedro.falcato@xxxxxxxxx>
Cc: Fangrui Song <maskray@xxxxxxxxxx>
Cc: Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>
Cc: Eric Biederman <ebiederm@xxxxxxxxxxxx>
Cc: linux-fsdevel@xxxxxxxxxxxxxxx
Cc: linux-mm@xxxxxxxxx
Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
---
v2: I realized we need to retain the final padding call.
v1: https://lore.kernel.org/linux-hardening/20221111055747.never.202-kees@xxxxxxxxxx/
---
fs/binfmt_elf.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 528e2ac8931f..0a24bbbef1d6 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -673,15 +673,25 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
				last_bss = k;
				bss_prot = elf_prot;
			}
+
+			/*
+			 * Clear any p_memsz > p_filesz area up to the end
+			 * of the page to wipe anything left over from the
+			 * loaded file contents.
+			 */
+			if (last_bss > elf_bss && padzero(elf_bss))

Missing {

But after fixing this, I get a musl ld.so error.

+				error = -EFAULT;
+				goto out;
+			}
		}
	}

	/*
-	 * Now fill out the bss section: first pad the last page from
-	 * the file up to the page boundary, and zero it from elf_bss
-	 * up to the end of the page.
+	 * Finally, pad the last page from the file up to the page boundary,
+	 * and zero it from elf_bss up to the end of the page, if this did
+	 * not already happen with the last PT_LOAD.
	 */
-	if (padzero(elf_bss)) {
+	if (last_bss == elf_bss && padzero(elf_bss)) {
		error = -EFAULT;
		goto out;
	}
--
2.34.1


I added a new section to https://maskray.me/blog/2022-11-05-lld-musl-powerpc64
Copying here:

To test that the kernel ELF loader can handle more RW `PT_LOAD` program headers, we can create an executable with more RW `PT_LOAD` program headers with `p_filesz < p_memsz`.
We can place a read-only section after `.bss` followed by a `SHT_NOBITS` `SHF_ALLOC|SHF_WRITE` section. The read-only section will form a read-only `PT_LOAD` while the RW section will form a RW `PT_LOAD`.

```text
#--- a.c
#include <assert.h>
#include <stdio.h>

extern const char toc[];
char nobits0[0] __attribute__((section(".nobits0")));
char nobits1[0] __attribute__((section(".nobits1")));

int main(void) {
  assert(toc[4096-1] == 0);
  for (int i = 0; i < 1024; i++)
    assert(nobits0[i] == 0);
  nobits0[0] = nobits0[1024-1] = 1;
  for (int i = 0; i < 4096; i++)
    assert(nobits1[i] == 0);
  nobits1[0] = nobits1[4096-1] = 1;

  puts("hello");
}

#--- toc.s
.section .toc,"aw",@nobits
.globl toc
toc:
.space 4096

.section .ro0,"a"; .byte 255
.section .nobits0,"aw",@nobits; .space 1024
.section .ro1,"a"; .byte 255
.section .nobits1,"aw",@nobits; .space 4096

#--- a.lds
SECTIONS { .ro0 : {} .nobits0 : {} .ro1 : {} .nobits1 : {} } INSERT AFTER .bss;
```

```sh
split-file a.txt a
path/to/musl-gcc -Wl,--dynamic-linker=/lib/libc.so a/a.c a/a.lds -o toy
```

split-file is a utility in llvm-project.




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux