2021-12-12 20:41:49

by H.J. Lu

Subject: [PATCH v2] fs/binfmt_elf.c: disallow invalid entry point address

On Linux, the start of the first PT_LOAD segment is the ELF header and
the address 0 points to the ELF magic bytes. Update the ELF loader to
disallow ELF binaries with entry point address smaller than the ELF
header size. This fixes:

https://bugzilla.kernel.org/show_bug.cgi?id=215303

Tested by booting Fedora 35 and running a shared library with invalid
entry point address:

$ readelf -h load.so | grep "Entry point address:"
Entry point address: 0x4
$ ./load.so
bash: ./load.so: cannot execute binary file: Exec format error
$

Signed-off-by: H.J. Lu <[email protected]>
---
fs/binfmt_elf.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index bd78587194dc..7f035022131b 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -850,6 +850,8 @@ static int load_elf_binary(struct linux_binprm *bprm)

if (elf_ex->e_type != ET_EXEC && elf_ex->e_type != ET_DYN)
goto out;
+ if (elf_ex->e_entry < sizeof(*elf_ex))
+ goto out;
if (!elf_check_arch(elf_ex))
goto out;
if (elf_check_fdpic(elf_ex))
--
2.33.1

2021-12-13 16:41:01

by Eric W. Biederman

Subject: Re: [PATCH v2] fs/binfmt_elf.c: disallow invalid entry point address

"H.J. Lu" <[email protected]> writes:

> On Linux, the start of the first PT_LOAD segment is the ELF header and
> the address 0 points to the ELF magic bytes. Update the ELF loader to
> disallow ELF binaries with entry point address smaller than the ELF
> header size.

I kind of get why that was suggested but there is most definitely no
requirement for the program headers to be loaded let alone be at any
particular virtual address. We could be talking about a static elf
binary.

> This fixes:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=215303
>
> Tested by booting Fedora 35 and running a shared library with invalid
> entry point address:
>
> $ readelf -h load.so | grep "Entry point address:"
> Entry point address: 0x4
> $ ./load.so
> bash: ./load.so: cannot execute binary file: Exec format error
> $

Is the point of this keeping shared libraries from executing?
What is gained by this patch?

Eric


> Signed-off-by: H.J. Lu <[email protected]>
> ---
> fs/binfmt_elf.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index bd78587194dc..7f035022131b 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -850,6 +850,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
>
> if (elf_ex->e_type != ET_EXEC && elf_ex->e_type != ET_DYN)
> goto out;
> + if (elf_ex->e_entry < sizeof(*elf_ex))
> + goto out;
> if (!elf_check_arch(elf_ex))
> goto out;
> if (elf_check_fdpic(elf_ex))