Received-SPF: pass (google.com: domain of linux-kernel+bounces-7146-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Subject: Re: [PATCH] fix null pointer dereference in
 bpf_object__collect_prog_relos
To: Xin Liu <liuxin350@huawei.com>, ast@kernel.org, andrii@kernel.org,
 martin.lau@linux.dev, song@kernel.org, yhs@fb.com, john.fastabend@gmail.com,
 kpsingh@kernel.org, sdf@google.com, haoluo@google.com, jolsa@kernel.org
Cc: bpf@vger.kernel.org, linux-kernel@vger.kernel.org, yanan@huawei.com,
 wuchangye@huawei.com, xiesongyang@huawei.com, kongweibin2@huawei.com,
 tianmuyang@huawei.com, zhangmingyi5@huawei.com
References: <20231220134151.144224-1-liuxin350@huawei.com>
From: Daniel Borkmann <daniel@iogearbox.net>
Message-ID: <fb06f00f-4a82-4b78-928e-775edd6340d0@iogearbox.net>
Date: Wed, 20 Dec 2023 16:18:14 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.7.2
Precedence: bulk
MIME-Version: 1.0
In-Reply-To: <20231220134151.144224-1-liuxin350@huawei.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit

On 12/20/23 2:41 PM, Xin Liu wrote:
> From: zhangmingyi <zhangmingyi5@huawei.com>

Small nits only, otherwise lgtm.

Please prefix subject with libbpf e.g. the full one should look like
"libbpf: Fix NULL pointer dereference in bpf_object__collect_prog_relos"

> a issue occurred while reading an ELF file in libbpf.c during fuzzing:

"An issue [...]"

> 	Program received signal SIGSEGV, Segmentation fault.
> 	0x0000000000958e97 in bpf_object.collect_prog_relos () at libbpf.c:4206
> 	4206 in libbpf.c
> 	(gdb) bt
> 	#0 0x0000000000958e97 in bpf_object.collect_prog_relos () at libbpf.c:4206
> 	#1 0x000000000094f9d6 in bpf_object.collect_relos () at libbpf.c:6706
> 	#2 0x000000000092bef3 in bpf_object_open () at libbpf.c:7437
> 	#3 0x000000000092c046 in bpf_object.open_mem () at libbpf.c:7497
> 	#4 0x0000000000924afa in LLVMFuzzerTestOneInput () at fuzz/bpf-object-fuzzer.c:16
> 	#5 0x000000000060be11 in testblitz_engine::fuzzer::Fuzzer::run_one ()
> 	#6 0x000000000087ad92 in tracing::span::Span::in_scope ()
> 	#7 0x00000000006078aa in testblitz_engine::fuzzer::util::walkdir ()
> 	#8 0x00000000005f3217 in testblitz_engine::entrypoint::main::{{closure}} ()
> 	#9 0x00000000005f2601 in main ()
> 	(gdb)
> 
> scn_data was null at this code(tools/lib/bpf/src/libbpf.c):
> 
> 	if (rel->r_offset % BPF_INSN_SZ || rel->r_offset >= scn_data->d_size) {
> 
> The scn_data is derived from the code above:
>      
> 	scn = elf_sec_by_idx(obj, sec_idx);
> 	scn_data = elf_sec_data(obj, scn);
> 
> 	relo_sec_name = elf_sec_str(obj, shdr->sh_name);
> 	sec_name = elf_sec_name(obj, scn);
> 	if (!relo_sec_name || !sec_name)// don't check whether scn_data is NULL
> 		return -EINVAL;
> 
> In certain special scenarios, such as reading a malformed ELF file,
> it is possible that scn_data may be a null pointer
> 
> Signed-off-by: zhangmingyi  <zhangmingyi5@huawei.com>

Zhang Mingyi <zhangmingyi5@huawei.com> ?

If that is correct, please also make sure that this is the same in From: line.

> Signed-off-by: Xin Liu <liuxin350@huawei.com>
> Signed-off-by: Changye Wu <wuchangye@huawei.com>
> ---
>   tools/lib/bpf/libbpf.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index e067be95da3c..df1b550f7460 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -4344,6 +4344,8 @@ bpf_object__collect_prog_relos(struct bpf_object *obj, Elf64_Shdr *shdr, Elf_Dat
>   
>   	scn = elf_sec_by_idx(obj, sec_idx);
>   	scn_data = elf_sec_data(obj, scn);
> +	if (!scn_data)
> +		return -LIBBPF_ERRNO__FORMAT;
>   
>   	relo_sec_name = elf_sec_str(obj, shdr->sh_name);
>   	sec_name = elf_sec_name(obj, scn);
>