Thread (9 messages) 9 messages, 2 authors, 14d ago

Re: [PATCH net 1/3 v2] net: Extend bpf_net_context lifetime to cover qdisc enqueue

From: Daniel Borkmann <daniel@iogearbox.net>
Date: 2026-06-29 14:01:07
Also in: bpf, linux-rt-devel, stable
Subsystem: bpf [general] (safe dynamic programs and tools), bpf [networking] (tcx & tc bpf, sock_addr), networking [general], the rest · Maintainers: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

On 6/29/26 3:48 PM, Daniel Borkmann wrote:
On 6/29/26 3:36 PM, Jamal Hadi Salim wrote:
quoted
On Mon, Jun 29, 2026 at 9:01 AM Daniel Borkmann [off-list ref] wrote:
quoted
On 6/29/26 12:21 PM, Jamal Hadi Salim wrote:
quoted
The bpf_net_context used by sch_handle_egress() is stack-allocated and torn
down in that function returned. By the time tcf_qevent_handle() runs
current->bpf_net_context is NULL.

When a filter attached to a qevent block (e.g. RED's early_drop or mark
qevents, which always use shared blocks) returns TC_ACT_REDIRECT,
tcf_qevent_handle() calls skb_do_redirect(), which in turn calls bpf helper
bpf_net_ctx_get_ri().  That helper unconditionally dereferences
current->bpf_net_context resulting in a NULL pointer dereference.

Note: The same holds for actions that invoke BPF redirect helpers
(e.g. act_bpf running a program that calls bpf_redirect()) during qevent
classification itself.

Fix:
Move the bpf_net_context lifecycle out of sch_handle_egress() into
__dev_queue_xmit(), so that it spans both the egress TC fast path and the
qdisc enqueue.
Note: The call is placed outside the egress_needed_key static branch
to cover the case where clsact static key is disabled. Unfortunately this
adds a small unconditional penalty to the code path _per packet_ only
guarded by CONFIG_NET_XGRESS (two writes and one read).

As pointed by sashiko [1]:
The same context must also be set up in net_tx_action()'s qdisc drain
path, since qdisc_run() -> netem_dequeue() -> qdisc_enqueue( RED child)
can trigger qevent classification asynchronously from softirq context.

This keeps all bpf_net_context management in net/core/dev.c i.e the
existing boundary between tc core and BPF without requiring any net/sched/
code to know about BPF plumbing.

Reproducer:

    tc qdisc add dev eth0 root handle 1: red limit 1MB min 10KB max 20KB \
        avpkt 1000 burst 100 qevent early_drop block 10
    tc filter add block 10 pref 1 bpf obj redirect.o

    traffic through eth0 triggers red_enqueue() -> tcf_qevent_handle() and,
    on a redirect verdict, a NULL deref in skb_do_redirect().

Fixes: 3625750f05ec ("net: sched: Introduce helpers for qevent blocks")
Tested-by: Victor Nogueira <redacted>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Could we simplify patch 1 & 2 by just moving the bpf_net_ctx_set() and
bpf_net_ctx_clear() into a tcf_classify_qdisc() wrapper where we don't
end up having to touch the core TX code?

Untested diff :
This is bpf plumbing which doesnt belong in tc really. You already
moved most ebpf/clsact stuff into dev.c - let's just keep it there.

As a side note: calling a hierachy of N qdiscs we would incur N
set/clear cycles for N levels — and worse, qdiscs like HTB and HFSC
iterate filters while loop calling tcf_classify_qdisc() per iteration,
so each filter chain traversal does set/clear per proto.
I'm just saying that this is a lot simpler and taken out of the core fast
path. Also, I think you forgot to Cc Sebastian on the whole v2 given the
bpf_net_ctx_{set,clear} dance. Imho, having them via tcf_classify_qdisc or
something similar would be the much better choice compared to sprinkling
ifdefs since you want to block the TC_ACT_REDIRECT from classifiers attached
to qdiscs.

Q @ Sebastian, we could probably also do sth like this, wdyt between the
two options :
diff --git a/net/core/filter.c b/net/core/filter.c
index b446aa8be5c3..04683569ee0c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2554,7 +2554,7 @@ BPF_CALL_2(bpf_redirect, u32, ifindex, u64, flags)
  {
         struct bpf_redirect_info *ri = bpf_net_ctx_get_ri();
  
-       if (unlikely(flags & (~(BPF_F_INGRESS) | BPF_F_REDIRECT_INTERNAL)))
+       if (unlikely(!ri || (flags & (~(BPF_F_INGRESS) | BPF_F_REDIRECT_INTERNAL))))
                 return TC_ACT_SHOT;
  
         ri->flags = flags;
@@ -2575,7 +2575,7 @@ BPF_CALL_2(bpf_redirect_peer, u32, ifindex, u64, flags)
  {
         struct bpf_redirect_info *ri = bpf_net_ctx_get_ri();
  
-       if (unlikely(flags))
+       if (unlikely(!ri || flags))
                 return TC_ACT_SHOT;
  
         ri->flags = BPF_F_PEER;
@@ -2597,7 +2597,7 @@ BPF_CALL_4(bpf_redirect_neigh, u32, ifindex, struct bpf_redir_neigh *, params,
  {
         struct bpf_redirect_info *ri = bpf_net_ctx_get_ri();
  
-       if (unlikely((plen && plen < sizeof(*params)) || flags))
+       if (unlikely((plen && plen < sizeof(*params)) || !ri || flags))
                 return TC_ACT_SHOT;
  
         ri->flags = BPF_F_NEIGH | (plen ? BPF_F_NEXTHOP : 0);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help