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 <[email protected]>
Link: https://maskray.me/blog/2022-11-05-lld-musl-powerpc64
Cc: Pedro Falcato <[email protected]>
Cc: Fangrui Song <[email protected]>
Cc: Alexander Viro <[email protected]>
Cc: Eric Biederman <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Kees Cook <[email protected]>
---
v2: I realized we need to retain the final padding call.
v1: https://lore.kernel.org/linux-hardening/[email protected]/
---
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))
+ 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
(+ [email protected] 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 <[email protected]>
>Link: https://maskray.me/blog/2022-11-05-lld-musl-powerpc64
>Cc: Pedro Falcato <[email protected]>
>Cc: Fangrui Song <[email protected]>
>Cc: Alexander Viro <[email protected]>
>Cc: Eric Biederman <[email protected]>
>Cc: [email protected]
>Cc: [email protected]
>Signed-off-by: Kees Cook <[email protected]>
>---
>v2: I realized we need to retain the final padding call.
>v1: https://lore.kernel.org/linux-hardening/[email protected]/
>---
> 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.
On Thu, Nov 10, 2022 at 11:42:34PM -0800, Fangrui Song wrote:
> (+ [email protected] 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 <[email protected]>
> > Link: https://maskray.me/blog/2022-11-05-lld-musl-powerpc64
> > Cc: Pedro Falcato <[email protected]>
> > Cc: Fangrui Song <[email protected]>
> > Cc: Alexander Viro <[email protected]>
> > Cc: Eric Biederman <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Kees Cook <[email protected]>
> > ---
> > v2: I realized we need to retain the final padding call.
> > v1: https://lore.kernel.org/linux-hardening/[email protected]/
> > ---
> > 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
On 2022-11-11, Kees Cook wrote:
>On Thu, Nov 10, 2022 at 11:42:34PM -0800, Fangrui Song wrote:
>> (+ [email protected] 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 <[email protected]>
>> > Link: https://maskray.me/blog/2022-11-05-lld-musl-powerpc64
>> > Cc: Pedro Falcato <[email protected]>
>> > Cc: Fangrui Song <[email protected]>
>> > Cc: Alexander Viro <[email protected]>
>> > Cc: Eric Biederman <[email protected]>
>> > Cc: [email protected]
>> > Cc: [email protected]
>> > Signed-off-by: Kees Cook <[email protected]>
>> > ---
>> > v2: I realized we need to retain the final padding call.
>> > v1: https://lore.kernel.org/linux-hardening/[email protected]/
>> > ---
>> > 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.
Sorry for the unclear description. I rewrite it.
(`char nobits0[0] __attribute__((section(".nobits0")));` is not effective. It's SHT_PROGBITS and makes the output section SHT_PROGBITS.
The new example addresses the deficiency.)
Create some files. If you have split-file (a [test utility](https://llvm.org/docs/TestingGuide.html#extra-files) from llvm-project), you may place the following content into `a.txt`.
```text
#--- a.c
#include <assert.h>
#include <stdio.h>
extern const char toc[];
extern char nobits0[], nobits1[];
int main(void) {
assert(toc[4096-1] == 0);
for (int i = 0; i < 1024; i++) {
assert(nobits0[i] == 0);
nobits0[i] = 1;
}
for (int i = 0; i < 8192; i++) {
assert(nobits1[i] == 0);
nobits1[i] = 1;
}
puts("hello");
}
#--- toc.s
.globl toc, nobits0, nobits1
.section .toc,"aw",@nobits; toc: .space 4096
.section .ro0,"a"; .byte 255
.section .nobits0,"aw",@nobits; nobits0: .space 1024
.section .ro1,"a"; .byte 255
.section .nobits1,"aw",@nobits; nobits1: .space 8192
#--- a.lds
SECTIONS { .ro0 : {} .nobits0 : {} .ro1 : {} .nobits1 : {} } INSERT AFTER .bss;
```
Then run:
```sh
split-file a.txt a
path/to/musl-gcc -Wl,--dynamic-linker=/lib/libc.so a/a.c a/a.lds -o toy
```
Note: when a `SHT_NOBITS` section is followed by another section, the `SHT_NOBITS` section behaves as if it occupies the file offset range. This is because ld.lld does not implement a file size optimization.
For this simple example, using glibc based gcc works as well (musl provides __assert_fail and puts referenced by the executable):
gcc -Wl,--dynamic-linker=/lib/libc.so a/a.c a/a.lds -o toy