public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/sched: act_mirred: Fix blockcast recursion bypass leading to stack overflow
@ 2026-04-13  8:20 Kito Xu (veritas501)
  2026-04-15 14:19 ` Simon Horman
  0 siblings, 1 reply; 3+ messages in thread
From: Kito Xu (veritas501) @ 2026-04-13  8:20 UTC (permalink / raw)
  To: jhs, jiri, davem, edumazet, kuba, pabeni
  Cc: horms, victor, pctammela, netdev, linux-kernel,
	Kito Xu (veritas501)

tcf_mirred_act() checks sched_mirred_nest against MIRRED_NEST_LIMIT (4)
to prevent deep recursion.  However, when the action uses blockcast
(tcfm_blockid != 0), the function returns at the tcf_blockcast() call
BEFORE reaching the counter increment.  As a result, the recursion
counter never advances and the limit check is entirely bypassed.

When two devices share a TC egress block with a mirred blockcast rule,
a packet egressing on device A is mirrored to device B via blockcast;
device B's egress TC re-enters tcf_mirred_act() via blockcast and
mirrors back to A, creating an unbounded recursion loop:

  tcf_mirred_act -> tcf_blockcast -> tcf_mirred_to_dev -> dev_queue_xmit
  -> sch_handle_egress -> tcf_classify -> tcf_mirred_act -> (repeat)

This recursion continues until the kernel stack overflows.

The bug is reachable from an unprivileged user via
unshare(CLONE_NEWUSER | CLONE_NEWNET): user namespaces grant
CAP_NET_ADMIN in the new network namespace, which is sufficient to
create dummy devices, attach clsact qdiscs with shared blocks, and
install mirred blockcast filters.

 BUG: TASK stack guard page was hit at ffffc90000b7fff8
 Oops: stack guard page: 0000 [#1] SMP KASAN NOPTI
 CPU: 2 UID: 1000 PID: 169 Comm: poc Not tainted 7.0.0-rc7-next-20260410
 RIP: 0010:xas_find+0x17/0x480
 Call Trace:
  xa_find+0x17b/0x1d0
  tcf_mirred_act+0x640/0x1060
  tcf_action_exec+0x400/0x530
  basic_classify+0x128/0x1d0
  tcf_classify+0xd83/0x1150
  tc_run+0x328/0x620
  __dev_queue_xmit+0x797/0x3100
  tcf_mirred_to_dev+0x7b1/0xf70
  tcf_mirred_act+0x68a/0x1060
  [repeating ~30+ times until stack overflow]
 Kernel panic - not syncing: Fatal exception in interrupt

Fix this by incrementing sched_mirred_nest before calling
tcf_blockcast() and decrementing it on return, mirroring the
non-blockcast path.  This ensures subsequent recursive entries see the
updated counter and are correctly limited by MIRRED_NEST_LIMIT.

Fixes: 42f39036cda8 ("net/sched: act_mirred: Allow mirred to block")
Signed-off-by: Kito Xu (veritas501) <hxzene@gmail.com>
---
 net/sched/act_mirred.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 05e0b14b5773..5928fcf3e651 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -444,8 +444,12 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
 	tcf_action_update_bstats(&m->common, skb);
 
 	blockid = READ_ONCE(m->tcfm_blockid);
-	if (blockid)
-		return tcf_blockcast(skb, m, blockid, res, retval);
+	if (blockid) {
+		xmit->sched_mirred_nest++;
+		retval = tcf_blockcast(skb, m, blockid, res, retval);
+		xmit->sched_mirred_nest--;
+		return retval;
+	}
 
 	dev = rcu_dereference_bh(m->tcfm_dev);
 	if (unlikely(!dev)) {
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] net/sched: act_mirred: Fix blockcast recursion bypass leading to stack overflow
  2026-04-13  8:20 [PATCH] net/sched: act_mirred: Fix blockcast recursion bypass leading to stack overflow Kito Xu (veritas501)
@ 2026-04-15 14:19 ` Simon Horman
  2026-04-16 15:23   ` Jamal Hadi Salim
  0 siblings, 1 reply; 3+ messages in thread
From: Simon Horman @ 2026-04-15 14:19 UTC (permalink / raw)
  To: hxzene
  Cc: 'Simon Horman', jhs, jiri, davem, edumazet, kuba, pabeni,
	victor, pctammela, netdev, linux-kernel

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
net/sched: act_mirred: Fix blockcast recursion bypass leading to stack overflow

This patch fixes a stack overflow bug where mirred blockcast actions would
bypass the recursion limit check because the recursion counter was not
incremented for blockcast actions.

> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index 05e0b14b57731..5928fcf3e6514 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -444,8 +444,12 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
>  	tcf_action_update_bstats(&m->common, skb);
>  
>  	blockid = READ_ONCE(m->tcfm_blockid);
> -	if (blockid)
> -		return tcf_blockcast(skb, m, blockid, res, retval);
> +	if (blockid) {
> +		xmit->sched_mirred_nest++;

Could this leave a stale pointer in sched_mirred_dev since netdev_xmit is
persistent per-CPU (or per-task in PREEMPT_RT) memory?

If a nested mirred action later iterates over sched_mirred_dev up to the new
nesting depth to detect routing loops, could it match a stale pointer from
whatever unrelated packet previously reached this nesting level?

This might cause a false-positive loop detection and incorrectly drop a valid
packet. Should we set xmit->sched_mirred_dev[xmit->sched_mirred_nest] = NULL
before incrementing the counter?

> +		retval = tcf_blockcast(skb, m, blockid, res, retval);

This isn't a regression introduced by this patch, but it exists in the early
error returns of tcf_mirred_act() (e.g., when the target device is gone, or a
routing loop is detected) and in tcf_blockcast() (e.g., when
xa_empty(&block->ports) is true).

In these error paths, the functions simply return retval. For mirred redirect
actions, retval is initialized to m->tcf_action, which is typically
TC_ACT_STOLEN.

When TC_ACT_STOLEN is returned, the TC framework assumes the action has taken
ownership of the packet and will not free the sk_buff. Since the code does
not free the sk_buff before returning in these error paths, does this leak
the sk_buff?

Could an unprivileged user inside a network namespace trigger this
intentionally by creating a routing loop with mirred redirect rules, leading
to continuous sk_buff leaks and system memory exhaustion?

Should these error paths ensure the sk_buff is freed or force the return value
to TC_ACT_SHOT if the action is a redirect, mirroring the existing behavior in
tcf_mirred_to_dev()?

> +		xmit->sched_mirred_nest--;
> +		return retval;
> +	}

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] net/sched: act_mirred: Fix blockcast recursion bypass leading to stack overflow
  2026-04-15 14:19 ` Simon Horman
@ 2026-04-16 15:23   ` Jamal Hadi Salim
  0 siblings, 0 replies; 3+ messages in thread
From: Jamal Hadi Salim @ 2026-04-16 15:23 UTC (permalink / raw)
  To: Simon Horman
  Cc: hxzene, jiri, davem, edumazet, kuba, pabeni, victor, pctammela,
	netdev, linux-kernel, Stephen Hemminger

On Wed, Apr 15, 2026 at 10:19 AM Simon Horman <horms@kernel.org> wrote:
>
> From: 'Simon Horman' <horms@kernel.org>
>
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev
> ---
> net/sched: act_mirred: Fix blockcast recursion bypass leading to stack overflow
>
> This patch fixes a stack overflow bug where mirred blockcast actions would
> bypass the recursion limit check because the recursion counter was not
> incremented for blockcast actions.
>
> > diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> > index 05e0b14b57731..5928fcf3e6514 100644
> > --- a/net/sched/act_mirred.c
> > +++ b/net/sched/act_mirred.c
> > @@ -444,8 +444,12 @@ TC_INDIRECT_SCOPE int tcf_mirred_act(struct sk_buff *skb,
> >       tcf_action_update_bstats(&m->common, skb);
> >
> >       blockid = READ_ONCE(m->tcfm_blockid);
> > -     if (blockid)
> > -             return tcf_blockcast(skb, m, blockid, res, retval);
> > +     if (blockid) {
> > +             xmit->sched_mirred_nest++;
>
> Could this leave a stale pointer in sched_mirred_dev since netdev_xmit is
> persistent per-CPU (or per-task in PREEMPT_RT) memory?
>
> If a nested mirred action later iterates over sched_mirred_dev up to the new
> nesting depth to detect routing loops, could it match a stale pointer from
> whatever unrelated packet previously reached this nesting level?
>
> This might cause a false-positive loop detection and incorrectly drop a valid
> packet. Should we set xmit->sched_mirred_dev[xmit->sched_mirred_nest] = NULL
> before incrementing the counter?
>
> > +             retval = tcf_blockcast(skb, m, blockid, res, retval);
>
> This isn't a regression introduced by this patch, but it exists in the early
> error returns of tcf_mirred_act() (e.g., when the target device is gone, or a
> routing loop is detected) and in tcf_blockcast() (e.g., when
> xa_empty(&block->ports) is true).
>
> In these error paths, the functions simply return retval. For mirred redirect
> actions, retval is initialized to m->tcf_action, which is typically
> TC_ACT_STOLEN.
>
> When TC_ACT_STOLEN is returned, the TC framework assumes the action has taken
> ownership of the packet and will not free the sk_buff. Since the code does
> not free the sk_buff before returning in these error paths, does this leak
> the sk_buff?
>
> Could an unprivileged user inside a network namespace trigger this
> intentionally by creating a routing loop with mirred redirect rules, leading
> to continuous sk_buff leaks and system memory exhaustion?
>
> Should these error paths ensure the sk_buff is freed or force the return value
> to TC_ACT_SHOT if the action is a redirect, mirroring the existing behavior in
> tcf_mirred_to_dev()?
>
> > +             xmit->sched_mirred_nest--;
> > +             return retval;
> > +     }

There's a dependency with the skb bits patch[1] with this one.
Stephen, I will take this set and consider what Sashiko is saying here
[2]. I just became aware of this - but it seems to have some merit.
I will also review this patch and sashiko's input from that
perspective. If it merits it, I will resend the set incorporating this
fix and Sashiko's suggestions.

cheers,
jamal

[1]https://lore.kernel.org/netdev/20260326181701.308275-1-stephen@networkplumber.org/
[2] https://sashiko.dev/#/patchset/20260326181701.308275-1-stephen%40networkplumber.org

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-04-16 15:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-13  8:20 [PATCH] net/sched: act_mirred: Fix blockcast recursion bypass leading to stack overflow Kito Xu (veritas501)
2026-04-15 14:19 ` Simon Horman
2026-04-16 15:23   ` Jamal Hadi Salim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox