On 16-01-19 01:56, Ian Lance Taylor wrote:
> On Tue, Dec 11, 2018 at 2:14 AM Tom de Vries <tdevr...@suse.de> wrote:
>>
>> Read the elf file pointed at by the .gnu_debugaltlink section, and verify
>> that
>> the build id matches.
>>
>> 2018-11-11 Tom de Vries <tdevr...@suse.de>
>>
>> * elf.c (elf_add): Add and handle with_buildid_data and
>> with_buildid_size parameters. Handle .gnu_debugaltlink section.
>> (phdr_callback, backtrace_initialize): Add arguments to elf_add
>> calls.
>> ---
>
>
>
> @@ -2899,6 +2918,27 @@ elf_add (struct backtrace_state *state, const
> char *filename, int descriptor,
>> }
>> }
>>
>> + if (!debugaltlink_view_valid
>> + && strcmp (name, ".gnu_debugaltlink") == 0)
>> + {
>> + const char *debugaltlink_data;
>> + size_t debugaltlink_name_len;
>> +
>> + if (!backtrace_get_view (state, descriptor, shdr->sh_offset,
>> + shdr->sh_size, error_callback, data,
>> + &debugaltlink_view))
>> + goto fail;
>> +
>> + debugaltlink_view_valid = 1;
>> + debugaltlink_data = (const char *) debugaltlink_view.data;
>> + debugaltlink_name = debugaltlink_data;
>> + debugaltlink_name_len = strnlen (debugaltlink_data, shdr->sh_size);
>> + debugaltlink_buildid_data = (debugaltlink_data
>> + + debugaltlink_name_len
>> + + 1);
>> + debugaltlink_buildid_size = shdr->sh_size - debugaltlink_name_len
>> - 1;
>> + }
>> +
>
> This doesn't look quite right. debugaltlink_buildid_size is unsigned.
> If there is some misunderstanding of the format it's possible for
> strnlen to return shdr->sh_size. If it does,
> debugaltlink_buildid_size will be set to a very large value.
>
I see, thanks for finding that.
Fixed like this:
...
debugaltlink_name_len = strnlen (debugaltlink_data, shdr->sh_size);
if (debugaltlink_name_len < shdr->sh_size)
{
/* Include terminating zero. */
debugaltlink_name_len =+ 1;
debugaltlink_buildid_data
= debugaltlink_data + debugaltlink_name_len;
debugaltlink_buildid_size
= shdr->sh_size - debugaltlink_name_len;
}
...
>> + if (debugaltlink_name != NULL)
>> + {
>> + int d;
>> +
>> + d = elf_open_debugfile_by_debuglink (state, filename,
>> debugaltlink_name,
>> + 0, error_callback, data);
>> + if (d >= 0)
>> + {
>> + int ret;
>> +
>> + ret = elf_add (state, filename, d, base_address, error_callback,
>> data,
>> + fileline_fn, found_sym, found_dwarf, 0, 1,
>> + debugaltlink_buildid_data,
>> debugaltlink_buildid_size);
>> + backtrace_release_view (state, &debugaltlink_view, error_callback,
>> + data);
>> + debugaltlink_view_valid = 0;
>> + if (ret < 0)
>> + {
>> + backtrace_close (d, error_callback, data);
>> + return ret;
>> + }
>> + }
>> + else
>> + {
>> + error_callback (data,
>> + "Could not open .gnu_debugaltlink", 0);
>> + /* Don't goto fail, but try continue without the info in the
>> + .gnu_debugaltlink. */
>> + }
>> + }
>
> The strings passed to error_callback always start with a lowercase
> letter (unless they start with something like ELF) because the
> callback will most likely print them with some prefix.
>
Fixed.
> More seriously, we don't call error_callback in any cases that
> correspond to this. We just carry on.
Indeed, I noticed that, and filed PR88244 - "[libbacktrace] Failure to
open .gnu_debuglink is silent".
[ The scenario there is: an executable has a .gnu_debuglink, but the
file the .gnu_debuglink is pointing to is absent, because f.i. it has
been removed, or moved to a different location. If a user runs this
executable and a backtrace is triggered, the information relating to the
functions in the executable will be missing in the backtrace, but
there's no error explaining to the user why that information is missing.
Note: there is a default error "no debug info in ELF executable" in
elf_nodebug, but AFAIU this is not triggered if debug info for one of
the shared libraries is present. ]
BTW, though in the code above an error_callback is called, we don't
error out, but do carry on afterwards (as the comment explicitly states).
> Is there any reason to call
> error_callback here?
A similar scenario: an executable has a .gnu_altdebuglink, but the file
the .gnu_debugaltlink is pointing to is absent, because f.i. it has been
removed, or moved to a different location. If a user runs this
executable and a backtrace is triggered, the information stored in the
.gnu_debugaltlink file will be missing in the backtrace, but there's no
error explaining to the user why that information is missing.
Thanks,
- Tom
[libbacktrace] Read .gnu_debugaltlink
Read the elf file pointed at by the .gnu_debugaltlink section, and verify that
the build id matches.
2018-11-11 Tom de Vries <tdevr...@suse.de>
* elf.c (elf_add): Add and handle with_buildid_data and
with_buildid_size parameters. Handle .gnu_debugaltlink section.
(phdr_callback, backtrace_initialize): Add arguments to elf_add calls.
---
libbacktrace/elf.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 95 insertions(+), 5 deletions(-)
diff --git a/libbacktrace/elf.c b/libbacktrace/elf.c
index 85a323c5876..e363e470525 100644
--- a/libbacktrace/elf.c
+++ b/libbacktrace/elf.c
@@ -2638,7 +2638,8 @@ static int
elf_add (struct backtrace_state *state, const char *filename, int descriptor,
uintptr_t base_address, backtrace_error_callback error_callback,
void *data, fileline *fileline_fn, int *found_sym, int *found_dwarf,
- int exe, int debuginfo)
+ int exe, int debuginfo, const char *with_buildid_data,
+ uint32_t with_buildid_size)
{
struct backtrace_view ehdr_view;
b_elf_ehdr ehdr;
@@ -2670,6 +2671,11 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
int debuglink_view_valid;
const char *debuglink_name;
uint32_t debuglink_crc;
+ struct backtrace_view debugaltlink_view;
+ int debugaltlink_view_valid;
+ const char *debugaltlink_name;
+ const char *debugaltlink_buildid_data;
+ uint32_t debugaltlink_buildid_size;
off_t min_offset;
off_t max_offset;
struct backtrace_view debug_view;
@@ -2694,6 +2700,10 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
debuglink_view_valid = 0;
debuglink_name = NULL;
debuglink_crc = 0;
+ debugaltlink_view_valid = 0;
+ debugaltlink_name = NULL;
+ debugaltlink_buildid_data = NULL;
+ debugaltlink_buildid_size = 0;
debug_view_valid = 0;
opd = NULL;
@@ -2873,6 +2883,15 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
buildid_data = ¬e->name[0] + ((note->namesz + 3) & ~ 3);
buildid_size = note->descsz;
}
+
+ if (with_buildid_size != 0)
+ {
+ if (buildid_size != with_buildid_size)
+ goto fail;
+
+ if (memcmp (buildid_data, with_buildid_data, buildid_size) != 0)
+ goto fail;
+ }
}
/* Read the debuglink file if present. */
@@ -2899,6 +2918,32 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
}
}
+ if (!debugaltlink_view_valid
+ && strcmp (name, ".gnu_debugaltlink") == 0)
+ {
+ const char *debugaltlink_data;
+ size_t debugaltlink_name_len;
+
+ if (!backtrace_get_view (state, descriptor, shdr->sh_offset,
+ shdr->sh_size, error_callback, data,
+ &debugaltlink_view))
+ goto fail;
+
+ debugaltlink_view_valid = 1;
+ debugaltlink_data = (const char *) debugaltlink_view.data;
+ debugaltlink_name = debugaltlink_data;
+ debugaltlink_name_len = strnlen (debugaltlink_data, shdr->sh_size);
+ if (debugaltlink_name_len < shdr->sh_size)
+ {
+ /* Include terminating zero. */
+ debugaltlink_name_len =+ 1;
+
+ debugaltlink_buildid_data
+ = debugaltlink_data + debugaltlink_name_len;
+ debugaltlink_buildid_size = shdr->sh_size - debugaltlink_name_len;
+ }
+ }
+
/* Read the .opd section on PowerPC64 ELFv1. */
if (ehdr.e_machine == EM_PPC64
&& (ehdr.e_flags & EF_PPC64_ABI) < 2
@@ -2993,8 +3038,11 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
if (debuglink_view_valid)
backtrace_release_view (state, &debuglink_view, error_callback,
data);
+ if (debugaltlink_view_valid)
+ backtrace_release_view (state, &debugaltlink_view, error_callback,
+ data);
ret = elf_add (state, NULL, d, base_address, error_callback, data,
- fileline_fn, found_sym, found_dwarf, 0, 1);
+ fileline_fn, found_sym, found_dwarf, 0, 1, NULL, 0);
if (ret < 0)
backtrace_close (d, error_callback, data);
else
@@ -3028,8 +3076,11 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
backtrace_release_view (state, &debuglink_view, error_callback,
data);
+ if (debugaltlink_view_valid)
+ backtrace_release_view (state, &debugaltlink_view, error_callback,
+ data);
ret = elf_add (state, NULL, d, base_address, error_callback, data,
- fileline_fn, found_sym, found_dwarf, 0, 1);
+ fileline_fn, found_sym, found_dwarf, 0, 1, NULL, 0);
if (ret < 0)
backtrace_close (d, error_callback, data);
else
@@ -3044,6 +3095,43 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
debuglink_view_valid = 0;
}
+ if (debugaltlink_name != NULL)
+ {
+ int d;
+
+ d = elf_open_debugfile_by_debuglink (state, filename, debugaltlink_name,
+ 0, error_callback, data);
+ if (d >= 0)
+ {
+ int ret;
+
+ ret = elf_add (state, filename, d, base_address, error_callback, data,
+ fileline_fn, found_sym, found_dwarf, 0, 1,
+ debugaltlink_buildid_data, debugaltlink_buildid_size);
+ backtrace_release_view (state, &debugaltlink_view, error_callback,
+ data);
+ debugaltlink_view_valid = 0;
+ if (ret < 0)
+ {
+ backtrace_close (d, error_callback, data);
+ return ret;
+ }
+ }
+ else
+ {
+ error_callback (data,
+ "could not open .gnu_debugaltlink", 0);
+ /* Don't goto fail, but try continue without the info in the
+ .gnu_debugaltlink. */
+ }
+ }
+
+ if (debugaltlink_view_valid)
+ {
+ backtrace_release_view (state, &debugaltlink_view, error_callback, data);
+ debugaltlink_view_valid = 0;
+ }
+
/* Read all the debug sections in a single view, since they are
probably adjacent in the file. We never release this view. */
@@ -3199,6 +3287,8 @@ elf_add (struct backtrace_state *state, const char *filename, int descriptor,
backtrace_release_view (state, &strtab_view, error_callback, data);
if (debuglink_view_valid)
backtrace_release_view (state, &debuglink_view, error_callback, data);
+ if (debugaltlink_view_valid)
+ backtrace_release_view (state, &debugaltlink_view, error_callback, data);
if (buildid_view_valid)
backtrace_release_view (state, &buildid_view, error_callback, data);
if (debug_view_valid)
@@ -3269,7 +3359,7 @@ phdr_callback (struct dl_phdr_info *info, size_t size ATTRIBUTE_UNUSED,
if (elf_add (pd->state, filename, descriptor, info->dlpi_addr,
pd->error_callback, pd->data, &elf_fileline_fn, pd->found_sym,
- &found_dwarf, 0, 0))
+ &found_dwarf, 0, 0, NULL, 0))
{
if (found_dwarf)
{
@@ -3297,7 +3387,7 @@ backtrace_initialize (struct backtrace_state *state, const char *filename,
struct phdr_data pd;
ret = elf_add (state, filename, descriptor, 0, error_callback, data,
- &elf_fileline_fn, &found_sym, &found_dwarf, 1, 0);
+ &elf_fileline_fn, &found_sym, &found_dwarf, 1, 0, NULL, 0);
if (!ret)
return 0;
