On 12/6/24 09:56, Quentin Monnet wrote:
2024-12-06 09:11 UTC+0800 ~ Rong Tao <rtoax@xxxxxxxxxxx>
On 12/6/24 05:34, Andrii Nakryiko wrote:
On Thu, Dec 5, 2024 at 4:22 AM Quentin Monnet <qmo@xxxxxxxxxx> wrote:
On 05/12/2024 12:09, Rong Tao wrote:
From: Rong Tao <rongtao@xxxxxxxx>
If the input file and output file are the same, the input file is
cleared
due to opening, resulting in a NULL pointer access by libbpf.
$ bpftool gen object prog.o prog.o
libbpf: failed to get ELF header for prog.o: invalid `Elf' handle
Segmentation fault
(gdb) bt
#0 0x0000000000450285 in linker_append_elf_syms
(linker=0x4feda0, obj=0x7fffffffe100) at linker.c:1296
#1 bpf_linker__add_file (linker=0x4feda0, filename=<optimized
out>, opts=<optimized out>) at linker.c:453
#2 0x000000000040c235 in do_object ()
#3 0x00000000004021d7 in main ()
(gdb) frame 0
#0 0x0000000000450285 in linker_append_elf_syms
(linker=0x4feda0, obj=0x7fffffffe100) at linker.c:1296
1296 Elf64_Sym *sym = symtab->data->d_buf;
Signed-off-by: Rong Tao <rongtao@xxxxxxxx>
Tested-by: Quentin Monnet <qmo@xxxxxxxxxx>
Reviewed-by: Quentin Monnet <qmo@xxxxxxxxxx>
Isn't this papering over a deeper underlying issue? Why do we get
SIGSEGV inside the linker at all instead of just erroring out?
Comparison based on file path isn't a reliable way to check if input
and output are both the same file, so this fixes the most obvious
case, but not the actual issue.
Thanks for your replay! The current scenario is similar to the following
code.
After a.txt is opened in read mode, it is opened in write mode again, which
causes the contents of a.txt file to be cleared, resulting in no data
being read,
fpr = fopen("a.txt", "r");
fpw = fopen("a.txt", "w");
/* fgets() will get nothing, It's not glibc's fault. */
while (fgets(buff, sizeof(buff), fpr))
printf("%s", buff);
fprintf(fpw, "....");
fclose(fpr);
fclose(fpw);
corresponding to the SEGV of bpftool. Perhaps we can add the following
warning
if (x == NULL) {
fprintf(stderr, "Maybe the file was opened for writing after
opened for read\n");
return -EINVAL;
}
Whether this warning can be added may depend on libelf's processing. I will
try to fix this SEGV in libbpf, hopefully it can be fixed.
Thank you Rong, I'm not sure I followed your explanation (the above is
not bpftool code, is it?), but I think we just addressed the issue in
libbpf with:
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=e10500b69c3f3378f3dcfc8c2fe4cdb74fc844f5
We can drop the patch with the check on the names (sorry!). As Andrii
mentioned, it's not very reliable to compare filenames. It's true that
users can truncate files if they pass the same input and output file,
but then that's the case with many command-line tools if you don't use
them properly.
I think we could do the both check for 'filename', right.
https://lore.kernel.org/lkml/tencent_4C02217218D4082166DD5C52A8D8BF228F0A@xxxxxx/
Thanks,
Rong Tao
So, no action required. Feel free to test with the patch above, the
segfault should not longer occur.
Thanks,
Quentin
