On Thu, Nov 10, 2022 at 11:42:34PM -0800, Fangrui Song wrote: > (+ 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. Where is a.txt? Also, it'd be nice to have this without needing the musl-gcc. -- Kees Cook
