netdev - Re: [PATCH bpf-next 3/9] libbpf: improve relocation ambiguity detection

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] [day] [month] [year] [list]

Message-ID: <2249675.irdbgypaU6@7940hx>
Date: Tue, 13 Jan 2026 15:36:43 +0800
From: Menglong Dong <menglong.dong@...ux.dev>
To: Andrii Nakryiko <andriin@...com>
Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...com,
 daniel@...earbox.net, Andrii Nakryiko <andriin@...com>,
 andrii.nakryiko@...il.com, kernel-team@...com
Subject:
 Re: [PATCH bpf-next 3/9] libbpf: improve relocation ambiguity detection

On 2020/8/19 06:39 Andrii Nakryiko <andriin@...com> write:
> Split the instruction patching logic into relocation value calculation and
> application of relocation to instruction. Using this, evaluate relocation
> against each matching candidate and validate that all candidates agree on
> relocated value. If not, report ambiguity and fail load.
> 
> This logic is necessary to avoid dangerous (however unlikely) accidental match
> against two incompatible candidate types. Without this change, libbpf will
> pick a random type as *the* candidate and apply potentially invalid
> relocation.
> 
> Signed-off-by: Andrii Nakryiko <andriin@...com>
> ---
>  tools/lib/bpf/libbpf.c | 170 ++++++++++++++++++++++++++++++-----------
>  1 file changed, 124 insertions(+), 46 deletions(-)
> 
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 2047e4ed0076..1ba458140f50 100644
> --- a/tools/lib/bpf/libbpf.c
[......]
> @@ -5005,16 +5063,31 @@ static int bpf_core_reloc_field(struct bpf_program *prog,
>  		if (err == 0)
>  			continue;
>  
> +		err = bpf_core_calc_relo(prog, relo, relo_idx, &local_spec, &cand_spec, &cand_res);
> +		if (err)
> +			return err;
> +
>  		if (j == 0) {
> +			targ_res = cand_res;
>  			targ_spec = cand_spec;
>  		} else if (cand_spec.bit_offset != targ_spec.bit_offset) {
> -			/* if there are many candidates, they should all
> -			 * resolve to the same bit offset
> +			/* if there are many field relo candidates, they
> +			 * should all resolve to the same bit offset
>  			 */
> -			pr_warn("prog '%s': relo #%d: offset ambiguity: %u != %u\n",
> +			pr_warn("prog '%s': relo #%d: field offset ambiguity: %u != %u\n",
>  				prog_name, relo_idx, cand_spec.bit_offset,
>  				targ_spec.bit_offset);
>  			return -EINVAL;
> +		} else if (cand_res.poison != targ_res.poison || cand_res.new_val != targ_res.new_val) {
> +			/* all candidates should result in the same relocation
> +			 * decision and value, otherwise it's dangerous to
> +			 * proceed due to ambiguity
> +			 */
> +			pr_warn("prog '%s': relo #%d: relocation decision ambiguity: %s %u != %s %u\n",
> +				prog_name, relo_idx,
> +				cand_res.poison ? "failure" : "success", cand_res.new_val,
> +				targ_res.poison ? "failure" : "success", targ_res.new_val);
> +			return -EINVAL;
>  		}

Hi, Andrii. This approach is not friend to bpf_core_cast() if the struct
is not used in the vmlinux, but the kernel modules.

Take "struct nft_chain" for example. Following code will fail:
    struct nft_chain *chain = bpf_core_cast(ptr, struct nft_chain).

The bpf_core_cast() will record a BPF_CORE_TYPE_ID_TARGET relocation
for "struct nft_chain". The libbpf will find multi btf type of nft_chain
in the modules nf_tables, nft_reject, etc, and it will fail the verification
due to the "new_val", which is btf type id, not the same, even if all
the "struct nft_chain" are exactly the same in different kernel modules.

I think this is a common case. So how about we check the consistence of
struct nft_chain in all the candidate list, and use the first one if all of
them have exactly the same definition?

We can check all the members in the struct iteratively, and make
sure they are all the same.

Thanks!
Menglong Dong

>  
>  		cand_ids->data[j++] = cand_spec.spec[0].type_id;
> @@ -5042,13 +5115,18 @@ static int bpf_core_reloc_field(struct bpf_program *prog,
>  	 * verifier. If it was an error, then verifier will complain and point
>  	 * to a specific instruction number in its log.
>  	 */
> -	if (j == 0)
> +	if (j == 0) {
>  		pr_debug("prog '%s': relo #%d: no matching targets found\n",
>  			 prog_name, relo_idx);
>  
> -	/* bpf_core_reloc_insn should know how to handle missing targ_spec */
> -	err = bpf_core_reloc_insn(prog, relo, relo_idx, &local_spec,
> -				  j ? &targ_spec : NULL);
> +		/* calculate single target relo result explicitly */
> +		err = bpf_core_calc_relo(prog, relo, relo_idx, &local_spec, NULL, &targ_res);
> +		if (err)
> +			return err;
> +	}
> +
> +	/* bpf_core_patch_insn() should know how to handle missing targ_spec */
> +	err = bpf_core_patch_insn(prog, relo, relo_idx, &targ_res);
>  	if (err) {
>  		pr_warn("prog '%s': relo #%d: failed to patch insn at offset %d: %d\n",
>  			prog_name, relo_idx, relo->insn_off, err);
> -- 
> 2.24.1
> 
>