diff mbox

[Libbacktrace] Fix possible SEGV when handling stripped PIE binaries.

Message ID 56D6F6DE.4090509@partner.samsung.com
State New
Headers show

Commit Message

max March 2, 2016, 2:21 p.m. UTC 
On 02/03/16 16:59, Ian Lance Taylor wrote:
> On Wed, Mar 2, 2016 at 12:51 AM, Maxim Ostapenko
> <m.ostapenko@partner.samsung.com> wrote:
>> When testing ASan on large system, I've noticed that sometimes it crashes
>> with SEGV in Libbacktrace when trying to symbolize stripped PIE (compiled
>> with -pie -fPIC) binaries in fully stripped environment (this means that all
>> dependent libraries are also stripped). Here a scenario I've observed:
>>
>> 1) _asan_backtrace_initialize calls elf_add passing &elf_fileline_fn as
>> output parameter to properly initialize it.
>> 2) elf_add doesn't elf_fileline_fn initialize and returns -1 for stripped
>> PIE binary.
>> 3) _asan_backtrace_initialize calls phdr_callback on each dependent library
>> via dl_iterate_phdr.
>> 4) phdr_callback initializes elf_fileline_fn iff it found debug info in some
>> library (found_dwarf == 1), but this is false since all libs are stripped.
>> So, we still have uninitialized elf_fileline_fn value.
>> 5) _asan_backtrace_initialize uses elf_fileline_fn to initialize proper
>> fileline_fn callback.
>> 6) Libbacktrace uses fileline_fn callback later and crashes because it
>> contains garbage.
>>
>> This patch fixes the issue by simply initializing elf_fileline_fn via
>> elf_nodebug in _asan_backtrace_initialize prologue.
>>
>> Tested on x86_64-linux-gnu and arm-linux-gnueabi, OK for trunk?
> Thanks for the analysis.  I would rather set *fileline_fn in the case
> where elf_add returns -1.  Or, remove the setting of *fileline_fn =
> elf_nodebug in elf_add, since that would become the default.
>
> Ian
>

Thanks, does this look better (I used the second option)?

-Maxim

Comments

Ian Lance Taylor March 2, 2016, 2:22 p.m. UTC | #1 
On Wed, Mar 2, 2016 at 6:21 AM, Maxim Ostapenko
<m.ostapenko@partner.samsung.com> wrote:
> On 02/03/16 16:59, Ian Lance Taylor wrote:
>>
>> On Wed, Mar 2, 2016 at 12:51 AM, Maxim Ostapenko
>> <m.ostapenko@partner.samsung.com> wrote:
>>>
>>> When testing ASan on large system, I've noticed that sometimes it crashes
>>> with SEGV in Libbacktrace when trying to symbolize stripped PIE (compiled
>>> with -pie -fPIC) binaries in fully stripped environment (this means that
>>> all
>>> dependent libraries are also stripped). Here a scenario I've observed:
>>>
>>> 1) _asan_backtrace_initialize calls elf_add passing &elf_fileline_fn as
>>> output parameter to properly initialize it.
>>> 2) elf_add doesn't elf_fileline_fn initialize and returns -1 for stripped
>>> PIE binary.
>>> 3) _asan_backtrace_initialize calls phdr_callback on each dependent
>>> library
>>> via dl_iterate_phdr.
>>> 4) phdr_callback initializes elf_fileline_fn iff it found debug info in
>>> some
>>> library (found_dwarf == 1), but this is false since all libs are
>>> stripped.
>>> So, we still have uninitialized elf_fileline_fn value.
>>> 5) _asan_backtrace_initialize uses elf_fileline_fn to initialize proper
>>> fileline_fn callback.
>>> 6) Libbacktrace uses fileline_fn callback later and crashes because it
>>> contains garbage.
>>>
>>> This patch fixes the issue by simply initializing elf_fileline_fn via
>>> elf_nodebug in _asan_backtrace_initialize prologue.
>>>
>>> Tested on x86_64-linux-gnu and arm-linux-gnueabi, OK for trunk?
>>
>> Thanks for the analysis.  I would rather set *fileline_fn in the case
>> where elf_add returns -1.  Or, remove the setting of *fileline_fn =
>> elf_nodebug in elf_add, since that would become the default.
>>
>> Ian
>>
>
> Thanks, does this look better (I used the second option)?

This is OK with a ChangeLog entry.  Thanks.

Ian
diff mbox

Patch

libbacktrace/ChangeLog:

2016-03-02  Maxim Ostapenko  <m.ostapenko@partner.samsung.com>

	* elf.c (backtrace_initialize): Properly initialize elf_fileline_fn to
	avoid possible crash.
	(elf_add): Don't set *fileline_fn to elf_nodebug value in case of
	missing debug info anymore.

diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c
index 05cc5c0..f85ac65 100644
--- a/libbacktrace/elf.c
+++ b/libbacktrace/elf.c
@@ -791,7 +791,6 @@  elf_add (struct backtrace_state *state, int descriptor, uintptr_t base_address,
     {
       if (!backtrace_close (descriptor, error_callback, data))
 	goto fail;
-      *fileline_fn = elf_nodebug;
       return 1;
     }
 
@@ -925,7 +924,7 @@  backtrace_initialize (struct backtrace_state *state, int descriptor,
   int ret;
   int found_sym;
   int found_dwarf;
-  fileline elf_fileline_fn;
+  fileline elf_fileline_fn = elf_nodebug;
   struct phdr_data pd;
 
   ret = elf_add (state, descriptor, 0, error_callback, data, &elf_fileline_fn,