[PATCH net] net/sched: act_api: fix TOCTOU NULL deref on a->goto_chain

[PATCH net] net/sched: act_api: fix TOCTOU NULL deref on a->goto_chain

[Jamal Hadi Salim]

tcf_action_exec() handles TC_ACT_GOTO_CHAIN by first checking
rcu_access_pointer(a->goto_chain) and then calling
tcf_action_goto_chain_exec(), which does a second, independent
rcu_dereference_bh(a->goto_chain) read and immediately dereferences
chain->filter_chain. A concurrent tcf_action_set_ctrlact() (e.g. the gact
replace path) can clear a->goto_chain between the two reads, so the second
read returns NULL and tcf_action_goto_chain_exec() dereferences NULL.

Fix the race by doing a single rcu_dereference_bh() read of a->goto_chain
in tcf_action_exec(), checking it once for NULL, and passing the resulting
chain pointer into tcf_action_goto_chain_exec(). This turns the split
check/use into a single check/use on one value.

Fixes: ee3bbfe806cd ("net/sched: let actions use RCU to access 'goto_chain'")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/act_api.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f141634df214..600b7804befd 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -41,11 +41,9 @@ int tcf_dev_queue_xmit(struct sk_buff *skb, int (*xmit)(struct sk_buff *skb))
 }
 EXPORT_SYMBOL_GPL(tcf_dev_queue_xmit);
 
-static void tcf_action_goto_chain_exec(const struct tc_action *a,
+static void tcf_action_goto_chain_exec(const struct tcf_chain *chain,
 				       struct tcf_result *res)
 {
-	const struct tcf_chain *chain = rcu_dereference_bh(a->goto_chain);
-
 	res->goto_tp = rcu_dereference_bh(chain->filter_chain);
 }
 
@@ -1170,12 +1168,14 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
 					return TC_ACT_OK;
 			}
 		} else if (TC_ACT_EXT_CMP(ret, TC_ACT_GOTO_CHAIN)) {
-			if (unlikely(!rcu_access_pointer(a->goto_chain))) {
+			struct tcf_chain *chain = rcu_dereference_bh(a->goto_chain);
+
+			if (unlikely(!chain)) {
 				tcf_set_drop_reason(skb,
 						    SKB_DROP_REASON_TC_CHAIN_NOTFOUND);
 				return TC_ACT_SHOT;
 			}
-			tcf_action_goto_chain_exec(a, res);
+			tcf_action_goto_chain_exec(chain, res);
 		}
 
 		if (ret != TC_ACT_PIPE)
-- 
2.34.1

Re: [PATCH net] net/sched: act_api: fix TOCTOU NULL deref on a->goto_chain

[Davide Caratti]

On Sun, Aug 09, 2026 at 05:09:28AM -0400, Jamal Hadi Salim wrote:
> tcf_action_exec() handles TC_ACT_GOTO_CHAIN by first checking
> rcu_access_pointer(a->goto_chain) and then calling
> tcf_action_goto_chain_exec(), which does a second, independent
> rcu_dereference_bh(a->goto_chain) read and immediately dereferences
> chain->filter_chain. A concurrent tcf_action_set_ctrlact() (e.g. the gact
> replace path) can clear a->goto_chain between the two reads, so the second
> read returns NULL and tcf_action_goto_chain_exec() dereferences NULL.
> 
> Fix the race by doing a single rcu_dereference_bh() read of a->goto_chain
> in tcf_action_exec(), checking it once for NULL, and passing the resulting
> chain pointer into tcf_action_goto_chain_exec(). This turns the split
> check/use into a single check/use on one value.
> 
> Fixes: ee3bbfe806cd ("net/sched: let actions use RCU to access 'goto_chain'")
> Reported-by: vega@nebusec.ai
> Tested-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

[...]

Reviewed-by: Davide Caratti <dcaratti@redhat.com>

Thanks!

-- 
davide

Re: [PATCH net] net/sched: act_api: fix TOCTOU NULL deref on a->goto_chain

[Jakub Kicinski]

On Sun,  9 Aug 2026 05:09:28 -0400 Jamal Hadi Salim wrote:
> tcf_action_exec() handles TC_ACT_GOTO_CHAIN by first checking
> rcu_access_pointer(a->goto_chain) and then calling
> tcf_action_goto_chain_exec(), which does a second, independent
> rcu_dereference_bh(a->goto_chain) read and immediately dereferences
> chain->filter_chain. A concurrent tcf_action_set_ctrlact() (e.g. the gact
> replace path) can clear a->goto_chain between the two reads, so the second
> read returns NULL and tcf_action_goto_chain_exec() dereferences NULL.

FWIW *shiko suggests another tweak but looks orthogonal, LMK if you
disagree:

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260809090928.868186-1-jhs@mojatatu.com

(the patch is "too fresh" for me to apply right now anyway)

Re: [PATCH net] net/sched: act_api: fix TOCTOU NULL deref on a->goto_chain

[Jamal Hadi Salim]

On Mon, Aug 10, 2026 at 7:45 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Sun,  9 Aug 2026 05:09:28 -0400 Jamal Hadi Salim wrote:
> > tcf_action_exec() handles TC_ACT_GOTO_CHAIN by first checking
> > rcu_access_pointer(a->goto_chain) and then calling
> > tcf_action_goto_chain_exec(), which does a second, independent
> > rcu_dereference_bh(a->goto_chain) read and immediately dereferences
> > chain->filter_chain. A concurrent tcf_action_set_ctrlact() (e.g. the gact
> > replace path) can clear a->goto_chain between the two reads, so the second
> > read returns NULL and tcf_action_goto_chain_exec() dereferences NULL.
>
> FWIW *shiko suggests another tweak but looks orthogonal, LMK if you
> disagree:
>
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260809090928.868186-1-jhs@mojatatu.com
>
> (the patch is "too fresh" for me to apply right now anyway)

The concern is valid but pre-existing and orthogonal to this patch
(and a lot less severe than the posted fix)
I had this discussion with Paolo: When the sashikos raise a concern on
"pre-existing" issues, what should be the reaction?
In general the conclusion was to follow up later if worth it; however,
sometimes we need to make a judgement call - if the pointed to issue
is serious (and yes, the AI bots are now reading what Sashikos are
saying  and constructing bug reports) then a v2 is needed.
In this case, I was planning to follow up. I will start more actively
looking at sashiko reports and analyzing if worth a followup or a v2.
I dont know how to do these pw signals, but in case i see it as "needs
v2" it won't be worth waiting for one of you guys to comment.

Do we need a written policy somewhere?

cheers,
jamal

Re: [PATCH net] net/sched: act_api: fix TOCTOU NULL deref on a->goto_chain

[Jakub Kicinski]

On Tue, 11 Aug 2026 06:43:32 -0400 Jamal Hadi Salim wrote:
> On Mon, Aug 10, 2026 at 7:45 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Sun,  9 Aug 2026 05:09:28 -0400 Jamal Hadi Salim wrote:  
> > > tcf_action_exec() handles TC_ACT_GOTO_CHAIN by first checking
> > > rcu_access_pointer(a->goto_chain) and then calling
> > > tcf_action_goto_chain_exec(), which does a second, independent
> > > rcu_dereference_bh(a->goto_chain) read and immediately dereferences
> > > chain->filter_chain. A concurrent tcf_action_set_ctrlact() (e.g. the gact
> > > replace path) can clear a->goto_chain between the two reads, so the second
> > > read returns NULL and tcf_action_goto_chain_exec() dereferences NULL.  
> >
> > FWIW *shiko suggests another tweak but looks orthogonal, LMK if you
> > disagree:
> >
> > https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260809090928.868186-1-jhs@mojatatu.com
> >
> > (the patch is "too fresh" for me to apply right now anyway)  
> 
> The concern is valid but pre-existing and orthogonal to this patch
> (and a lot less severe than the posted fix)
> I had this discussion with Paolo: When the sashikos raise a concern on
> "pre-existing" issues, what should be the reaction?
> In general the conclusion was to follow up later if worth it; however,
> sometimes we need to make a judgement call - if the pointed to issue
> is serious (and yes, the AI bots are now reading what Sashikos are
> saying  and constructing bug reports) then a v2 is needed.
> In this case, I was planning to follow up. I will start more actively
> looking at sashiko reports and analyzing if worth a followup or a v2.
> I dont know how to do these pw signals, but in case i see it as "needs
> v2" it won't be worth waiting for one of you guys to comment.

Right, I was hoping my question was clear enough. I don't think v2 was
needed here either. But we had time to confirm...

> Do we need a written policy somewhere?

The only written policy should be that everyone who asks for a written
policy in this rapidly changing environment owes maintainers a beer :)

More seriously I tried to float two written policies recently - for net
vs net-next and requirements for information in fixes. And each time
there was a long discussion and questions. So y'all need to either
stop making written policies so painful, or stop asking for the
policies. *%$#.

Re: [PATCH net] net/sched: act_api: fix TOCTOU NULL deref on a->goto_chain

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Sun,  9 Aug 2026 05:09:28 -0400 you wrote:
> tcf_action_exec() handles TC_ACT_GOTO_CHAIN by first checking
> rcu_access_pointer(a->goto_chain) and then calling
> tcf_action_goto_chain_exec(), which does a second, independent
> rcu_dereference_bh(a->goto_chain) read and immediately dereferences
> chain->filter_chain. A concurrent tcf_action_set_ctrlact() (e.g. the gact
> replace path) can clear a->goto_chain between the two reads, so the second
> read returns NULL and tcf_action_goto_chain_exec() dereferences NULL.
> 
> [...]

Here is the summary with links:
  - [net] net/sched: act_api: fix TOCTOU NULL deref on a->goto_chain
    https://git.kernel.org/netdev/net/c/f60b396ee174

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH net] net/sched: act_api: fix TOCTOU NULL deref on a->goto_chain

[Jamal Hadi Salim]

tcf_action_exec() handles TC_ACT_GOTO_CHAIN by first checking
rcu_access_pointer(a->goto_chain) and then calling
tcf_action_goto_chain_exec(), which does a second, independent
rcu_dereference_bh(a->goto_chain) read and immediately dereferences
chain->filter_chain. A concurrent tcf_action_set_ctrlact() (e.g. the gact
replace path) can clear a->goto_chain between the two reads, so the second
read returns NULL and tcf_action_goto_chain_exec() dereferences NULL.

Fix the race by doing a single rcu_dereference_bh() read of a->goto_chain
in tcf_action_exec(), checking it once for NULL, and passing the resulting
chain pointer into tcf_action_goto_chain_exec(). This turns the split
check/use into a single check/use on one value.

Fixes: ee3bbfe806cd ("net/sched: let actions use RCU to access 'goto_chain'")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/act_api.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f141634df214..600b7804befd 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -41,11 +41,9 @@ int tcf_dev_queue_xmit(struct sk_buff *skb, int (*xmit)(struct sk_buff *skb))
 }
 EXPORT_SYMBOL_GPL(tcf_dev_queue_xmit);
 
-static void tcf_action_goto_chain_exec(const struct tc_action *a,
+static void tcf_action_goto_chain_exec(const struct tcf_chain *chain,
 				       struct tcf_result *res)
 {
-	const struct tcf_chain *chain = rcu_dereference_bh(a->goto_chain);
-
 	res->goto_tp = rcu_dereference_bh(chain->filter_chain);
 }
 
@@ -1170,12 +1168,14 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
 					return TC_ACT_OK;
 			}
 		} else if (TC_ACT_EXT_CMP(ret, TC_ACT_GOTO_CHAIN)) {
-			if (unlikely(!rcu_access_pointer(a->goto_chain))) {
+			struct tcf_chain *chain = rcu_dereference_bh(a->goto_chain);
+
+			if (unlikely(!chain)) {
 				tcf_set_drop_reason(skb,
 						    SKB_DROP_REASON_TC_CHAIN_NOTFOUND);
 				return TC_ACT_SHOT;
 			}
-			tcf_action_goto_chain_exec(a, res);
+			tcf_action_goto_chain_exec(chain, res);
 		}
 
 		if (ret != TC_ACT_PIPE)
-- 
2.34.1