| 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
| ||
|
Message-ID: <CAEf4Bzb04C97K=S1av_6EKG3jKHoG+mKwaxVw3cCnNsbyiDzmw@mail.gmail.com>
Date: Tue, 13 Jan 2026 15:26:16 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Menglong Dong <menglong.dong@...ux.dev>
Cc: Andrii Nakryiko <andriin@...com>, bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...com,
daniel@...earbox.net, kernel-team@...com
Subject: Re: [PATCH bpf-next 3/9] libbpf: improve relocation ambiguity detection
On Mon, Jan 12, 2026 at 11:36 PM Menglong Dong <menglong.dong@...ux.dev> wrote:
>
> 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?
BTF type ID for some type in some kernel is not meaningful without
also capturing module's BTF ID or FD, so we'd be just capturing some
relatively random and meaningless type ID.
I'm actually not sure bpf_core_cast() can work with BTF types defined
in module's BTF. Can you please check what we do if we have
non-ambiguous BTF type defined only in module's BTF?
>
> We can check all the members in the struct iteratively, and make
> sure they are all the same.
>
It's not even clear what "same" would mean here, btw. None of the
issues you bring up are easy to solve :)
> 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
> >
> >
>
>
>
>
Powered by blists - more mailing lists