Re: [PATCH net 1/3 v2] net: Extend bpf_net_context lifetime to cover qdisc enqueue
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: 2026-06-29 14:08:47
Also in:
bpf, linux-rt-devel, stable
On Mon, Jun 29, 2026 at 9:49 AM Daniel Borkmann [off-list ref] 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.
And you are clearly not listening to what i said. Something similar would be fine - but stop using tc as your dumping ground. cheers, jamal