[PATCH] Fix computations with (potentially) NULL pointer

[Paul Pluzhnikov]

On Tue, Nov 14, 2023 at 9:55 AM Mark Wielaard <mark@klomp.org> wrote:

> Unfortunately our 32bit buildbots were also very quick to point out an
> issue: https://builder.sourceware.org/buildbot/#/changes/35202

Sorry about the break.

I just tried "./configure "CC=gcc -m32" "CXX=g++ -m32" and that didn't
reproduce the failure.

> Which does expose an interesting issue that (theoretically) mmaped
> 64bit Elf files cannot be used on 32bit architectures... hohum.

The failure here would be when map_addr ends up high in memory, and
e_shoff is so large that it causes a wrap around.

Section headers do tend to be near the end of the ELF. One of our
large 64-bit binaries (3.6GiB in size) has e_shoff == 3803727624, so
the overflow seems very likely here ... except the mmap would have
failed already, because the mmap covers the entire file. So I think
the overflow can not happen in practice.

If that's true, we can cast e_shoff to ptrdiff_t to suppress the warning.

diff --git a/libelf/elf_begin.c b/libelf/elf_begin.c
index 9f8196b6..dcaad8ee 100644
--- a/libelf/elf_begin.c
+++ b/libelf/elf_begin.c
@@ -492,7 +492,7 @@ file_read_elf (int fildes, void *map_address,
unsigned char *e_ident,
            goto free_and_out;

          if (scncnt > 0)
-           elf->state.elf64.shdr = (Elf64_Shdr *) (ehdr + e_shoff);
+           elf->state.elf64.shdr = (Elf64_Shdr *) (ehdr + (ptrdiff_t) e_shoff);

          for (size_t cnt = 0; cnt < scncnt; ++cnt)
            {


-- 
Paul Pluzhnikov