Re: [PATCH v4 3/7] bfd: Improve nm and objdump without section header

[Alan Modra <amodra@gmail.com>]

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