diff mbox series

[v3] mtrace: Fix output with PIE and ASLR [BZ #22716]

Message ID 20210812090746.612090-1-siddhesh@sourceware.org
State Committed
Commit f2e33c3268db9adf8e57e991676ed0d5ac74e8a8
Headers
Series [v3] mtrace: Fix output with PIE and ASLR [BZ #22716] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Siddhesh Poyarekar Aug. 12, 2021, 9:07 a.m. UTC 
  Record only the relative address of the caller in mtrace file.  Use
LD_TRACE_PRELINKING to get the executable as well as binary vs
executable load offsets so that we may compute a base to add to the
relative address in the mtrace file.  This allows us to get a valid
address to pass to addr2line in all cases.

Co-authored-by:  John Ogness <john.ogness@linutronix.de>
---
Changes from v2:
- Don't use ""

Changes from v1:
- Set LD_TRACE_PRELINK to an empty string so that it doesn't match any
  files and add a comment explaining it.
- Formatting change in mtrace-impl.c

 malloc/mtrace-impl.c |  6 +++---
 malloc/mtrace.pl     | 15 +++++++--------
 2 files changed, 10 insertions(+), 11 deletions(-)

Comments

John Ogness Aug. 12, 2021, 10:39 a.m. UTC | #1 
On 2021-08-12, Siddhesh Poyarekar <siddhesh@sourceware.org> wrote:
> Record only the relative address of the caller in mtrace file.  Use
> LD_TRACE_PRELINKING to get the executable as well as binary vs
> executable load offsets so that we may compute a base to add to the
> relative address in the mtrace file.  This allows us to get a valid
> address to pass to addr2line in all cases.

FWIW, I tested this using an executable with a memory leak that is
linked to a shared library that also has a leak.

Executable 1 compiled with "-pie -fPIE".
Executable 2 compiled with "-no-pie -fno-PIE".
Shared library compiled with "-fPIC".

Tested both executables in ASLR and non-ASLR environments successfully.

The only issue is that obviously trace files from older glibc will not
work with this new mtrace and vice versa. But I suppose that is not a
concern.

John Ogness
Siddhesh Poyarekar Aug. 17, 2021, 2:57 a.m. UTC | #2 
Ping!  I'll push this if there are no further objections.

Thanks,
Siddhesh

On 8/12/21 2:37 PM, Siddhesh Poyarekar via Libc-alpha wrote:
> Record only the relative address of the caller in mtrace file.  Use
> LD_TRACE_PRELINKING to get the executable as well as binary vs
> executable load offsets so that we may compute a base to add to the
> relative address in the mtrace file.  This allows us to get a valid
> address to pass to addr2line in all cases.
> 
> Co-authored-by:  John Ogness <john.ogness@linutronix.de>
> ---
> Changes from v2:
> - Don't use ""
> 
> Changes from v1:
> - Set LD_TRACE_PRELINK to an empty string so that it doesn't match any
>    files and add a comment explaining it.
> - Formatting change in mtrace-impl.c
> 
>   malloc/mtrace-impl.c |  6 +++---
>   malloc/mtrace.pl     | 15 +++++++--------
>   2 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/malloc/mtrace-impl.c b/malloc/mtrace-impl.c
> index 9dffddfe2d..e7ace7f6fd 100644
> --- a/malloc/mtrace-impl.c
> +++ b/malloc/mtrace-impl.c
> @@ -67,9 +67,9 @@ tr_where (const void *caller, Dl_info *info)
>   		       offset);
>               }
>   
> -	  fprintf (mallstream, "@ %s%s%s[%p] ", info->dli_fname ? : "",
> -		   info->dli_fname ? ":" : "",
> -                   buf, caller);
> +	  fprintf (mallstream, "@ %s%s%s[0x%" PRIxPTR "] ",
> +		   info->dli_fname ? : "", info->dli_fname ? ":" : "", buf,
> +		   caller - info->dli_fbase);
>           }
>         else
>           fprintf (mallstream, "@ [%p] ", caller);
> diff --git a/malloc/mtrace.pl b/malloc/mtrace.pl
> index 6f49c8338d..b1073a1931 100644
> --- a/malloc/mtrace.pl
> +++ b/malloc/mtrace.pl
> @@ -75,11 +75,15 @@ if ($#ARGV == 0) {
>       } else {
>   	$prog = "./$binary";
>       }
> -    if (open (LOCS, "env LD_TRACE_LOADED_OBJECTS=1 $prog |")) {
> +    # Set the environment variable LD_TRACE_PRELINKING to an empty string so
> +    # that we trigger tracing but do not match with the executable or any of
> +    # its dependencies.
> +    if (open (LOCS, "env LD_TRACE_PRELINKING= $prog |")) {
>   	while (<LOCS>) {
>   	    chop;
> -	    if (/^.*=> (.*) .(0x[0123456789abcdef]*).$/) {
> +	    if (/^.*=> (.*) \((0x[0123456789abcdef]*), (0x[0123456789abcdef]*).*/) {
>   		$locs{$1} = $2;
> +		$rel{$1} = hex($2) - hex($3);
>   	    }
>   	}
>   	close (LOCS);
> @@ -110,12 +114,7 @@ sub location {
>   	my $addr = $2;
>   	my $searchaddr;
>   	return $cache{$addr} if (exists $cache{$addr});
> -	if ($locs{$prog} ne "") {
> -	    $searchaddr = sprintf "%#x", $addr - $locs{$prog};
> -	} else {
> -	    $searchaddr = $addr;
> -	    $prog = $binary;
> -	}
> +	$searchaddr = sprintf "%#x", hex($addr) + $rel{$prog};
>   	if ($binary ne "" && open (ADDR, "addr2line -e $prog $searchaddr|")) {
>   	    my $line = <ADDR>;
>   	    chomp $line;
>
Siddhesh Poyarekar Aug. 23, 2021, 2:45 a.m. UTC | #3 
I've pushed this now since the changes in v2 and v3 were trivial.

Thanks,
Siddhesh


On 8/17/21 8:27 AM, Siddhesh Poyarekar wrote:
> Ping!  I'll push this if there are no further objections.
> 
> Thanks,
> Siddhesh
> 
> On 8/12/21 2:37 PM, Siddhesh Poyarekar via Libc-alpha wrote:
>> Record only the relative address of the caller in mtrace file.  Use
>> LD_TRACE_PRELINKING to get the executable as well as binary vs
>> executable load offsets so that we may compute a base to add to the
>> relative address in the mtrace file.  This allows us to get a valid
>> address to pass to addr2line in all cases.
>>
>> Co-authored-by:  John Ogness <john.ogness@linutronix.de>
>> ---
>> Changes from v2:
>> - Don't use ""
>>
>> Changes from v1:
>> - Set LD_TRACE_PRELINK to an empty string so that it doesn't match any
>>    files and add a comment explaining it.
>> - Formatting change in mtrace-impl.c
>>
>>   malloc/mtrace-impl.c |  6 +++---
>>   malloc/mtrace.pl     | 15 +++++++--------
>>   2 files changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/malloc/mtrace-impl.c b/malloc/mtrace-impl.c
>> index 9dffddfe2d..e7ace7f6fd 100644
>> --- a/malloc/mtrace-impl.c
>> +++ b/malloc/mtrace-impl.c
>> @@ -67,9 +67,9 @@ tr_where (const void *caller, Dl_info *info)
>>                  offset);
>>               }
>> -      fprintf (mallstream, "@ %s%s%s[%p] ", info->dli_fname ? : "",
>> -           info->dli_fname ? ":" : "",
>> -                   buf, caller);
>> +      fprintf (mallstream, "@ %s%s%s[0x%" PRIxPTR "] ",
>> +           info->dli_fname ? : "", info->dli_fname ? ":" : "", buf,
>> +           caller - info->dli_fbase);
>>           }
>>         else
>>           fprintf (mallstream, "@ [%p] ", caller);
>> diff --git a/malloc/mtrace.pl b/malloc/mtrace.pl
>> index 6f49c8338d..b1073a1931 100644
>> --- a/malloc/mtrace.pl
>> +++ b/malloc/mtrace.pl
>> @@ -75,11 +75,15 @@ if ($#ARGV == 0) {
>>       } else {
>>       $prog = "./$binary";
>>       }
>> -    if (open (LOCS, "env LD_TRACE_LOADED_OBJECTS=1 $prog |")) {
>> +    # Set the environment variable LD_TRACE_PRELINKING to an empty 
>> string so
>> +    # that we trigger tracing but do not match with the executable or 
>> any of
>> +    # its dependencies.
>> +    if (open (LOCS, "env LD_TRACE_PRELINKING= $prog |")) {
>>       while (<LOCS>) {
>>           chop;
>> -        if (/^.*=> (.*) .(0x[0123456789abcdef]*).$/) {
>> +        if (/^.*=> (.*) \((0x[0123456789abcdef]*), 
>> (0x[0123456789abcdef]*).*/) {
>>           $locs{$1} = $2;
>> +        $rel{$1} = hex($2) - hex($3);
>>           }
>>       }
>>       close (LOCS);
>> @@ -110,12 +114,7 @@ sub location {
>>       my $addr = $2;
>>       my $searchaddr;
>>       return $cache{$addr} if (exists $cache{$addr});
>> -    if ($locs{$prog} ne "") {
>> -        $searchaddr = sprintf "%#x", $addr - $locs{$prog};
>> -    } else {
>> -        $searchaddr = $addr;
>> -        $prog = $binary;
>> -    }
>> +    $searchaddr = sprintf "%#x", hex($addr) + $rel{$prog};
>>       if ($binary ne "" && open (ADDR, "addr2line -e $prog 
>> $searchaddr|")) {
>>           my $line = <ADDR>;
>>           chomp $line;
>>
>
diff mbox series

Patch

diff --git a/malloc/mtrace-impl.c b/malloc/mtrace-impl.c
index 9dffddfe2d..e7ace7f6fd 100644
--- a/malloc/mtrace-impl.c
+++ b/malloc/mtrace-impl.c
@@ -67,9 +67,9 @@  tr_where (const void *caller, Dl_info *info)
 		       offset);
             }
 
-	  fprintf (mallstream, "@ %s%s%s[%p] ", info->dli_fname ? : "",
-		   info->dli_fname ? ":" : "",
-                   buf, caller);
+	  fprintf (mallstream, "@ %s%s%s[0x%" PRIxPTR "] ",
+		   info->dli_fname ? : "", info->dli_fname ? ":" : "", buf,
+		   caller - info->dli_fbase);
         }
       else
         fprintf (mallstream, "@ [%p] ", caller);
diff --git a/malloc/mtrace.pl b/malloc/mtrace.pl
index 6f49c8338d..b1073a1931 100644
--- a/malloc/mtrace.pl
+++ b/malloc/mtrace.pl
@@ -75,11 +75,15 @@  if ($#ARGV == 0) {
     } else {
 	$prog = "./$binary";
     }
-    if (open (LOCS, "env LD_TRACE_LOADED_OBJECTS=1 $prog |")) {
+    # Set the environment variable LD_TRACE_PRELINKING to an empty string so
+    # that we trigger tracing but do not match with the executable or any of
+    # its dependencies.
+    if (open (LOCS, "env LD_TRACE_PRELINKING= $prog |")) {
 	while (<LOCS>) {
 	    chop;
-	    if (/^.*=> (.*) .(0x[0123456789abcdef]*).$/) {
+	    if (/^.*=> (.*) \((0x[0123456789abcdef]*), (0x[0123456789abcdef]*).*/) {
 		$locs{$1} = $2;
+		$rel{$1} = hex($2) - hex($3);
 	    }
 	}
 	close (LOCS);
@@ -110,12 +114,7 @@  sub location {
 	my $addr = $2;
 	my $searchaddr;
 	return $cache{$addr} if (exists $cache{$addr});
-	if ($locs{$prog} ne "") {
-	    $searchaddr = sprintf "%#x", $addr - $locs{$prog};
-	} else {
-	    $searchaddr = $addr;
-	    $prog = $binary;
-	}
+	$searchaddr = sprintf "%#x", hex($addr) + $rel{$prog};
 	if ($binary ne "" && open (ADDR, "addr2line -e $prog $searchaddr|")) {
 	    my $line = <ADDR>;
 	    chomp $line;