Hi Paul,

On Tue, Nov 14, 2023 at 10:56:50AM -0800, Paul Pluzhnikov wrote:
> On Tue, Nov 14, 2023 at 9:55 AM Mark Wielaard <m...@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.

No worries. That is what the bots are for :)

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

Are you sure? It does for me. It also confirmed your suggestion below
fixes it.

> > 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.

O, good observation. You are right.

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

Thanks. I pushed that fix as attached.

Cheers,

Mark
>From f84f1cd7e296bf223cb45b5469978d4bea82cec0 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Tue, 14 Nov 2023 21:34:50 +0100
Subject: [PATCH] libelf: Fix elf_begin.c build on 32bit arches.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

On 32bit architectures gcc produces an error:

elf_begin.c: In function ‘file_read_elf’:
elf_begin.c:495:30: error: cast to pointer from integer of different
size [-Werror=int-to-pointer-cast]
      elf->state.elf64.shdr = (Elf64_Shdr *) (ehdr + e_shoff);
                              ^

This is because we are adding an uintptr to an Elf64_Off which
promotes the result to a 64bit value. Fix this by casting the
e_shoff to an ptrdiff_t. This is fine since the mmap of the file
would have failed if it didn't fit in the 32bit address space
and we check that e_shoff fits inside the image.

        * libelf/elf_begin.c (file_read_elf): Cast e_shoff to ptrdiff_t
        before adding to ehdr.

Suggested-by: Paul Pluzhnikov <ppluzhni...@google.com>
Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 libelf/elf_begin.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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)
            {
-- 
2.39.3

Reply via email to