From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x12b.google.com (mail-il1-x12b.google.com [IPv6:2607:f8b0:4864:20::12b]) by sourceware.org (Postfix) with ESMTPS id 67A193858C41; Thu, 13 Jul 2023 05:02:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 67A193858C41 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-il1-x12b.google.com with SMTP id e9e14a558f8ab-34642952736so1075685ab.3; Wed, 12 Jul 2023 22:02:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689224549; x=1691816549; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=mxZI4YUh1UVWint51vD4VEYnkyFBi/QatOT5cdD/mHY=; b=SnEgZQypaCGHRvAf2eOzHwT2GICoSHN8EpKPpLKIG4yQaL34SS6oVllZXWoqpI+Ys7 Ys0HrVQY29fCRWXCY+jMZ7LWjKeAbJHu5/BhCp7ByOLqfNrgm8K7OMFkj4z566598lYq 8mcT72Gt0wXQWtcAkTu3ybvZLGSTldCYMwZx8PK25/25gvYNBDs+rceUZ5to8Oy+PGml uE0d6ncBsXiUkOtBYhZ5HMUgPwUkqFTi10ScQWd2IxqYqGZfkqh8fES/kLgyKGGwCyDG 4JFw7gFukq4ZtrbGKyNJ1IlCzeU5GBXM7c1nWPYqkiNh3vLGTJtoPXXB68Vy0fXheQK+ EiDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689224549; x=1691816549; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=mxZI4YUh1UVWint51vD4VEYnkyFBi/QatOT5cdD/mHY=; b=K1R5jWxskk4zcVVtu7KnMFCSIU5HRGbjKzcDf6/AfCmCOfI8l3GucN+YvrXWSPHAqY ks7sEydpBFkN+/8asvu/rx2oKk65RGDjCxZLEBZX5g8wstJjrKefT059kwccW95e2+l+ NG30QLXHEc0VlwogfOrFLWr9neLpSTHikboR9Y6912enAqL22+AQbNYXzcTCVUOPD6oR Q0PwTydUY75JNhVJX1rIDZR9TBWmttH6QFZ4DsBoB2QvJ/DWk1Kry2qqhJ8DbbCgRy0u iYaHBuxaZfFzevH4U7uGx4XwBQPRFrqkixIOsNXF1odxwQnQOfC+MPCXvfAyCC3ONOb1 TgPQ== X-Gm-Message-State: ABy/qLaG5m/VCNmZZWaIv92Mwq52FMFRAHbQBUSyaosrzi3K+OnT2fEM yOGZdDhA8xiXgzatW1Fe61k= X-Google-Smtp-Source: APBJJlGdRUpjq+NW8kdt7KzhHFfYT07knyzRLo9IMJrGXsctdTvqIy+ds954ILAEU547ywDLmUAXgg== X-Received: by 2002:a92:cd0d:0:b0:346:28e:ebf with SMTP id z13-20020a92cd0d000000b00346028e0ebfmr592254iln.2.1689224549413; Wed, 12 Jul 2023 22:02:29 -0700 (PDT) Received: from squeak.grove.modra.org ([2406:3400:51d:8cc0:e582:971b:6a6c:77a9]) by smtp.gmail.com with ESMTPSA id p2-20020aa78602000000b006687b4f2044sm4458305pfn.164.2023.07.12.22.02.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 12 Jul 2023 22:02:28 -0700 (PDT) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id D11211142A59; Thu, 13 Jul 2023 14:32:25 +0930 (ACST) Date: Thu, 13 Jul 2023 14:32:25 +0930 From: Alan Modra To: Simon Marchi Cc: "H.J. Lu" , binutils@sourceware.org, Florian Weimer , Kaylee Blake , "gdb-patches@sourceware.org" Subject: Re: [PATCH v4 3/7] bfd: Improve nm and objdump without section header Message-ID: References: <20230606175846.399377-1-hjl.tools@gmail.com> <20230606175846.399377-4-hjl.tools@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-3033.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,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: On Sun, Jul 09, 2023 at 11:30:01PM -0400, Simon Marchi wrote: > > > It works for me: > > > > $ make check TESTS="gdb.base/eu-strip-infcall.exp" > > .... > > === gdb Summary === > > > > # of expected passes 1 > > > > My change only impacts files without section header. eu-strip-infcall.exp does > > "eu-strip -f ${binfile}.debug $binfile", which doesn't remove section header. > > > > I can reliably reproduce the problem on two separate machine, one Ubuntu > 22.04 and one failrly up to date Arch Linux. elfutils version 0.186 and > 0.189, respectively. > > It goes wrong when GDB does a bfd_check_format call on > testsuite/outputs/gdb.base/eu-strip-infcall/eu-strip-infcall.debug. > Before you commit it works, and after your commit it returns false. It > happens in this new statement added to elf_object_p, added by the commit: > > if ((i_phdr->p_offset + i_phdr->p_filesz) > filesize) > goto got_no_match; > > (top-gdb) p i_phdr->p_offset > $1 = 8192 > (top-gdb) p i_phdr->p_filesz > $2 = 196 > (top-gdb) p filesize > $3 = 5104 > (top-gdb) p i > $4 = 4 > > It would be this program header causing the condition to fail: > > Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align > ... > LOAD 0x002000 0x0000000000002000 0x0000000000002000 0x0000c4 0x0000c4 R 0x1000 > > So, the program header of the .debug file describes the segments of the > main binary, not sure if that's expected. No, that's not expected. Program headers in a .debug file ought to describe the contents of the debug file. You'll typically see many with p_filesz zero. eu-strip appears to be broken in this respect. There is another problem with the code added to elf_object_p: _bfd_elf_get_dynamic_symbols is told that it can access up to e_phnum program headers, but they very likely haven't all been swapped in. I'm going to apply the following patch. ---- elf_object_p load of dynamic symbols This fixes an uninitialised memory access on a fuzzed file: 0 0xf22e9b in offset_from_vma /src/binutils-gdb/bfd/elf.c:1899:2 1 0xf1e90f in _bfd_elf_get_dynamic_symbols /src/binutils-gdb/bfd/elf.c:2099:13 2 0x10e6a54 in bfd_elf32_object_p /src/binutils-gdb/bfd/elfcode.h:851:9 Hopefully it will also stop any attempt to load dynamic symbols from eu-strip debug files. * elfcode.h (elf_object_p): Do not attempt to load dynamic symbols for a file with no section headers until all the program headers are swapped in. Do not fail on eu-strip debug files. diff --git a/bfd/elfcode.h b/bfd/elfcode.h index aae66bcebf8..b2277921680 100644 --- a/bfd/elfcode.h +++ b/bfd/elfcode.h @@ -819,6 +819,7 @@ elf_object_p (bfd *abfd) goto got_no_match; if (bfd_seek (abfd, (file_ptr) i_ehdrp->e_phoff, SEEK_SET) != 0) goto got_no_match; + bool eu_strip_broken_phdrs = false; i_phdr = elf_tdata (abfd)->phdr; for (i = 0; i < i_ehdrp->e_phnum; i++, i_phdr++) { @@ -839,21 +840,31 @@ elf_object_p (bfd *abfd) abfd->read_only = 1; } } - if (i_phdr->p_filesz != 0) - { - if ((i_phdr->p_offset + i_phdr->p_filesz) > filesize) - goto got_no_match; - /* Try to reconstruct dynamic symbol table from PT_DYNAMIC - segment if there is no section header. */ - if (i_phdr->p_type == PT_DYNAMIC - && i_ehdrp->e_shstrndx == 0 - && i_ehdrp->e_shoff == 0 - && !_bfd_elf_get_dynamic_symbols (abfd, i_phdr, - elf_tdata (abfd)->phdr, - i_ehdrp->e_phnum, - filesize)) - goto got_no_match; - } + /* Detect eu-strip -f debug files, which have program + headers that describe the original file. */ + if (i_phdr->p_filesz != 0 + && (i_phdr->p_filesz > filesize + || i_phdr->p_offset > filesize - i_phdr->p_filesz)) + eu_strip_broken_phdrs = true; + } + if (!eu_strip_broken_phdrs + && i_ehdrp->e_shoff == 0 + && i_ehdrp->e_shstrndx == 0) + { + /* Try to reconstruct dynamic symbol table from PT_DYNAMIC + segment if there is no section header. */ + i_phdr = elf_tdata (abfd)->phdr; + for (i = 0; i < i_ehdrp->e_phnum; i++, i_phdr++) + if (i_phdr->p_type == PT_DYNAMIC) + { + if (i_phdr->p_filesz != 0 + && !_bfd_elf_get_dynamic_symbols (abfd, i_phdr, + elf_tdata (abfd)->phdr, + i_ehdrp->e_phnum, + filesize)) + goto got_no_match; + break; + } } } -- Alan Modra Australia Development Lab, IBM