* [patch 1/6]: PIE: Attach binary even after re-prelinked underneath [repost]
2010-03-29 16:30 [patch 1/6]: PIE: Attach binary even after re-prelinked underneath Jan Kratochvil
@ 2010-03-29 16:15 ` Jan Kratochvil
2010-06-09 15:08 ` ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath Jan Kratochvil
1 sibling, 0 replies; 9+ messages in thread
From: Jan Kratochvil @ 2010-03-29 16:15 UTC (permalink / raw)
To: gdb-patches
[ the first mail got lost somewhere, trying again ]
Hi,
there is a regression (against previous unreleased commits) by:
Re: RFC: Verify AT_ENTRY before using it
http://sourceware.org/ml/gdb-patches/2010-03/msg00395.html
for loading PIE executables which have changed on the disk since started.
There are in fact 3 different addresses one has to properly deal with.
This patch uses explicit "file" so it is not dependent on pending:
[patch] Attach to running but deleted executable
http://sourceware.org/ml/gdb-patches/2010-03/msg00950.html
The two copy-pasted blocks for elf32 and elf64 are "not nice" but this is the
current style in GDB.
No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu for the whole
patch series together.
Thanks,
Jan
gdb/
2010-03-29 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix attaching to PIEs prelinked on the disk since their start.
* 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-03-29 Jan Kratochvil <jan.kratochvil@redhat.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
@@ -1750,13 +1750,183 @@ 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 since the executable has been loaded.
+ Moreover the address of placement in target memory can be
+ different from what say the target memory program headers - this
+ is the goal of PIE.
+
+ Detected DISPLACEMENT covers both the offsets of PIE placement and
+ possible new prelink since 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 already read BUF does not contain ehdr. */
+
+ 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 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. */
+
+ if (memcmp (phdrp, phdr2p, sizeof (*phdrp)) == 0)
+ continue;
+
+ 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 already read BUF does not contain ehdr. */
+
+ 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 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. */
+
+ if (memcmp (phdrp, phdr2p, sizeof (*phdrp)) == 0)
+ continue;
+
+ 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"
@@ -319,38 +325,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
@@ -358,9 +338,13 @@ proc test_attach {file displacement} {
# Print the "PIE (Position Independent Executable) displacement" message.
gdb_test "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
}
@@ -396,11 +380,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
@@ -609,7 +638,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"} {
@@ -628,7 +660,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} {
@@ -680,16 +712,45 @@ 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 na 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 executables + libraries get modified since
+ # they have been run. They cannot be used for
+ # problem reproducibility after the testcase ends.
+ 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
}
}
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* [patch 1/6]: PIE: Attach binary even after re-prelinked underneath
@ 2010-03-29 16:30 Jan Kratochvil
2010-03-29 16:15 ` [patch 1/6]: PIE: Attach binary even after re-prelinked underneath [repost] Jan Kratochvil
2010-06-09 15:08 ` ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath Jan Kratochvil
0 siblings, 2 replies; 9+ messages in thread
From: Jan Kratochvil @ 2010-03-29 16:30 UTC (permalink / raw)
To: gdb-patches
Hi,
there is a regression (against previous unreleased commits) by:
Re: RFC: Verify AT_ENTRY before using it
http://sourceware.org/ml/gdb-patches/2010-03/msg00395.html
for loading PIE executables which have changed on the disk since started.
There are in fact 3 different addresses one has to properly deal with.
This patch uses explicit "file" so it is not dependent on pending:
[patch] Attach to running but deleted executable
http://sourceware.org/ml/gdb-patches/2010-03/msg00950.html
The two copy-pasted blocks for elf32 and elf64 are "not nice" but this is the
current style in GDB.
No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu for the whole
patch series together.
Thanks,
Jan
gdb/
2010-03-29 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix attaching to PIEs prelinked on the disk since their start.
* 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-03-29 Jan Kratochvil <jan.kratochvil@redhat.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
@@ -1750,13 +1750,183 @@ 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 since the executable has been loaded.
+ Moreover the address of placement in target memory can be
+ different from what say the target memory program headers - this
+ is the goal of PIE.
+
+ Detected DISPLACEMENT covers both the offsets of PIE placement and
+ possible new prelink since 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 already read BUF does not contain ehdr. */
+
+ 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 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. */
+
+ if (memcmp (phdrp, phdr2p, sizeof (*phdrp)) == 0)
+ continue;
+
+ 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 already read BUF does not contain ehdr. */
+
+ 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 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. */
+
+ if (memcmp (phdrp, phdr2p, sizeof (*phdrp)) == 0)
+ continue;
+
+ 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"
@@ -319,38 +325,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
@@ -358,9 +338,13 @@ proc test_attach {file displacement} {
# Print the "PIE (Position Independent Executable) displacement" message.
gdb_test "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
}
@@ -396,11 +380,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
@@ -609,7 +638,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"} {
@@ -628,7 +660,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} {
@@ -680,16 +712,45 @@ 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 na 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 executables + libraries get modified since
+ # they have been run. They cannot be used for
+ # problem reproducibility after the testcase ends.
+ 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
}
}
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath
2010-03-29 16:30 [patch 1/6]: PIE: Attach binary even after re-prelinked underneath Jan Kratochvil
2010-03-29 16:15 ` [patch 1/6]: PIE: Attach binary even after re-prelinked underneath [repost] Jan Kratochvil
@ 2010-06-09 15:08 ` Jan Kratochvil
2010-06-29 17:49 ` Joel Brobecker
1 sibling, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2010-06-09 15:08 UTC (permalink / raw)
To: gdb-patches
Hi,
originally posted as:
[patch 1/6]: PIE: Attach binary even after re-prelinked underneath
http://sourceware.org/ml/gdb-patches/2010-03/msg01008.html
Rediffed only.
------------------------------------------------------------------------------
there is a regression (against previous unreleased commits) by:
Re: RFC: Verify AT_ENTRY before using it
http://sourceware.org/ml/gdb-patches/2010-03/msg00395.html
for loading PIE executables which have changed on the disk since started.
There are in fact 3 different addresses one has to properly deal with.
This patch uses explicit "file" so it is not dependent on pending:
[patch] Attach to running but deleted executable
http://sourceware.org/ml/gdb-patches/2010-03/msg00950.html
The two copy-pasted blocks for elf32 and elf64 are "not nice" but this is the
current style in GDB.
No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu for the whole
patch series together.
Thanks,
Jan
gdb/
2010-03-29 Jan Kratochvil <jan.kratochvil@redhat.com>
Fix attaching to PIEs prelinked on the disk since their start.
* 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-03-29 Jan Kratochvil <jan.kratochvil@redhat.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,183 @@ 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 since the executable has been loaded.
+ Moreover the address of placement in target memory can be
+ different from what say the target memory program headers - this
+ is the goal of PIE.
+
+ Detected DISPLACEMENT covers both the offsets of PIE placement and
+ possible new prelink since 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 already read BUF does not contain ehdr. */
+
+ 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 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. */
+
+ if (memcmp (phdrp, phdr2p, sizeof (*phdrp)) == 0)
+ continue;
+
+ 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 already read BUF does not contain ehdr. */
+
+ 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 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. */
+
+ if (memcmp (phdrp, phdr2p, sizeof (*phdrp)) == 0)
+ continue;
+
+ 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"
@@ -320,38 +326,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
@@ -359,9 +339,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
}
@@ -397,11 +381,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
@@ -610,7 +639,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"} {
@@ -629,7 +661,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} {
@@ -677,16 +709,45 @@ 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 na 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 executables + libraries get modified since
+ # they have been run. They cannot be used for
+ # problem reproducibility after the testcase ends.
+ 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
}
}
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath
2010-06-09 15:08 ` ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath Jan Kratochvil
@ 2010-06-29 17:49 ` Joel Brobecker
2010-07-04 10:17 ` Jan Kratochvil
0 siblings, 1 reply; 9+ messages in thread
From: Joel Brobecker @ 2010-06-29 17:49 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
> gdb/
> 2010-03-29 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> Fix attaching to PIEs prelinked on the disk since their start.
> * 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-03-29 Jan Kratochvil <jan.kratochvil@redhat.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.
OK with a few editorial changes: Instead of saying "*since* [process]
started", can you use "*after* the process was started". That would make
things a little clearer for me. I'll just highlight areas where I think
the change should be made.
> + /* 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 since the executable has been loaded.
^^^^^ after
> + Moreover the address of placement in target memory can be
> + different from what say the target memory program headers - this
what the program headers in target memory say
> + is the goal of PIE.
> + Detected DISPLACEMENT covers both the offsets of PIE placement and
> + possible new prelink since start of the program. Here relocate
^^^^^ performed after (?)
> + /* DISPLACEMENT could be found easier by the difference of
^^^^^^ more easily
> + ehdr2->e_entry but already read BUF does not contain ehdr. */
"already read BUF" is a bit terse and sounds like incomplete English
to me (I am not a specialist, though). Is the "already read" part the
important part? I think we need to explain why we use the more
complicated route. For instance, we could say something like this:
/* 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. */
> + /* 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?
> - set displacement "ZERO"
> + # If the file has been randomly prelinked it must
> + # be "NONZERO". We could see "ZERO" only if it was
> + # unprelinked na it is now running at the same
^^
> + # 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.
?
> + # 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.
And I use "reused" instead of "used", to make it clearer that the binaries
are saved in order to help reproduce issues found by this testcase.
--
Joel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath
2010-06-29 17:49 ` Joel Brobecker
@ 2010-07-04 10:17 ` Jan Kratochvil
2010-07-05 17:06 ` Joel Brobecker
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2010-07-04 10:17 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
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
}
}
}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath
2010-07-04 10:17 ` Jan Kratochvil
@ 2010-07-05 17:06 ` Joel Brobecker
2010-07-05 17:22 ` Jan Kratochvil
0 siblings, 1 reply; 9+ messages in thread
From: Joel Brobecker @ 2010-07-05 17:06 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
> 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.
This is OK, with one English error in one of my suggestions (mea culpa).
I also still have a question, but this should not prevent the patch to
go in as is.
> + /* DISPLACEMENT could be found easier by the difference of
^^^^^^ more easily
Same thing for the 64bit case.
> + /* PT_GNU_STACK is an exception by being never relocated by
> + prelink as its addresses are always zero. */
I understand why you mean, now, about the PT_GNU_STACK entry not being
changed during the prelink. But I don't get the relationship between
this comment and the code surrounding it. Can you explain that?
--
Joel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath
2010-07-05 17:06 ` Joel Brobecker
@ 2010-07-05 17:22 ` Jan Kratochvil
2010-07-05 17:49 ` Joel Brobecker
0 siblings, 1 reply; 9+ messages in thread
From: Jan Kratochvil @ 2010-07-05 17:22 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On Mon, 05 Jul 2010 19:06:02 +0200, Joel Brobecker wrote:
> > 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.
>
> This is OK, with one English error in one of my suggestions (mea culpa).
The "easier" -> "more easily" one? My fault, I forgot to include this fix by
you, sorry.
> > + /* PT_GNU_STACK is an exception by being never relocated by
> > + prelink as its addresses are always zero. */
>
> I understand why you mean, now, about the PT_GNU_STACK entry not being
> changed during the prelink. But I don't get the relationship between
> this comment and the code surrounding it. Can you explain that?
Code simplified for better readability in this mail:
/* 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. */
*buf_vaddr_p -= displacement;
*buf_paddr_p -= displacement;
if (memcmp (phdrp, phdr2p, sizeof (*phdrp)) == 0)
continue;
For detected DISPLACEMENT value 0x3000000000 the latter test works:
PHDR 0x000040 0x0000000000000040 0x0000000000000040 0x0001c0 0x0001c0 R E 0x8
->
PHDR 0x000040 0x0000003000000040 0x0000003000000040 0x0001c0 0x0001c0 R E 0x8
as 0x0000003000000040 - 0x3000000000 == 0x0000000000000040.
But in the same executable there is also
GNU_STACK 0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW 0x8
->
GNU_STACK 0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW 0x8
but 0x0000000000000000 - 0x3000000000 == 0xffffffd000000000 and thus
0x0000000000000000 - 0x3000000000 != 0x0000000000000000
and we would fail the verification despite the executable perfectly matches.
I believe one should be looking at some two `readelf -Wa' dumps of an
executable with two prelink addresses while checking this code so it should be
apparent during real updates of that code.
Thanks,
Jan
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath
2010-07-05 17:22 ` Jan Kratochvil
@ 2010-07-05 17:49 ` Joel Brobecker
2010-07-05 18:09 ` Jan Kratochvil
0 siblings, 1 reply; 9+ messages in thread
From: Joel Brobecker @ 2010-07-05 17:49 UTC (permalink / raw)
To: Jan Kratochvil; +Cc: gdb-patches
> I believe one should be looking at some two `readelf -Wa' dumps of an
> executable with two prelink addresses while checking this code so it should be
> apparent during real updates of that code.
That makes sense.
I look forward to the patches series being checked in...
--
Joel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath
2010-07-05 17:49 ` Joel Brobecker
@ 2010-07-05 18:09 ` Jan Kratochvil
0 siblings, 0 replies; 9+ messages in thread
From: Jan Kratochvil @ 2010-07-05 18:09 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches
On Mon, 05 Jul 2010 19:49:20 +0200, Joel Brobecker wrote:
> I look forward to the patches series being checked in...
Checked-in.
Thanks,
Jan
http://sourceware.org/ml/gdb-cvs/2010-07/msg00024.html
--- src/gdb/ChangeLog 2010/07/02 21:22:28 1.11963
+++ src/gdb/ChangeLog 2010/07/05 17:57:49 1.11964
@@ -1,3 +1,12 @@
+2010-07-05 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.
+
2010-07-02 Tom Tromey <tromey@redhat.com>
PR exp/11780:
--- src/gdb/solib-svr4.c 2010/05/16 23:49:58 1.134
+++ src/gdb/solib-svr4.c 2010/07/05 17:57:50 1.135
@@ -1775,13 +1775,187 @@
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 more easily 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 more easily 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);
--- src/gdb/testsuite/ChangeLog 2010/07/02 19:11:55 1.2370
+++ src/gdb/testsuite/ChangeLog 2010/07/05 17:57:50 1.2371
@@ -1,3 +1,14 @@
+2010-07-05 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.
+
2010-07-02 Tom Tromey <tromey@redhat.com>
* gdb.base/bitops.exp: Remove extraneous "pass".
--- src/gdb/testsuite/gdb.base/break-interp.exp 2010/06/29 21:48:10 1.13
+++ src/gdb/testsuite/gdb.base/break-interp.exp 2010/07/05 17:57:50 1.14
@@ -154,6 +154,12 @@
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 @@
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 @@
# 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 @@
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 @@
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 @@
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 @@
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
}
}
}
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-07-05 18:09 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-29 16:30 [patch 1/6]: PIE: Attach binary even after re-prelinked underneath Jan Kratochvil
2010-03-29 16:15 ` [patch 1/6]: PIE: Attach binary even after re-prelinked underneath [repost] Jan Kratochvil
2010-06-09 15:08 ` ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath Jan Kratochvil
2010-06-29 17:49 ` Joel Brobecker
2010-07-04 10:17 ` Jan Kratochvil
2010-07-05 17:06 ` Joel Brobecker
2010-07-05 17:22 ` Jan Kratochvil
2010-07-05 17:49 ` Joel Brobecker
2010-07-05 18:09 ` Jan Kratochvil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox