From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gnu.wildebeest.org (gnu.wildebeest.org [45.83.234.184]) by sourceware.org (Postfix) with ESMTPS id 1F2463858D39 for ; Tue, 14 Nov 2023 20:52:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1F2463858D39 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=klomp.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=klomp.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 1F2463858D39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=45.83.234.184 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699995143; cv=none; b=LlpeFUhyYyM0YoDlcYRzIbctpqn6vD9JjpcZfg6XAynMI+UryFJ9JzUc3BzCdMnuH/QproJh68Bp99CKlyQvHUhc87ovBUxKOUecZz/MtQux7wbeBjn2qbAqZ6AqNaOws4HYFCKDCL2pq2CrPpE+VUJq+8z7n5BZhFKgAen6mCk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699995143; c=relaxed/simple; bh=LDIFrLvbputLHtzk+BvbOrK+eHHxNdk9rYcVV2OVtkg=; h=Date:From:To:Subject:Message-ID:MIME-Version; b=AC5MrfVhU4AN3I9prTMz2FjI14fTNKbiMJRd39gPY4kyeUoVHKines0zXDtYLsWVrYviFkVjSIT6SJVZ/elazBv1HClg7efbz7+T1ZnoDV5aL5PQXn8tKIVfwk7UeqLltn8XlezJO4DBCnIr55ZQAiEYpRpyugI7Ey22GF7su94= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by gnu.wildebeest.org (Postfix, from userid 1000) id E3EB6302FDDB; Tue, 14 Nov 2023 21:52:20 +0100 (CET) Date: Tue, 14 Nov 2023 21:52:20 +0100 From: Mark Wielaard To: Paul Pluzhnikov Cc: elfutils-devel@sourceware.org, nafi@google.com, maennich@google.com Subject: Re: [PATCH] Fix computations with (potentially) NULL pointer Message-ID: <20231114205220.GT31613@gnu.wildebeest.org> References: <20231113225835.4083255-2-ppluzhnikov@google.com> <1753a9db4e9b8fd4efe1b2cb8bf330c69d6f4657.camel@klomp.org> <0dcd4ebd0c41044efaea3a5b38c605bb79bc5b61.camel@klomp.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="IS0zKkzwUGydFO0o" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Status: No, score=-3034.1 required=5.0 tests=BAYES_00,GIT_PATCH_0,JMQ_SPF_NEUTRAL,KAM_DMARC_STATUS,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: --IS0zKkzwUGydFO0o Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit 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 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 --IS0zKkzwUGydFO0o Content-Type: text/plain; charset=utf-8 Content-Disposition: attachment; filename="0001-libelf-Fix-elf_begin.c-build-on-32bit-arches.patch" Content-Transfer-Encoding: 8bit >From f84f1cd7e296bf223cb45b5469978d4bea82cec0 Mon Sep 17 00:00:00 2001 From: Mark Wielaard 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 Signed-off-by: Mark Wielaard --- 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 --IS0zKkzwUGydFO0o--