This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RFC: Verify AT_ENTRY before using it


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 ();
 }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]