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 13:36:47
Also in:
bpf, linux-rt-devel, stable
On Mon, Jun 29, 2026 at 9:01 AM Daniel Borkmann [off-list ref] wrote:
Hi Jamal, 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. cheers, jamal
quoted hunk ↗ jump to hunk
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index 3bd08d7f39c1..1828cc16c5d7 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h@@ -93,6 +93,8 @@ int tcf_classify(struct sk_buff *skb, const struct tcf_block *block, const struct tcf_proto *tp, struct tcf_result *res, bool compat_mode); +int tcf_classify_qdisc(struct sk_buff *skb, const struct tcf_proto *tp, + struct tcf_result *res, bool compat_mode); static inline bool tc_cls_stats_dump(struct tcf_proto *tp, struct tcf_walker *arg,@@ -157,6 +159,13 @@ static inline int tcf_classify(struct sk_buff *skb, return TC_ACT_UNSPEC; } +static inline int tcf_classify_qdisc(struct sk_buff *skb, + const struct tcf_proto *tp, + struct tcf_result *res, bool compat_mode) +{ + return tcf_classify(skb, NULL, tp, res, compat_mode); +} + #endif static inline unsigned longdiff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 3e67600a4a1a..982409702c7f 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c@@ -23,6 +23,7 @@ #include <linux/jhash.h> #include <linux/rculist.h> #include <linux/rhashtable.h> +#include <linux/filter.h> #include <net/net_namespace.h> #include <net/sock.h> #include <net/netlink.h>@@ -1884,6 +1885,24 @@ int tcf_classify(struct sk_buff *skb, } EXPORT_SYMBOL(tcf_classify); +int tcf_classify_qdisc(struct sk_buff *skb, const struct tcf_proto *tp, + struct tcf_result *res, bool compat_mode) +{ + struct bpf_net_context __bpf_net_ctx, *bpf_net_ctx; + int ret; + + bpf_net_ctx = bpf_net_ctx_set(&__bpf_net_ctx); + ret = tcf_classify(skb, NULL, tp, res, compat_mode); + bpf_net_ctx_clear(bpf_net_ctx); + + if (unlikely(ret == TC_ACT_REDIRECT)) { + pr_warn_once("TC_ACT_REDIRECT from qdisc filter chains is not supported\n"); + ret = TC_ACT_SHOT; + } + return ret; +} +EXPORT_SYMBOL(tcf_classify_qdisc); + struct tcf_chain_info { struct tcf_proto __rcu **pprev; struct tcf_proto __rcu *next;@@ -4033,7 +4052,7 @@ struct sk_buff *tcf_qevent_handle(struct tcf_qevent *qe, struct Qdisc *sch, stru fl = rcu_dereference_bh(qe->filter_chain); - switch (tcf_classify(skb, NULL, fl, &cl_res, false)) { + switch (tcf_classify_qdisc(skb, fl, &cl_res, false)) { case TC_ACT_SHOT: qdisc_qstats_drop(sch); __qdisc_drop(skb, to_free);@@ -4045,10 +4064,6 @@ struct sk_buff *tcf_qevent_handle(struct tcf_qevent *qe, struct Qdisc *sch, stru __qdisc_drop(skb, to_free); *ret = __NET_XMIT_STOLEN; return NULL; - case TC_ACT_REDIRECT: - skb_do_redirect(skb); - *ret = __NET_XMIT_STOLEN; - return NULL; case TC_ACT_CONSUMED: *ret = __NET_XMIT_STOLEN; return NULL;diff --git a/net/sched/sch_cake.c b/net/sched/sch_cake.c index a3c185505afc..94eb47ac54ee 100644 --- a/net/sched/sch_cake.c +++ b/net/sched/sch_cake.c@@ -1730,7 +1730,7 @@ static u32 cake_classify(struct Qdisc *sch, struct cake_tin_data **t, goto hash; *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; - result = tcf_classify(skb, NULL, filter, &res, false); + result = tcf_classify_qdisc(skb, filter, &res, false); if (result >= 0) { #ifdef CONFIG_NET_CLS_ACTdiff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c index 020657f959b5..91b1ef824afa 100644 --- a/net/sched/sch_drr.c +++ b/net/sched/sch_drr.c@@ -312,7 +312,7 @@ static struct drr_class *drr_classify(struct sk_buff *skb, struct Qdisc *sch, *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; fl = rcu_dereference_bh(q->filter_list); - result = tcf_classify(skb, NULL, fl, &res, false); + result = tcf_classify_qdisc(skb, fl, &res, false); if (result >= 0) { #ifdef CONFIG_NET_CLS_ACT switch (result) {diff --git a/net/sched/sch_dualpi2.c b/net/sched/sch_dualpi2.c index 5434df6ca8ef..98364f74211e 100644 --- a/net/sched/sch_dualpi2.c +++ b/net/sched/sch_dualpi2.c@@ -364,7 +364,7 @@ static int dualpi2_skb_classify(struct dualpi2_sched_data *q, return NET_XMIT_SUCCESS; } - result = tcf_classify(skb, NULL, fl, &res, false); + result = tcf_classify_qdisc(skb, fl, &res, false); if (result >= 0) { #ifdef CONFIG_NET_CLS_ACT switch (result) {diff --git a/net/sched/sch_ets.c b/net/sched/sch_ets.c index cb8cf437ce87..25fcf4079fec 100644 --- a/net/sched/sch_ets.c +++ b/net/sched/sch_ets.c@@ -391,7 +391,7 @@ static struct ets_class *ets_classify(struct sk_buff *skb, struct Qdisc *sch, *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; if (TC_H_MAJ(skb->priority) != sch->handle) { fl = rcu_dereference_bh(q->filter_list); - err = tcf_classify(skb, NULL, fl, &res, false); + err = tcf_classify_qdisc(skb, fl, &res, false); #ifdef CONFIG_NET_CLS_ACT switch (err) { case TC_ACT_STOLEN:diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c index cafd1f943d99..6cce86ba383c 100644 --- a/net/sched/sch_fq_codel.c +++ b/net/sched/sch_fq_codel.c@@ -91,7 +91,7 @@ static unsigned int fq_codel_classify(struct sk_buff *skb, struct Qdisc *sch, return fq_codel_hash(q, skb) + 1; *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; - result = tcf_classify(skb, NULL, filter, &res, false); + result = tcf_classify_qdisc(skb, filter, &res, false); if (result >= 0) { #ifdef CONFIG_NET_CLS_ACT switch (result) {diff --git a/net/sched/sch_fq_pie.c b/net/sched/sch_fq_pie.c index 72f48fa4010b..069e1facd413 100644 --- a/net/sched/sch_fq_pie.c +++ b/net/sched/sch_fq_pie.c@@ -96,7 +96,7 @@ static unsigned int fq_pie_classify(struct sk_buff *skb, struct Qdisc *sch, return fq_pie_hash(q, skb) + 1; *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; - result = tcf_classify(skb, NULL, filter, &res, false); + result = tcf_classify_qdisc(skb, filter, &res, false); if (result >= 0) { #ifdef CONFIG_NET_CLS_ACT switch (result) {diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c index 7e537295b8b6..e87f5021a199 100644 --- a/net/sched/sch_hfsc.c +++ b/net/sched/sch_hfsc.c@@ -1143,7 +1143,7 @@ hfsc_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr) *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; head = &q->root; tcf = rcu_dereference_bh(q->root.filter_list); - while (tcf && (result = tcf_classify(skb, NULL, tcf, &res, false)) >= 0) { + while (tcf && (result = tcf_classify_qdisc(skb, tcf, &res, false)) >= 0) { #ifdef CONFIG_NET_CLS_ACT switch (result) { case TC_ACT_QUEUED:diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c index 908b9ba9ba2e..fdac0dc8f35a 100644 --- a/net/sched/sch_htb.c +++ b/net/sched/sch_htb.c@@ -243,7 +243,7 @@ static struct htb_class *htb_classify(struct sk_buff *skb, struct Qdisc *sch, } *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; - while (tcf && (result = tcf_classify(skb, NULL, tcf, &res, false)) >= 0) { + while (tcf && (result = tcf_classify_qdisc(skb, tcf, &res, false)) >= 0) { #ifdef CONFIG_NET_CLS_ACT switch (result) { case TC_ACT_QUEUED:diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c index 4e465d11e3d7..004f0d275caf 100644 --- a/net/sched/sch_multiq.c +++ b/net/sched/sch_multiq.c@@ -36,7 +36,7 @@ multiq_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr) int err; *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; - err = tcf_classify(skb, NULL, fl, &res, false); + err = tcf_classify_qdisc(skb, fl, &res, false); #ifdef CONFIG_NET_CLS_ACT switch (err) { case TC_ACT_STOLEN:diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c index e4dd56a89072..79437c587e7e 100644 --- a/net/sched/sch_prio.c +++ b/net/sched/sch_prio.c@@ -39,7 +39,7 @@ prio_classify(struct sk_buff *skb, struct Qdisc *sch, int *qerr) *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; if (TC_H_MAJ(skb->priority) != sch->handle) { fl = rcu_dereference_bh(q->filter_list); - err = tcf_classify(skb, NULL, fl, &res, false); + err = tcf_classify_qdisc(skb, fl, &res, false); #ifdef CONFIG_NET_CLS_ACT switch (err) { case TC_ACT_STOLEN:diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c index cb56787e1d25..6f3b7273cb16 100644 --- a/net/sched/sch_qfq.c +++ b/net/sched/sch_qfq.c@@ -709,7 +709,7 @@ static struct qfq_class *qfq_classify(struct sk_buff *skb, struct Qdisc *sch, *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; fl = rcu_dereference_bh(q->filter_list); - result = tcf_classify(skb, NULL, fl, &res, false); + result = tcf_classify_qdisc(skb, fl, &res, false); if (result >= 0) { #ifdef CONFIG_NET_CLS_ACT switch (result) {diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c index b1d465094276..ed39869199c0 100644 --- a/net/sched/sch_sfb.c +++ b/net/sched/sch_sfb.c@@ -260,7 +260,7 @@ static bool sfb_classify(struct sk_buff *skb, struct tcf_proto *fl, struct tcf_result res; int result; - result = tcf_classify(skb, NULL, fl, &res, false); + result = tcf_classify_qdisc(skb, fl, &res, false); if (result >= 0) { #ifdef CONFIG_NET_CLS_ACT switch (result) {diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c index 758b88f21865..77675f9a4c46 100644 --- a/net/sched/sch_sfq.c +++ b/net/sched/sch_sfq.c@@ -171,7 +171,7 @@ static unsigned int sfq_classify(struct sk_buff *skb, struct Qdisc *sch, return sfq_hash(q, skb) + 1; *qerr = NET_XMIT_SUCCESS | __NET_XMIT_BYPASS; - result = tcf_classify(skb, NULL, fl, &res, false); + result = tcf_classify_qdisc(skb, fl, &res, false); if (result >= 0) { #ifdef CONFIG_NET_CLS_ACT switch (result) {Thanks, Daniel