linux-kernel - Re: [PATCH bpf-next v3] bpftool: Fix gen object segfault

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <tencent_0F0D028440B2BE2E37547C5EFF467511FD09@qq.com>
Date: Fri, 6 Dec 2024 09:11:44 +0800
From: Rong Tao <rtoax@...mail.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>,
 Quentin Monnet <qmo@...nel.org>
Cc: ast@...nel.org, daniel@...earbox.net, rongtao@...tc.cn,
 Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
 <martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>,
 Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
 John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>,
 Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>,
 Jiri Olsa <jolsa@...nel.org>,
 "open list:BPF [TOOLING] (bpftool)" <bpf@...r.kernel.org>,
 open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH bpf-next v3] bpftool: Fix gen object segfault


On 12/6/24 05:34, Andrii Nakryiko wrote:
> On Thu, Dec 5, 2024 at 4:22 AM Quentin Monnet <qmo@...nel.org> wrote:
>> On 05/12/2024 12:09, Rong Tao wrote:
>>> From: Rong Tao <rongtao@...tc.cn>
>>>
>>> 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@...tc.cn>
>> Tested-by: Quentin Monnet <qmo@...nel.org>
>> Reviewed-by: Quentin Monnet <qmo@...nel.org>
> 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.

Thanks,
Rong Tao
>> Thank you!