ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath

Jan Kratochvil jan.kratochvil@redhat.com
Sun Jul 4 10:17:00 GMT 2010


Hi Joel,

thanks for the review, so many comments were probably so pleasant to check.
I can check-in the series (or you can) in a moment for the 7.2 branching.


On Tue, 29 Jun 2010 19:42:16 +0200, Joel Brobecker wrote:
> > +		  /* PT_GNU_STACK addresses are left as zero not being
> > +		     relocated by prelink, their displacing would create false
> > +		     verification failure.  Feel free to test the unrelocated
> > +		     comparison for any segment type.  */
> 
> Can you explain differently what you are try to say?

This whole code around does not modify anything.  It just makes many sanity
checks to say if it is safe (ok left as 1) or unsafe (ok = 0) to make the
detected DISPLACEMENT relocation for PIE.

PIE before prelink had (shortened):
Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  PHDR           0x000040 0x0000000000000040 0x0000000000000040 0x0001c0 0x0001c0 R E 0x8
  LOAD           0x010b60 0x0000000000210b60 0x0000000000210b60 0x001000 0x0021b0 RW  0x200000
  GNU_STACK      0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW  0x8
and after prelink to 0x0000003000000000 it has:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  PHDR           0x000040 0x0000003000000040 0x0000003000000040 0x0001c0 0x0001c0 R E 0x8
  LOAD           0x010b60 0x0000003000210b60 0x0000003000210b60 0x001000 0x0021b0 RW  0x200000
  GNU_STACK      0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW  0x8

While PHDR + LOAD have the right offset GNU_STACK is not prelinked to expected
0x0000003000000000.

I have checked now the prelink sources for the exact rules and there is really
an exception just for PT_GNU_STACK.

OTOH the relocated/unrelocated combinations can be more complex than just none
vs. p_paddr+p_vaddr.  In practice it works now but I filed now PR 11786 for it.

I have put there now just:
                  /* PT_GNU_STACK is an exception by being never relocated by
                     prelink as its addresses are always zero.  */


> > +			    # ATTACH executables + libraries get modified since
> > +			    # they have been run.
> 
> I'm having problems understanding this sentence. Do you mean perhaps
> 
>         ATTACH means that executables and libraries have been modified
>         after they have been run.
> ?

yes


> > +                           # they have been run.  They cannot be used for
> > +                           # problem reproducibility after the testcase ends.
> 
> I would personally add a conclusion to the last sentence, explaining that
> this is the reason why you are deleting all associated binary files.

Used:
                            # They 
                            # cannot be reused for problem reproducibility after
                            # the testcase ends in the ATTACH case.  Therefore 
                            # they are rather deleted not to confuse after the
                            # run finishes.

They could be also copied for later reproducibility (although rather not
renamed - they would be still accessible via /proc/PID/exe tracking renames
due to being bound to the inode).  Currently they are just deleted.


Thanks,
Jan


gdb/
2010-07-02  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Joel Brobecker  <brobecker@adacore.com>

	Fix attaching to PIEs prelinked on the disk after the process was
	started.
	* solib-svr4.c (svr4_exec_displacement): New variable arch_size.
	Verify it against bfd_get_arch_size.  Try to match arbitrary
	displacement for the phdrs comparison.

gdb/testsuite/
2010-07-02  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Joel Brobecker  <brobecker@adacore.com>

	* gdb.base/break-interp.exp: Run $binpie with new value "ATTACH", new
	code for it.  New variable relink_args.
	(prelinkYES): Call prelinkNO.
	(test_attach): Accept new parameter relink_args.  Re-prelink the binary
	in such case.  Move the core code to ...
	(test_attach_gdb): ... a new function.  Send GDB command "file".
	Extend expected "Attaching to " string.

--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1775,13 +1775,187 @@ svr4_exec_displacement (CORE_ADDR *displacementp)
 	 really do not match.  */
       int phdrs_size, phdrs2_size, ok = 1;
       gdb_byte *buf, *buf2;
+      int arch_size;
 
-      buf = read_program_header (-1, &phdrs_size, NULL);
+      buf = read_program_header (-1, &phdrs_size, &arch_size);
       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;
+      if (buf != NULL && buf2 != NULL)
+	{
+	  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch);
+
+	  /* We are dealing with three different addresses.  EXEC_BFD
+	     represents current address in on-disk file.  target memory content
+	     may be different from EXEC_BFD as the file may have been prelinked
+	     to a different address after the executable has been loaded.
+	     Moreover the address of placement in target memory can be
+	     different from what the program headers in target memory say - this
+	     is the goal of PIE.
+
+	     Detected DISPLACEMENT covers both the offsets of PIE placement and
+	     possible new prelink performed after start of the program.  Here
+	     relocate BUF and BUF2 just by the EXEC_BFD vs. target memory
+	     content offset for the verification purpose.  */
+
+	  if (phdrs_size != phdrs2_size
+	      || bfd_get_arch_size (exec_bfd) != arch_size)
+	    ok = 0;
+	  else if (arch_size == 32 && phdrs_size >= sizeof (Elf32_External_Phdr)
+	           && phdrs_size % sizeof (Elf32_External_Phdr) == 0)
+	    {
+	      Elf_Internal_Ehdr *ehdr2 = elf_tdata (exec_bfd)->elf_header;
+	      Elf_Internal_Phdr *phdr2 = elf_tdata (exec_bfd)->phdr;
+	      CORE_ADDR displacement = 0;
+	      int i;
+
+	      /* DISPLACEMENT could be found easier by the difference of
+		 ehdr2->e_entry.  But we haven't read the ehdr yet, and we
+		 already have enough information to compute that displacement
+		 with what we've read.  */
+
+	      for (i = 0; i < ehdr2->e_phnum; i++)
+		if (phdr2[i].p_type == PT_LOAD)
+		  {
+		    Elf32_External_Phdr *phdrp;
+		    gdb_byte *buf_vaddr_p, *buf_paddr_p;
+		    CORE_ADDR vaddr, paddr;
+		    CORE_ADDR displacement_vaddr = 0;
+		    CORE_ADDR displacement_paddr = 0;
+
+		    phdrp = &((Elf32_External_Phdr *) buf)[i];
+		    buf_vaddr_p = (gdb_byte *) &phdrp->p_vaddr;
+		    buf_paddr_p = (gdb_byte *) &phdrp->p_paddr;
+
+		    vaddr = extract_unsigned_integer (buf_vaddr_p, 4,
+						      byte_order);
+		    displacement_vaddr = vaddr - phdr2[i].p_vaddr;
+
+		    paddr = extract_unsigned_integer (buf_paddr_p, 4,
+						      byte_order);
+		    displacement_paddr = paddr - phdr2[i].p_paddr;
+
+		    if (displacement_vaddr == displacement_paddr)
+		      displacement = displacement_vaddr;
+
+		    break;
+		  }
+
+	      /* Now compare BUF and BUF2 with optional DISPLACEMENT.  */
+
+	      for (i = 0; i < phdrs_size / sizeof (Elf32_External_Phdr); i++)
+		{
+		  Elf32_External_Phdr *phdrp;
+		  Elf32_External_Phdr *phdr2p;
+		  gdb_byte *buf_vaddr_p, *buf_paddr_p;
+		  CORE_ADDR vaddr, paddr;
+
+		  phdrp = &((Elf32_External_Phdr *) buf)[i];
+		  buf_vaddr_p = (gdb_byte *) &phdrp->p_vaddr;
+		  buf_paddr_p = (gdb_byte *) &phdrp->p_paddr;
+		  phdr2p = &((Elf32_External_Phdr *) buf2)[i];
+
+		  /* PT_GNU_STACK is an exception by being never relocated by
+		     prelink as its addresses are always zero.  */
+
+		  if (memcmp (phdrp, phdr2p, sizeof (*phdrp)) == 0)
+		    continue;
+
+		  /* Check also other adjustment combinations - PR 11786.  */
+
+		  vaddr = extract_unsigned_integer (buf_vaddr_p, 4, byte_order);
+		  vaddr -= displacement;
+		  store_unsigned_integer (buf_vaddr_p, 4, byte_order, vaddr);
+
+		  paddr = extract_unsigned_integer (buf_paddr_p, 4, byte_order);
+		  paddr -= displacement;
+		  store_unsigned_integer (buf_paddr_p, 4, byte_order, paddr);
+
+		  if (memcmp (phdrp, phdr2p, sizeof (*phdrp)) == 0)
+		    continue;
+
+		  ok = 0;
+		  break;
+		}
+	    }
+	  else if (arch_size == 64 && phdrs_size >= sizeof (Elf64_External_Phdr)
+	           && phdrs_size % sizeof (Elf64_External_Phdr) == 0)
+	    {
+	      Elf_Internal_Ehdr *ehdr2 = elf_tdata (exec_bfd)->elf_header;
+	      Elf_Internal_Phdr *phdr2 = elf_tdata (exec_bfd)->phdr;
+	      CORE_ADDR displacement = 0;
+	      int i;
+
+	      /* DISPLACEMENT could be found easier by the difference of
+		 ehdr2->e_entry.  But we haven't read the ehdr yet, and we
+		 already have enough information to compute that displacement
+		 with what we've read.  */
+
+	      for (i = 0; i < ehdr2->e_phnum; i++)
+		if (phdr2[i].p_type == PT_LOAD)
+		  {
+		    Elf64_External_Phdr *phdrp;
+		    gdb_byte *buf_vaddr_p, *buf_paddr_p;
+		    CORE_ADDR vaddr, paddr;
+		    CORE_ADDR displacement_vaddr = 0;
+		    CORE_ADDR displacement_paddr = 0;
+
+		    phdrp = &((Elf64_External_Phdr *) buf)[i];
+		    buf_vaddr_p = (gdb_byte *) &phdrp->p_vaddr;
+		    buf_paddr_p = (gdb_byte *) &phdrp->p_paddr;
+
+		    vaddr = extract_unsigned_integer (buf_vaddr_p, 8,
+						      byte_order);
+		    displacement_vaddr = vaddr - phdr2[i].p_vaddr;
+
+		    paddr = extract_unsigned_integer (buf_paddr_p, 8,
+						      byte_order);
+		    displacement_paddr = paddr - phdr2[i].p_paddr;
+
+		    if (displacement_vaddr == displacement_paddr)
+		      displacement = displacement_vaddr;
+
+		    break;
+		  }
+
+	      /* Now compare BUF and BUF2 with optional DISPLACEMENT.  */
+
+	      for (i = 0; i < phdrs_size / sizeof (Elf64_External_Phdr); i++)
+		{
+		  Elf64_External_Phdr *phdrp;
+		  Elf64_External_Phdr *phdr2p;
+		  gdb_byte *buf_vaddr_p, *buf_paddr_p;
+		  CORE_ADDR vaddr, paddr;
+
+		  phdrp = &((Elf64_External_Phdr *) buf)[i];
+		  buf_vaddr_p = (gdb_byte *) &phdrp->p_vaddr;
+		  buf_paddr_p = (gdb_byte *) &phdrp->p_paddr;
+		  phdr2p = &((Elf64_External_Phdr *) buf2)[i];
+
+		  /* PT_GNU_STACK is an exception by being never relocated by
+		     prelink as its addresses are always zero.  */
+
+		  if (memcmp (phdrp, phdr2p, sizeof (*phdrp)) == 0)
+		    continue;
+
+		  /* Check also other adjustment combinations - PR 11786.  */
+
+		  vaddr = extract_unsigned_integer (buf_vaddr_p, 8, byte_order);
+		  vaddr -= displacement;
+		  store_unsigned_integer (buf_vaddr_p, 8, byte_order, vaddr);
+
+		  paddr = extract_unsigned_integer (buf_paddr_p, 8, byte_order);
+		  paddr -= displacement;
+		  store_unsigned_integer (buf_paddr_p, 8, byte_order, paddr);
+
+		  if (memcmp (phdrp, phdr2p, sizeof (*phdrp)) == 0)
+		    continue;
+
+		  ok = 0;
+		  break;
+		}
+	    }
+	  else
+	    ok = 0;
+	}
 
       xfree (buf);
       xfree (buf2);
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -154,6 +154,12 @@ proc prelinkYES {arg {name ""}} {
     if {$name == ""} {
 	set name [file tail $arg]
     }
+
+    # Try to unprelink it first so that if it has been already prelinked before
+    # we get different address now and the result is not affected by the
+    # previous $arg state..
+    prelinkNO $arg "$name pre-unprelink"
+
     set test "prelink $name"
     set command "exec /usr/sbin/prelink -qNR --no-exec-shield $arg"
     verbose -log "command is $command"
@@ -325,38 +331,12 @@ proc test_core {file displacement} {
     set pf_prefix $old_ldprefix
 }
 
-proc test_attach {file displacement} {
-    global board_info gdb_prompt expect_out
-
-    gdb_exit
-
-    set test "sleep function started"
-
-    set command "${file} sleep"
-    set res [remote_spawn host $command];
-    if { $res < 0 || $res == "" } {
-	perror "Spawning $command failed."
-	fail $test
-	return
-    }
-    set pid [exp_pid -i $res]
-    gdb_expect {
-	-re "sleeping\r\n" {
-	    pass $test
-	}
-	eof {
-	    fail "$test (eof)"
-	    return
-	}
-	timeout {
-	    fail "$test (timeout)"
-	    return
-	}
-    }
+proc test_attach_gdb {file pid displacement prefix} {
+    global gdb_prompt expect_out
 
     global pf_prefix
     set old_ldprefix $pf_prefix
-    lappend pf_prefix "attach:"
+    lappend pf_prefix "$prefix:"
 
     gdb_exit
     gdb_start
@@ -364,9 +344,13 @@ proc test_attach {file displacement} {
     # Print the "PIE (Position Independent Executable) displacement" message.
     gdb_test_no_output "set verbose on"
 
+    if {$file != ""} {
+	gdb_test "file $file" "Reading symbols from .*done\\." "file"
+    }
+
     set test "attach"
     gdb_test_multiple "attach $pid" $test {
-	-re "Attaching to process $pid\r\n" {
+	-re "Attaching to (program: .*, )?process $pid\r\n" {
 	    # Missing "$gdb_prompt $" is intentional.
 	    pass $test
 	}
@@ -402,11 +386,56 @@ proc test_attach {file displacement} {
     gdb_test "bt" "#\[0-9\]+ +\[^\r\n\]*\\mlibfunc\\M\[^\r\n\]*\r\n#\[0-9\]+ +\[^\r\n\]*\\mmain\\M.*" "attach main bt"
     gdb_exit
 
-    remote_exec host "kill -9 $pid"
-
     set pf_prefix $old_ldprefix
 }
 
+proc test_attach {file displacement {relink_args ""}} {
+    global board_info
+
+    gdb_exit
+
+    set test "sleep function started"
+
+    set command "${file} sleep"
+    set res [remote_spawn host $command];
+    if { $res < 0 || $res == "" } {
+	perror "Spawning $command failed."
+	fail $test
+	return
+    }
+    set pid [exp_pid -i $res]
+    gdb_expect {
+	-re "sleeping\r\n" {
+	    pass $test
+	}
+	eof {
+	    fail "$test (eof)"
+	    return
+	}
+	timeout {
+	    fail "$test (timeout)"
+	    return
+	}
+    }
+
+    if {$relink_args == ""} {
+	test_attach_gdb "" $pid $displacement "attach"
+    } else {
+	# These could be rather passed as arguments.
+	global exec interp_saved interp
+
+	foreach relink {YES NO} {
+	    if {[prelink$relink $relink_args [file tail $exec]]
+	        && [copy $interp_saved $interp]} {
+		# /proc/PID/exe cannot be loaded as it is "EXECNAME (deleted)".
+		test_attach_gdb $exec $pid $displacement "attach-relink$relink"
+	    }
+	}
+    }
+
+    remote_exec host "kill -9 $pid"
+}
+
 proc test_ld {file ifmain trynosym displacement} {
     global srcdir subdir gdb_prompt expect_out
 
@@ -615,7 +644,10 @@ foreach ldprelink {NO YES} {
 	set old_binprefix $pf_prefix
 	foreach binprelink {NO YES} {
 	    foreach binsepdebug {NO IN SEP} {
-		foreach binpie {NO YES} {
+		# "ATTACH" is like "YES" but it is modified during run.
+		# It cannot be used for problem reproducibility after the
+		# testcase ends.
+		foreach binpie {NO YES ATTACH} {
 		    # This combination is not possible, non-PIE (fixed address)
 		    # binary cannot be prelinked to any (other) address.
 		    if {$binprelink == "YES" && $binpie == "NO"} {
@@ -634,7 +666,7 @@ foreach ldprelink {NO YES} {
 		    if {$binsepdebug != "NO"} {
 			lappend opts {debug}
 		    }
-		    if {$binpie == "YES"} {
+		    if {$binpie != "NO"} {
 			lappend opts {additional_flags=-fPIE -pie}
 		    }
 		    if {[build_executable ${test}.exp [file tail $exec] $srcfile $opts] == -1} {
@@ -682,16 +714,48 @@ foreach ldprelink {NO YES} {
 			lappend dests $dest
 		    }
 
-		    if {[prelink$binprelink "--dynamic-linker=$interp --ld-library-path=$dir $exec $interp [concat $dests]" [file tail $exec]]
+		    if {$binpie == "NO"} {
+			set displacement "NONE"
+		    } elseif {$binprelink == "NO"} {
+			set displacement "NONZERO"
+		    } else {
+			set displacement "ZERO"
+		    }
+
+		    set relink_args "--dynamic-linker=$interp --ld-library-path=$dir $exec $interp [concat $dests]"
+		    if {[prelink$binprelink $relink_args [file tail $exec]]
 		        && [copy $interp_saved $interp]} {
-			if {$binpie == "NO"} {
-			    set displacement "NONE"
-			} elseif {$binprelink == "NO"} {
-			    set displacement "NONZERO"
+			if {$binpie != "ATTACH"} {
+			    test_ld $exec 1 [expr {$binsepdebug == "NO"}] $displacement
 			} else {
-			    set displacement "ZERO"
+			    # If the file has been randomly prelinked it must
+			    # be "NONZERO".  We could see "ZERO" only if it was
+			    # unprelinked and it is now running at the same
+			    # address - which is 0 but executable can never run
+			    # at address 0.
+
+			    set displacement "NONZERO"
+			    test_attach $exec $displacement $relink_args
+
+			    # ATTACH means that executables and libraries have
+			    # been modified after they have been run.  They
+			    # cannot be reused for problem reproducibility after
+			    # the testcase ends in the ATTACH case.  Therefore
+			    # they are rather deleted not to confuse after the
+			    # run finishes.
+			    set exec_debug [system_debug_get $exec]
+			    if {$exec_debug != ""} {
+				# `file delete [glob "${exec_debug}*"]' does not work.
+				foreach f [glob "${exec_debug}*"] {
+				    file delete $f
+				}
+			    }
+			    file delete -force $dir
+			    # `file delete [glob "${exec}*"]' does not work.
+			    foreach f [glob "${exec}*"] {
+				file delete $f
+			    }
 			}
-			test_ld $exec 1 [expr {$binsepdebug == "NO"}] $displacement
 		    }
 		}
 	    }



More information about the Gdb-patches mailing list