| 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: <877btu8wz2.fsf@cloudflare.com>
Date: Tue, 06 Jan 2026 18:36:17 +0100
From: Jakub Sitnicki <jakub@...udflare.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Amery Hung <ameryhung@...il.com>, Martin KaFai Lau
<martin.lau@...ux.dev>, Martin KaFai Lau <martin.lau@...nel.org>, bpf
<bpf@...r.kernel.org>, Network Development <netdev@...r.kernel.org>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet
<edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
<pabeni@...hat.com>, Alexei Starovoitov <ast@...nel.org>, Daniel
Borkmann <daniel@...earbox.net>, Jesper Dangaard Brouer
<hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>, Stanislav
Fomichev <sdf@...ichev.me>, Simon Horman <horms@...nel.org>, Andrii
Nakryiko <andrii@...nel.org>, Eduard Zingerman <eddyz87@...il.com>, Song
Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>, KP Singh
<kpsingh@...nel.org>, Hao Luo <haoluo@...gle.com>, Jiri Olsa
<jolsa@...nel.org>, kernel-team <kernel-team@...udflare.com>
Subject: Re: [PATCH bpf-next v2 15/16] bpf: Realign skb metadata for TC
progs using data_meta
On Mon, Jan 05, 2026 at 06:04 PM -08, Alexei Starovoitov wrote:
> On Mon, Jan 5, 2026 at 3:19 PM Amery Hung <ameryhung@...il.com> wrote:
>>
>> >
>> > >
>> > > I guess we can mark such emitted call in insn_aux_data as finalized
>> > > and get_func_proto() isn't needed.
>> >
>> > It is a good idea.
>> >
>>
>> Hmm, insn_aux_data has to be marked in gen_{pro,epi}logue since this
>> is the only place we know whether the call needs fixup or not. However
>> insn_aux_data is not available yet in gen_{pro,epi}logue because we
>> haven't resized insn_aux_data.
>>
>> Can we do some hack based on the fact that calls emitted by
>> BPF_EMIT_CALL() are finalized while calls emitted by BPF_RAW_INSN()
>> most likely are not?
>> Let BPF_EMIT_CALL() mark the call insn as finalized temporarily (e.g.,
>> .off = 1). Then, when do_misc_fixups() encounters it just reset off to
>> 0 and don't call get_func_proto().
>
> marking inside insn via off=1 or whatever is an option,
> but once we remove BPF_CALL_KFUNC from gen_prologue we can
> delete add_kfunc_in_insns() altogether and replace it with
> a similar loop that does
> if (bpf_helper_call()) mark insn_aux_data.
>
> That would be a nice benefit, since add_kfunc_call() from there
> was always a bit odd, since we're adding kfuncs early before the main
> verifier pass and after, because of gen_prologue.
Thanks for all the pointers.
I understood we're looking for something like this:
---8<---
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index b32ddf0f0ab3..9ccd56c04a45 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -561,6 +561,7 @@ struct bpf_insn_aux_data {
bool non_sleepable; /* helper/kfunc may be called from non-sleepable context */
bool is_iter_next; /* bpf_iter_<type>_next() kfunc call */
bool call_with_percpu_alloc_ptr; /* {this,per}_cpu_ptr() with prog percpu alloc */
+ bool finalized_call; /* call holds function offset relative to __bpf_base_call */
u8 alu_state; /* used in combination with alu_limit */
/* true if STX or LDX instruction is a part of a spill/fill
* pattern for a bpf_fastcall call.
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1ca5c5e895ee..cc737d103cdd 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -21806,6 +21806,14 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
env->prog = new_prog;
delta += cnt - 1;
+ /* gen_prologue emits function calls with target address
+ * relative to __bpf_call_base. Skip patch_call_imm fixup.
+ */
+ for (i = 0; i < cnt - 1; i++) {
+ if (bpf_helper_call(&env->prog->insnsi[i]))
+ env->insn_aux_data[i].finalized_call = true;
+ }
+
ret = add_kfunc_in_insns(env, insn_buf, cnt - 1);
if (ret < 0)
return ret;
@@ -23412,6 +23420,9 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
goto next_insn;
}
patch_call_imm:
+ if (env->insn_aux_data[i + delta].finalized_call)
+ goto next_insn;
+
fn = env->ops->get_func_proto(insn->imm, env->prog);
/* all functions that have prototype and verifier allowed
* programs to call them, must be real in-kernel functions
@@ -23423,6 +23434,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
return -EFAULT;
}
insn->imm = fn->func - __bpf_call_base;
+ env->insn_aux_data[i + delta].finalized_call = true;
next_insn:
if (subprogs[cur_subprog + 1].start == i + delta + 1) {
subprogs[cur_subprog].stack_depth += stack_depth_extra;
diff --git a/net/core/filter.c b/net/core/filter.c
index 7f5bc6a505e1..53993c2c492d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -9082,8 +9082,7 @@ static int bpf_unclone_prologue(struct bpf_insn *insn_buf, u32 pkt_access_flags,
/* ret = bpf_skb_pull_data(skb, 0); */
*insn++ = BPF_MOV64_REG(BPF_REG_6, BPF_REG_1);
*insn++ = BPF_ALU64_REG(BPF_XOR, BPF_REG_2, BPF_REG_2);
- *insn++ = BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
- BPF_FUNC_skb_pull_data);
+ *insn++ = BPF_EMIT_CALL(bpf_skb_pull_data);
/* if (!ret)
* goto restore;
* return TC_ACT_SHOT;
@@ -9135,11 +9134,8 @@ static int bpf_gen_ld_abs(const struct bpf_insn *orig,
return insn - insn_buf;
}
-__bpf_kfunc_start_defs();
-
-__bpf_kfunc void bpf_skb_meta_realign(struct __sk_buff *skb_)
+static void bpf_skb_meta_realign(struct sk_buff *skb)
{
- struct sk_buff *skb = (typeof(skb))skb_;
u8 *meta_end = skb_metadata_end(skb);
u8 meta_len = skb_metadata_len(skb);
u8 *meta;
@@ -9161,14 +9157,6 @@ __bpf_kfunc void bpf_skb_meta_realign(struct __sk_buff *skb_)
bpf_compute_data_pointers(skb);
}
-__bpf_kfunc_end_defs();
-
-BTF_KFUNCS_START(tc_cls_act_hidden_ids)
-BTF_ID_FLAGS(func, bpf_skb_meta_realign)
-BTF_KFUNCS_END(tc_cls_act_hidden_ids)
-
-BTF_ID_LIST_SINGLE(bpf_skb_meta_realign_ids, func, bpf_skb_meta_realign)
-
static int tc_cls_act_prologue(struct bpf_insn *insn_buf, u32 pkt_access_flags,
const struct bpf_prog *prog)
{
@@ -9182,8 +9170,10 @@ static int tc_cls_act_prologue(struct bpf_insn *insn_buf, u32 pkt_access_flags,
* r0 = bpf_skb_meta_realign(r1); // r0 is undefined
* r1 = r6;
*/
+ BUILD_BUG_ON(!__same_type(&bpf_skb_meta_realign,
+ (void (*)(struct sk_buff *skb))NULL));
*insn++ = BPF_MOV64_REG(BPF_REG_6, BPF_REG_1);
- *insn++ = BPF_CALL_KFUNC(0, bpf_skb_meta_realign_ids[0]);
+ *insn++ = BPF_EMIT_CALL(bpf_skb_meta_realign);
*insn++ = BPF_MOV64_REG(BPF_REG_1, BPF_REG_6);
}
cnt = bpf_unclone_prologue(insn, pkt_access_flags, prog, TC_ACT_SHOT);
Powered by blists - more mailing lists