Re: RFC: Verify AT_ENTRY before using it

Daniel Jacobowitz
2010-02-26T21:12:29Z

On Thu, Feb 25, 2010 at 11:16:20PM +0100, Jan Kratochvil wrote:
> > Any comments, or shall I commit this?
>
> I agree with the patch, I will rebase the "displacement #2" on top of it with
> some additional one to make "gdb /path/to/this/loader" working.

Thanks!

> > +static gdb_byte *
> > +bfd_read_program_headers (bfd *abfd, int *phdrs_size)
>
> (besides missing comment)
> /* Return newly allocated memory with ELF program headers content. Store the
> allocated size in *PHDRS_SIZE, PHDRS_SIZE must not be NULL. */
>
> Should GDB ever use symbols starting "bfd_*"? They may clash with bfd/ files.

Yeah, good point.

> > +{
> > + Elf_Internal_Ehdr *ehdr;
> > + gdb_byte *buf;
> > +
> > + ehdr = elf_elfheader (abfd);
>
> I miss here
> if (bfd_get_flavour (abfd) != bfd_target_elf_flavour)
> return NULL;

Can we get here without ELF? I guess we can, since other places in
the file check. We'll never get this far, since auxilliary vectors
are ELF-specific, so I'll put the check earlier.

>
>
> > + int phdrs_size, phdrs2_size, ok = 0;
> ...
> > + buf = read_program_header (-1, &phdrs_size, NULL);
> > + buf2 = bfd_read_program_headers (exec_bfd, &phdrs2_size);
> > + if (buf != NULL && buf2 != NULL
> > + && phdrs_size == phdrs2_size
> > + && memcmp (buf, buf2, phdrs_size) == 0)
> > + ok = 1;
> > + xfree (buf);
> > + xfree (buf2);
> > +
> > + if (ok)
> > + return entry_point - bfd_get_start_address (exec_bfd);
> > + }
> >
> > return svr4_static_exec_displacement ();
>
> If the comparison cannot be made (such as on non-ELF files - are they really
> sometimes used with solib-svr4.c?) the code is pessimistic and rather does NOT
> use the PIE displacement. Just a statement, unaware of non-PIE targets.

I meant to be conservative and do it the other way, but I botched the
check. Fixed in this version.

--
Daniel Jacobowitz
CodeSourcery

2010-02-26 Daniel Jacobowitz <dan@codesourcery.com>

* solib-svr4.c (read_program_header): Support type == -1 to read
all program headers.
(read_program_headers_from_bfd): New function.
(svr4_exec_displacement): Verify that AT_ENTRY is associated with
exec_bfd.

Index: solib-svr4.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-svr4.c,v
retrieving revision 1.125
diff -u -p -r1.125 solib-svr4.c
--- solib-svr4.c 24 Feb 2010 00:29:02 -0000 1.125
+++ solib-svr4.c 26 Feb 2010 21:10:28 -0000
@@ -451,6 +451,9 @@ bfd_lookup_symbol (bfd *abfd, char *symn
/* Read program header TYPE from inferior memory. The header is found
by scanning the OS auxillary vector.

+ If TYPE == -1, return the program headers instead of the contents of
+ one program header.
+
Return a pointer to allocated memory holding the program header contents,
or NULL on failure. If sucessful, and unless P_SECT_SIZE is NULL, the
size of those contents is returned to P_SECT_SIZE. Likewise, the target
@@ -483,8 +486,13 @@ read_program_header (int type, int *p_se
else
return 0;

- /* Find .dynamic section via the PT_DYNAMIC PHDR. */
- if (arch_size == 32)
+ /* Find the requested segment. */
+ if (type == -1)
+ {
+ sect_addr = at_phdr;
+ sect_size = at_phent * at_phnum;
+ }
+ else if (arch_size == 32)
{
Elf32_External_Phdr phdr;
int i;
@@ -1669,6 +1677,32 @@ svr4_static_exec_displacement (void)
return 0;
}

+/* Read the ELF program headers from ABFD. Return the contents and
+ set *PHDRS_SIZE to the size of the program headers. */
+
+static gdb_byte *
+read_program_headers_from_bfd (bfd *abfd, int *phdrs_size)
+{
+ Elf_Internal_Ehdr *ehdr;
+ gdb_byte *buf;
+
+ ehdr = elf_elfheader (abfd);
+
+ *phdrs_size = ehdr->e_phnum * ehdr->e_phentsize;
+ if (*phdrs_size == 0)
+ return NULL;
+
+ buf = xmalloc (*phdrs_size);
+ if (bfd_seek (abfd, ehdr->e_phoff, SEEK_SET) != 0
+ || bfd_bread (buf, *phdrs_size, abfd) != *phdrs_size)
+ {
+ xfree (buf);
+ return NULL;
+ }
+
+ return buf;
+}
+
/* We relocate all of the sections by the same amount. This
behavior is mandated by recent editions of the System V ABI.
According to the System V Application Binary Interface,
@@ -1702,7 +1736,40 @@ svr4_exec_displacement (void)
return 0;

if (target_auxv_search (&current_target, AT_ENTRY, &entry_point) == 1)
- return entry_point - bfd_get_start_address (exec_bfd);
+ {
+ /* Verify that the auxilliary vector describes the same file as
+ exec_bfd, by comparing their program headers. If the program
+ headers in the auxilliary vector do not match the program
+ headers in the executable, then we are looking at a different
+ file than the one used by the kernel - for instance, "gdb
+ program" connected to "gdbserver :PORT ld.so program". */
+ int phdrs_size, phdrs2_size, ok = 1;
+ gdb_byte *buf, *buf2;
+
+ /* Take a shortcut for the common case. If the entry addresses
+ match, then it is incredibly unlikely that anything
+ complicated has happened. It's not impossible, if the loader
+ and executable are both PIE, but it would still require a
+ rare conjunction of load addresses. */
+ if (entry_point == bfd_get_start_address (exec_bfd))
+ return 0;
+
+ if (bfd_get_flavour (abfd) == bfd_target_elf_flavour)
+ {
+ buf = read_program_header (-1, &phdrs_size, NULL);
+ buf2 = read_program_headers_from_bfd (exec_bfd, &phdrs2_size);
+ if (buf != NULL && buf2 != NULL
+ && (phdrs_size != phdrs2_size
+ || memcmp (buf, buf2, phdrs_size) != 0))
+ ok = 0;
+ }
+
+ xfree (buf);
+ xfree (buf2);
+
+ if (ok)
+ return entry_point - bfd_get_start_address (exec_bfd);
+ }

return svr4_static_exec_displacement ();
}