Re: [PATCH bpf-next] libbpf: Fix segfault due to libelf functions not setting errno

[Quentin Monnet <qmo@kernel.org>]

2024-12-05 13:46 UTC-0800 ~ Andrii Nakryiko <andrii.nakryiko@gmail.com>
> On Thu, Dec 5, 2024 at 5:59 AM Quentin Monnet <qmo@kernel.org> wrote:
>>
>> Libelf functions do not set errno on failure. Instead, it relies on its
>> internal _elf_errno value, that can be retrieved via elf_errno (or the
>> corresponding message via elf_errmsg()). From "man libelf":
>>
>>     If a libelf function encounters an error it will set an internal
>>     error code that can be retrieved with elf_errno. Each thread
>>     maintains its own separate error code. The meaning of each error
>>     code can be determined with elf_errmsg, which returns a string
>>     describing the error.
>>
>> As a consequence, libbpf should not return -errno when a function from
>> libelf fails, because an empty value will not be interpreted as an error
>> and won't prevent the program to stop. This is visible in
>> bpf_linker__add_file(), for example, where we call a succession of
>> functions that rely on libelf:
>>
>>     err = err ?: linker_load_obj_file(linker, filename, opts, &obj);
>>     err = err ?: linker_append_sec_data(linker, &obj);
>>     err = err ?: linker_append_elf_syms(linker, &obj);
>>     err = err ?: linker_append_elf_relos(linker, &obj);
>>     err = err ?: linker_append_btf(linker, &obj);
>>     err = err ?: linker_append_btf_ext(linker, &obj);
>>
>> If the object file that we try to process is not, in fact, a correct
>> object file, linker_load_obj_file() may fail with errno not being set,
>> and return 0. In this case we attempt to run linker_append_elf_sysms()
>> and may segfault.
>>
>> This can happen (and was discovered) with bpftool:
>>
>>     $ bpftool gen object output.o sample_ret0.bpf.c
>>     libbpf: failed to get ELF header for sample_ret0.bpf.c: invalid `Elf' handle
>>     zsh: segmentation fault (core dumped)  bpftool gen object output.o sample_ret0.bpf.c
>>
>> Fix the issue by returning a non-null error code (-EINVAL) when libelf
>> functions fail.
>>
>> Fixes: faf6ed321cf6 ("libbpf: Add BPF static linker APIs")
>> Signed-off-by: Quentin Monnet <qmo@kernel.org>
>> ---
>>  tools/lib/bpf/linker.c | 22 ++++++++--------------
>>  1 file changed, 8 insertions(+), 14 deletions(-)
>>
> 
> Ok, so *this* is the real issue with SIGSEGV that we were trying to
> "prevent" by file path comparison in that bpftool-specific patch,
> right? LGTM, I'll apply to bpf-next.


Correct, I wanted to find where that segfault was coming from, too :).
Thanks!