[PATCH net-next 0/2] net/sched: act_gate: reject oversized dumps and cap entry count

[PATCH net-next 0/2] net/sched: act_gate: reject oversized dumps and cap entry count

[Jamal Hadi Salim]

An action dump is carried inside nested netlink attributes whose lengths
are u16. When a single action's dump does not fit, a plain nla_nest_end()
writes a wrapped length and the kernel hands userspace a corrupt reply; on
the classifier path the -EMSGSIZE this produces is mistaken for socket
buffer exhaustion and retried in a loop that can never succeed.

Patch 1 makes every nest an action dump can be carried in fail cleanly
(-EMSGSIZE) instead of wrapping, and bounds the tfilter_notify_prep()
retry. Patch 2 caps the number of gate scheduling entries at 1024,
rejected with -E2BIG and an extack message.

This patchset is a retarget from net to net-next of the previous posting:
https://lore.kernel.org/netdev/QDISC-H19Z.v1.20261001053234@mojatatu.com/
The wrapped-nla_len corruption this series addresses is not a regression of
commit cfa165cbfbed ("net/sched: act_gate: budget the per-entry list in
get_fill_size"): the enclosing action and filter nests antedate it and a
plain nla_nest_end() has always written the wrapped length at U16_MAX.
Under Linus's post-merge-window rule a [PATCH net] must be a regression or
critical, so this is hardening for net-next:

  https://lore.kernel.org/netdev/CAHk-=wiSnTE9vBZ=5_v+3EEkdazCCbBM5YABRzRHUAeRdyd4Xw@mail.gmail.com/

Changes since the first posting:
  - Retarget to net-next and drop Fixes:/Cc: stable, as it is hardening
    rather than a regression.
  - Add the entry-count cap into the series as patch 2/2, as the parent
    commit promised for net-next.  The cap value is 1024, grounded in the
    SJA1105 schedule-table capacity and the 16-bit netlink TLV boundary.
  - Patch 1: add seven additional action-capable classifier
    TCA_OPTIONS closes  and correct the Conditions Kconfig name.
  - Sashiko review of that posting:
    https://sashiko.dev/#/patchset/QDISC-H19Z.v1.20261001053234%40mojatatu.com

Jamal Hadi Salim (2):
  net/sched: act_gate: reject oversized dumps instead of wrapping them
  net/sched: act_gate: cap the number of scheduling entries

 net/sched/act_api.c      | 10 +++++++---
 net/sched/act_gate.c     | 22 +++++++++++++++++++++-
 net/sched/cls_api.c      | 40 ++++++++++++++++++++++++++++++----------
 net/sched/cls_basic.c    |  3 ++-
 net/sched/cls_bpf.c      |  3 ++-
 net/sched/cls_cgroup.c   |  3 ++-
 net/sched/cls_flow.c     |  3 ++-
 net/sched/cls_flower.c   |  6 ++++--
 net/sched/cls_fw.c       |  3 ++-
 net/sched/cls_matchall.c |  3 ++-
 net/sched/cls_route.c    |  3 ++-
 net/sched/cls_u32.c      |  3 ++-
 12 files changed, 78 insertions(+), 24 deletions(-)

-- 
2.43.0

[PATCH net-next 1/2] net/sched: act_gate: reject oversized dumps instead of wrapping them

[Jamal Hadi Salim]

This is a followup of commit cfa165cbfbed ("net/sched: act_gate: budget
the per-entry list in get_fill_size") as reported by Sashiko.

The issue:
An action dump is carried inside nested netlink attributes whose lengths
are u16. A gate action with enough schedule entries produces a dump
larger than U16_MAX, and the plain nla_nest_end() closes by writing a
wrapped length: the reply is structurally corrupt. On a CONFIG_DEBUG_NET
kernel nla_nest_end() also warns for each wrapped close, which an
unprivileged RTM_GETACTION or RTM_GETTFILTER can reach once the action
or filter exists.

The oversized dump also reaches the filter notification path through
tcf_fill_node(), which reports the failure as -EMSGSIZE.
tfilter_notify_prep() treats that as socket-buffer exhaustion and
retries with an ever larger alloc_skb(); a dump that does not fit a u16
nest cannot be built at any skb size, so the loop only spins until the
allocation itself fails.

The Fix:
Close every wrap-capable nest an action dump travels in with
nla_nest_end_safe(), which reports -EMSGSIZE before writing a wrapped
length: TCA_GATE_ENTRY_LIST, TCA_ACT_OPTIONS, the per-action nest and
TCA_ACT_TAB; the shared action and police containers in
tcf_exts_dump()/tcf_exts_terse_dump(); and the caller-owned TCA_OPTIONS
of every action-capable classifier (flower, matchall, basic, bpf,
cgroup, flow, fw, route4, u32). The output is byte identical for every
message that serializes. Bound the tfilter_notify_prep() retry once the
skb is already larger than any valid message, while a regular dump that
only needs a bigger skb still retries.

Context note:
A notification that cannot be serialised must not veto the state change
the caller asked for. tfilter_del_notify() builds the delete
notification before calling ->delete() and propagated the prep
failure, so a filter whose dump cannot be represented in a u16 nest
could be created but never removed by "tc filter del ... handle H" - and
only when a notification was actually needed (rtnl_notify_needed(): an
RTNLGRP_TC listener or NLM_F_ECHO), which made it intermittent. Drop
the unbuildable notification instead: on -EMSGSIZE the delete proceeds
and the extack notes the dropped event. The same serialisation failure
on the action-add side - tcf_action_add() returns -EINVAL although
tcf_action_init() has already inserted the action into the idr, so the
action is created and reported as a failure - is not changed here:
returning success after a notify build failure there needs an
idr/refcount audit (tca_put_bound_many(), ACT_P_CREATED) that overlaps
the tcf_action_add_failed_notify_leaves_action follow-up. Preserve the
-EMSGSIZE tcf_action_dump() produces (goto errout, like
rtnl_fill_prop_list()) instead of overwriting it with -EINVAL.

why net-next?
This is hardening rather than a regression fix: the enclosing action and
filter-path nests have wrapped since the actions and classifiers allowed
large entry lists - TCA_ACT_TAB is core act_api and the filter-side
container predates the act_gate sizing change. A plain nla_nest_end()
has always written the wrapped length at U16_MAX; this stops the
corruption at the abstraction boundary instead of imposing a new policy
limit on accepted gate schedules. A separate patch caps the entry list
itself.

Conditions to recreate the bug:
CONFIG_NET_CLS_ACT=y; CONFIG_NET_ACT_GATE=y;
CONFIG_NET_CLS_MATCHALL=y and/or CONFIG_NET_CLS_BASIC=y. Create a
matchall or basic filter on an ingress/clsact qdisc whose action list
holds two gate actions of 1024 minimal TCA_GATE_ONE_ENTRY elements
each (TCA_ACT_MAX_PRIO admits up to 32 actions); the aggregate
TCA_ACT_TAB nest is ~74.3 KiB > 65532 and wraps, then RTM_GETTFILTER
it (or trigger the notify with NLM_F_ECHO).
For a single action the outermost TCA_ACT_TAB wraps from 1815 entries
(1821 for the innermost TCA_GATE_ENTRY_LIST); on the filter path the
outermost wrap-capable nest is the classifier's TCA_OPTIONS. Installing
needs CAP_NET_ADMIN in a user namespace; the RTM_GETTFILTER trigger needs
no capability once it exists. The two-action shape survives the
series' per-action 1024-entry cap (2/2): the enclosing nests overflow
from aggregate action size, which no per-action cap can bound.

Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824153903.4143642-1-victor@mojatatu.com
Link: https://lore.kernel.org/netdev/QDISC-H19Z.v1.20261001053234@mojatatu.com/
Link: https://lore.kernel.org/netdev/179111893181.434549.13449660670267048994@kernel.org/
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/act_api.c      | 10 +++++++---
 net/sched/act_gate.c     |  3 ++-
 net/sched/cls_api.c      | 40 ++++++++++++++++++++++++++++++----------
 net/sched/cls_basic.c    |  3 ++-
 net/sched/cls_bpf.c      |  3 ++-
 net/sched/cls_cgroup.c   |  3 ++-
 net/sched/cls_flow.c     |  3 ++-
 net/sched/cls_flower.c   |  6 ++++--
 net/sched/cls_fw.c       |  3 ++-
 net/sched/cls_matchall.c |  3 ++-
 net/sched/cls_route.c    |  3 ++-
 net/sched/cls_u32.c      |  3 ++-
 12 files changed, 59 insertions(+), 24 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 6e48b4bc2d75..747d91ae6446 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -558,7 +558,8 @@ tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
 		goto nla_put_failure;
 	err = tcf_action_dump_old(skb, a, bind, ref);
 	if (err > 0) {
-		nla_nest_end(skb, nest);
+		if (nla_nest_end_safe(skb, nest) < 0)
+			goto nla_put_failure;
 		return err;
 	}
 
@@ -1279,7 +1280,9 @@ int tcf_action_dump(struct sk_buff *skb, struct tc_action *actions[],
 			tcf_action_dump_1(skb, a, bind, ref);
 		if (err < 0)
 			goto errout;
-		nla_nest_end(skb, nest);
+		err = nla_nest_end_safe(skb, nest);
+		if (err < 0)
+			goto errout;
 	}
 
 	return 0;
@@ -1693,7 +1696,8 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
 	if (tcf_action_dump(skb, actions, bind, ref, false) < 0)
 		goto out_nlmsg_trim;
 
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto out_nlmsg_trim;
 
 	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
 
diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index 6d6d45e03c07..14801c604bd9 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -654,7 +654,8 @@ static int tcf_gate_dump(struct sk_buff *skb, struct tc_action *a,
 			goto nla_put_failure;
 	}
 
-	nla_nest_end(skb, entry_list);
+	if (nla_nest_end_safe(skb, entry_list) < 0)
+		goto nla_put_failure;
 
 	tcf_tm_dump(&t, &gact->tcf_tm);
 	if (nla_put_64bit(skb, TCA_GATE_TM, sizeof(t), &t, TCA_GATE_PAD))
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index a9f54988561f..3f0ce567fa2d 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2149,11 +2149,20 @@ static struct sk_buff *tfilter_notify_prep(struct net *net,
 			    rtnl_held, extack);
 	if (ret <= 0) {
 		kfree_skb(skb);
-		if (ret == -EMSGSIZE) {
-			size += NLMSG_GOODSIZE;
-			goto retry;
-		}
-		return ERR_PTR(-EINVAL);
+		if (ret != -EMSGSIZE)
+			return ERR_PTR(-EINVAL);
+		/* A filter dump is carried inside a nest whose u16 nla_len
+		 * caps it, so a dump that still does not serialize once the
+		 * skb is larger than any valid message can never be built,
+		 * however big the skb gets. Filling reports that structural
+		 * overflow and genuine capacity exhaustion the same way, so
+		 * stop at the bound instead of looping until alloc_skb()
+		 * fails on an order too large for the page allocator.
+		 */
+		if (size > U16_MAX + NLMSG_GOODSIZE)
+			return ERR_PTR(-EMSGSIZE);
+		size += NLMSG_GOODSIZE;
+		goto retry;
 	}
 	return skb;
 }
@@ -2200,8 +2209,16 @@ static int tfilter_del_notify(struct net *net, struct sk_buff *oskb,
 	skb = tfilter_notify_prep(net, oskb, n, tp, block, q, parent, fh,
 				  RTM_DELTFILTER, portid, rtnl_held, extack);
 	if (IS_ERR(skb)) {
-		NL_SET_ERR_MSG(extack, "Failed to build del event notification");
-		return PTR_ERR(skb);
+		if (PTR_ERR(skb) != -EMSGSIZE) {
+			NL_SET_ERR_MSG(extack, "Failed to build del event notification");
+			return PTR_ERR(skb);
+		}
+		/* The filter's dump cannot be represented in a u16 nest, so no
+		 * notification can ever be built for it. Drop the notification
+		 * rather than refusing to delete the filter.
+		 */
+		NL_SET_ERR_MSG(extack, "Filter deleted; del event notification could not be built");
+		return tp->ops->delete(tp, fh, last, rtnl_held, extack);
 	}
 
 	err = tp->ops->delete(tp, fh, last, rtnl_held, extack);
@@ -3529,7 +3546,8 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts)
 			if (tcf_action_dump(skb, exts->actions, 0, 0, false)
 			    < 0)
 				goto nla_put_failure;
-			nla_nest_end(skb, nest);
+			if (nla_nest_end_safe(skb, nest) < 0)
+				goto nla_put_failure;
 		} else if (exts->police) {
 			struct tc_action *act = tcf_exts_first_act(exts);
 			nest = nla_nest_start_noflag(skb, exts->police);
@@ -3537,7 +3555,8 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts)
 				goto nla_put_failure;
 			if (tcf_action_dump_old(skb, act, 0, 0) < 0)
 				goto nla_put_failure;
-			nla_nest_end(skb, nest);
+			if (nla_nest_end_safe(skb, nest) < 0)
+				goto nla_put_failure;
 		}
 	}
 	return 0;
@@ -3565,7 +3584,8 @@ int tcf_exts_terse_dump(struct sk_buff *skb, struct tcf_exts *exts)
 
 	if (tcf_action_dump(skb, exts->actions, 0, 0, true) < 0)
 		goto nla_put_failure;
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto nla_put_failure;
 	return 0;
 
 nla_put_failure:
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index e2a94ba9fba7..ba859110faae 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -305,7 +305,8 @@ static int basic_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	    tcf_em_tree_dump(skb, &f->ematches, TCA_BASIC_EMATCHES) < 0)
 		goto nla_put_failure;
 
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto nla_put_failure;
 
 	if (tcf_exts_dump_stats(skb, &f->exts) < 0)
 		goto nla_put_failure;
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 188cf0f949dd..232796d3a51f 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -631,7 +631,8 @@ static int cls_bpf_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	    nla_put_u32(skb, TCA_BPF_FLAGS_GEN, prog->gen_flags))
 		goto nla_put_failure;
 
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto nla_put_failure;
 
 	if (tcf_exts_dump_stats(skb, &prog->exts) < 0)
 		goto nla_put_failure;
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 210fd9fd26d8..28701d3916b8 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -185,7 +185,8 @@ static int cls_cgroup_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	    tcf_em_tree_dump(skb, &head->ematches, TCA_CGROUP_EMATCHES) < 0)
 		goto nla_put_failure;
 
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto nla_put_failure;
 
 	if (tcf_exts_dump_stats(skb, &head->exts) < 0)
 		goto nla_put_failure;
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index a9ac3acf6eda..2ef449706638 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -681,7 +681,8 @@ static int flow_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	    tcf_em_tree_dump(skb, &f->ematches, TCA_FLOW_EMATCHES) < 0)
 		goto nla_put_failure;
 #endif
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto nla_put_failure;
 
 	if (tcf_exts_dump_stats(skb, &f->exts) < 0)
 		goto nla_put_failure;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 0c4beff18d68..299b1493d74b 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -3759,7 +3759,8 @@ static int fl_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	if (tcf_exts_dump(skb, &f->exts))
 		goto nla_put_failure;
 
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto nla_put_failure;
 
 	if (tcf_exts_dump_stats(skb, &f->exts) < 0)
 		goto nla_put_failure;
@@ -3804,7 +3805,8 @@ static int fl_terse_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	if (tcf_exts_terse_dump(skb, &f->exts))
 		goto nla_put_failure;
 
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto nla_put_failure;
 
 	return skb->len;
 
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index a462b262719c..0f731595f839 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -414,7 +414,8 @@ static int fw_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	if (tcf_exts_dump(skb, &f->exts) < 0)
 		goto nla_put_failure;
 
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto nla_put_failure;
 
 	if (tcf_exts_dump_stats(skb, &f->exts) < 0)
 		goto nla_put_failure;
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index c14899b935bf..6ece63b82775 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -366,7 +366,8 @@ static int mall_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	if (tcf_exts_dump(skb, &head->exts))
 		goto nla_put_failure;
 
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto nla_put_failure;
 
 	if (tcf_exts_dump_stats(skb, &head->exts) < 0)
 		goto nla_put_failure;
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 0f211f030fd9..d5b009a7c89b 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -657,7 +657,8 @@ static int route4_dump(struct net *net, struct tcf_proto *tp, void *fh,
 	if (tcf_exts_dump(skb, &f->exts) < 0)
 		goto nla_put_failure;
 
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto nla_put_failure;
 
 	if (tcf_exts_dump_stats(skb, &f->exts) < 0)
 		goto nla_put_failure;
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 76ce2d124079..91f4e6458785 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -1482,7 +1482,8 @@ static int u32_dump(struct net *net, struct tcf_proto *tp, void *fh,
 #endif
 	}
 
-	nla_nest_end(skb, nest);
+	if (nla_nest_end_safe(skb, nest) < 0)
+		goto nla_put_failure;
 
 	if (TC_U32_KEY(n->handle))
 		if (tcf_exts_dump_stats(skb, &n->exts) < 0)
-- 
2.43.0

Re: [PATCH net-next 1/2] net/sched: act_gate: reject oversized dumps instead of wrapping them

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 1 (1) · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] This patch makes an unbuildable notification block
  RTM_DELACTION.
- [Medium] RTM_NEWACTION for an action set whose dump exceeds U16_MAX now
  returns -EINVAL even though the actions have already been created or…
- [Low] On the -EMSGSIZE branch, tfilter_del_notify()
  (net/sched/cls_api.c) sets the extack message "Filter deleted; del…
- [Low] The commit message misdescribes what the code did before the
  patch.
- [Low] In tfilter_del_notify(), -EMSGSIZE is taken to mean that the dump
  cannot be represented in a u16 nest ('no notification can ever be…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but
  tfilter_del_notify() still returns without calling ->delete() when…

--

net/sched: act_gate: reject oversized dumps instead of wrapping them

This commit switches the nest closes on the tc action and classifier dump
paths to nla_nest_end_safe(). A dump larger than U16_MAX now reports
-EMSGSIZE instead of writing a wrapped nla_len. It also bounds the
tfilter_notify_prep() retry loop, and makes tfilter_del_notify() drop a
notification it cannot build and delete the filter anyway.

Link: https://lore.kernel.org/netdev/QDISC-H19Z.v1.20261001053234@mojatatu.com/
Link: https://lore.kernel.org/netdev/179111893181.434549.13449660670267048994@kernel.org/

> tfilter_notify_prep() treats that as socket-buffer exhaustion and
> retries with an ever larger alloc_skb(); a dump that does not fit a u16
> nest cannot be built at any skb size, so the loop only spins until the
> allocation itself fails.

[Severity: Low]
Does this describe the code as it was before this patch?

Before, nla_nest_end() could not fail:

include/net/netlink.h:nla_nest_end() {
	unsigned long diff = skb_tail_pointer(skb) - (unsigned char *)start;

	DEBUG_NET_WARN_ON_ONCE(diff > U16_MAX);
	start->nla_len = diff;
	return skb->len;
}

Nothing under net/sched used nla_nest_end_safe(). So tcf_fill_node()
returned -EMSGSIZE only while the skb was short on tailroom. Once
tfilter_notify_prep() had grown the skb enough, the fill succeeded with a
wrapped length and the loop stopped.

The loop that runs until alloc_skb() fails seems to exist only after this
patch turns the u16 overflow into a fill failure. That makes the new bound
in tfilter_notify_prep() a requirement of the nla_nest_end_safe()
conversions, not a fix for an existing loop.

Could the commit message say this? A backport that takes the conversions
without the bound would bring in the allocation escalation loop.

> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 6e48b4bc2d75..747d91ae6446 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c

[ ... ]

> @@ -1693,7 +1696,8 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
>  	if (tcf_action_dump(skb, actions, bind, ref, false) < 0)
>  		goto out_nlmsg_trim;
>  
> -	nla_nest_end(skb, nest);
> +	if (nla_nest_end_safe(skb, nest) < 0)
> +		goto out_nlmsg_trim;

[Severity: High]
With this change, tca_get_fill() fails for an action set whose TCA_ACT_TAB
is larger than U16_MAX. Before, the same fill succeeded with a wrapped
length. Does this make a failed notification build refuse an
RTM_DELACTION?

tcf_del_notify() builds the notification before it deletes anything:

net/sched/act_api.c:tcf_del_notify() {
    ...
	} else {
		skb = tcf_del_notify_msg(net, n, actions, portid, attr_size,
					 extack);
		if (IS_ERR(skb))
			return PTR_ERR(skb);
	}

	/* now do the delete */
	ret = tcf_action_delete(net, actions);
    ...
}

tcf_del_notify_msg() turns any tca_get_fill() failure into -EINVAL:

	if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
			 0, 2, extack) <= 0) {
		NL_SET_ERR_MSG(extack, "Failed to fill netlink TC action attributes");
		kfree_skb(skb);
		return ERR_PTR(-EINVAL);
	}

The full path is:

tc_ctl_action(RTM_DELACTION)
  tca_action_gd()
    tcf_del_notify()
      tcf_del_notify_msg()
        tca_get_fill()
          nla_nest_end_safe() < 0

tcf_action_delete() is never reached.

With the 1024-entry cap applied, "tc actions del action gate index 1
action gate index 2" on two 1024-entry gate actions looks like it would
fail with -EINVAL and leave both actions installed. The commit message
says this two-action shape survives the cap.

The cap patch later in this series only adds GATE_ENTRIES_MAX in
parse_gate_list(). It does not touch tcf_del_notify(). Without the cap
patch, a single gate action with about 1815 or more entries could not be
deleted by index.

This needs rtnl_notify_needed() to be true, meaning an RTNLGRP_TC
listener or NLM_F_ECHO. rtnetlink allows non-root receive, so an
unprivileged listener in the netns seems to be enough to make an admin's
delete fail.

The commit message says "A notification that cannot be serialised must
not veto the state change the caller asked for", and tfilter_del_notify()
is changed to match. tcf_reoffload_del_notify() in this file already
drops a notification it cannot build:

		if (IS_ERR(skb))
			skb = NULL;

Should tcf_del_notify() be changed the same way? tca_get_fill() returns a
bare -1, so -EMSGSIZE would also need to be passed up. Otherwise the
delete path cannot tell the structural overflow from other failures,
which tfilter_del_notify() now relies on.

[Severity: Medium]
The same failure also shows up on the add side.
tcf_action_add()->tcf_action_init() has already inserted or replaced the
actions by the time tcf_add_notify()->tcf_add_notify_msg() calls
tca_get_fill():

	if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags,
			 RTM_NEWACTION, 0, 0, extack) <= 0) {
		NL_SET_ERR_MSG(extack, "Failed to fill netlink attributes while adding TC action");
		kfree_skb(skb);
		return ERR_PTR(-EINVAL);
	}

The commit message notes this case and leaves it for later. However,
before this patch an oversized set still filled (with wrapped lengths)
and the add reported success. Isn't the "-EINVAL after the actions were
created" result new for these sets?

For example, with the cap applied, adding two 1024-entry gate actions in
one request now returns -EINVAL with the actions installed. This happens
when an RTNLGRP_TC listener exists or NLM_F_ECHO is set. A retry would
then hit -EEXIST.

>  
>  	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
>  
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index a9f54988561f..3f0ce567fa2d 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c

[ ... ]

> @@ -2200,8 +2209,16 @@ static int tfilter_del_notify(struct net *net, struct sk_buff *oskb,
>  	skb = tfilter_notify_prep(net, oskb, n, tp, block, q, parent, fh,
>  				  RTM_DELTFILTER, portid, rtnl_held, extack);
>  	if (IS_ERR(skb)) {
> -		NL_SET_ERR_MSG(extack, "Failed to build del event notification");
> -		return PTR_ERR(skb);
> +		if (PTR_ERR(skb) != -EMSGSIZE) {
> +			NL_SET_ERR_MSG(extack, "Failed to build del event notification");
> +			return PTR_ERR(skb);
> +		}

[Severity: Medium]
This is a pre-existing issue, not one introduced by this patch. This
branch still returns without calling ->delete() when
tfilter_notify_prep() fails with something other than -EMSGSIZE, such as
-ENOBUFS from alloc_skb() or -EINVAL. Does that leak the flower filter
reference taken during the lookup?

In tc_del_tfilter(), fh comes from tp->ops->get(). For flower,
fl_get()->__fl_get() takes a reference:

	if (f && !refcount_inc_not_zero(&f->refcnt))
		f = NULL;

fl_delete() normally drops it with __fl_put(f). On this error path,
tc_del_tfilter() does:

		err = tfilter_del_notify(net, skb, n, tp, block, q, parent, fh,
					 &last, rtnl_held, extack);

		if (err)
			goto errout;

errout releases tp, chain and block, but never calls tfilter_put(tp, fh).
After that, even a later successful delete would not free the filter or
the actions bound to it.

The alloc_skb() failure seems realistic here. The retry loop grows the
skb toward U16_MAX + NLMSG_GOODSIZE, which needs order 4-5 allocations.

> +		/* The filter's dump cannot be represented in a u16 nest, so no
> +		 * notification can ever be built for it. Drop the notification
> +		 * rather than refusing to delete the filter.
> +		 */

[Severity: Low]
Does -EMSGSIZE always mean the dump cannot fit in a u16 nest?
tcf_fill_node() starts with:

	int ret = -EMSGSIZE;

and returns that for every ->dump() failure. Some dumps fail for reasons
that have nothing to do with size. One example is tcf_pedit_dump():

	opt = kzalloc(s, GFP_ATOMIC);
	if (unlikely(!opt)) {
		rcu_read_unlock();
		return -ENOBUFS;

u32_dump() also allocates with kzalloc_flex(), and tcf_action_dump_1()
reduces any error to -1.

If such a failure repeats through every retry in tfilter_notify_prep()
(about 17 with 4K pages), the loop returns ERR_PTR(-EMSGSIZE). This
branch then drops an RTM_DELTFILTER event that could have been built. The
comment in tfilter_notify_prep() only mentions structural overflow and
genuine capacity exhaustion.

> +		NL_SET_ERR_MSG(extack, "Filter deleted; del event notification could not be built");
> +		return tp->ops->delete(tp, fh, last, rtnl_held, extack);

[Severity: Low]
The extack text is set before ->delete() runs. If ->delete() then fails
without writing its own extack message, won't userspace get "Filter
deleted" together with a negative errno?

Flower, for example, runs without rtnl on an unlocked qdisc. Two
concurrent RTM_DELTFILTER requests for the same oversized filter can both
get through fl_get() and both hit -EMSGSIZE in prep. The second one then
reaches __fl_delete():

	if (f->deleted) {
		spin_unlock(&tp->lock);
		return -ENOENT;
	}

That path does not set extack. fw_delete() can return -EINVAL the same
way.

Could the message be set only after ->delete() returns 0?

>  	}
>  
>  	err = tp->ops->delete(tp, fh, last, rtnl_held, extack);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-H19Z.v2.20261007062551%40mojatatu.com

Re: [PATCH net-next 1/2] net/sched: act_gate: reject oversized dumps instead of wrapping them

[Jamal Hadi Salim]

On Sun, Oct 11, 2026 at 6:57 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 5 potential
> issue(s) to consider.
>
> Critical: 0 · High: 1 · Medium: 1 (1) · Low: 3
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [High] This patch makes an unbuildable notification block
>   RTM_DELACTION.

This is a really good catch. I doubt anyone on planet earth will
delete this way but it is a legit bug - so will send a v3.
Since I am sending a v3, I will fix all the complaints on commit
messages for this patch and patch 2/2.
The rest are followups we captured.

cheers,
jamal

> - [Medium] RTM_NEWACTION for an action set whose dump exceeds U16_MAX now
>   returns -EINVAL even though the actions have already been created or…
> - [Low] On the -EMSGSIZE branch, tfilter_del_notify()
>   (net/sched/cls_api.c) sets the extack message "Filter deleted; del…
> - [Low] The commit message misdescribes what the code did before the
>   patch.
> - [Low] In tfilter_del_notify(), -EMSGSIZE is taken to mean that the dump
>   cannot be represented in a u16 nest ('no notification can ever be…
>
> Pre-existing issues:
> - [Medium] This problem wasn't introduced by this patch, but
>   tfilter_del_notify() still returns without calling ->delete() when…
>
> --
>
> net/sched: act_gate: reject oversized dumps instead of wrapping them
>
> This commit switches the nest closes on the tc action and classifier dump
> paths to nla_nest_end_safe(). A dump larger than U16_MAX now reports
> -EMSGSIZE instead of writing a wrapped nla_len. It also bounds the
> tfilter_notify_prep() retry loop, and makes tfilter_del_notify() drop a
> notification it cannot build and delete the filter anyway.
>
> Link: https://lore.kernel.org/netdev/QDISC-H19Z.v1.20261001053234@mojatatu.com/
> Link: https://lore.kernel.org/netdev/179111893181.434549.13449660670267048994@kernel.org/
>
> > tfilter_notify_prep() treats that as socket-buffer exhaustion and
> > retries with an ever larger alloc_skb(); a dump that does not fit a u16
> > nest cannot be built at any skb size, so the loop only spins until the
> > allocation itself fails.
>
> [Severity: Low]
> Does this describe the code as it was before this patch?
>
> Before, nla_nest_end() could not fail:
>
> include/net/netlink.h:nla_nest_end() {
>         unsigned long diff = skb_tail_pointer(skb) - (unsigned char *)start;
>
>         DEBUG_NET_WARN_ON_ONCE(diff > U16_MAX);
>         start->nla_len = diff;
>         return skb->len;
> }
>
> Nothing under net/sched used nla_nest_end_safe(). So tcf_fill_node()
> returned -EMSGSIZE only while the skb was short on tailroom. Once
> tfilter_notify_prep() had grown the skb enough, the fill succeeded with a
> wrapped length and the loop stopped.
>
> The loop that runs until alloc_skb() fails seems to exist only after this
> patch turns the u16 overflow into a fill failure. That makes the new bound
> in tfilter_notify_prep() a requirement of the nla_nest_end_safe()
> conversions, not a fix for an existing loop.
>
> Could the commit message say this? A backport that takes the conversions
> without the bound would bring in the allocation escalation loop.
>
> > diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> > index 6e48b4bc2d75..747d91ae6446 100644
> > --- a/net/sched/act_api.c
> > +++ b/net/sched/act_api.c
>
> [ ... ]
>
> > @@ -1693,7 +1696,8 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
> >       if (tcf_action_dump(skb, actions, bind, ref, false) < 0)
> >               goto out_nlmsg_trim;
> >
> > -     nla_nest_end(skb, nest);
> > +     if (nla_nest_end_safe(skb, nest) < 0)
> > +             goto out_nlmsg_trim;
>
> [Severity: High]
> With this change, tca_get_fill() fails for an action set whose TCA_ACT_TAB
> is larger than U16_MAX. Before, the same fill succeeded with a wrapped
> length. Does this make a failed notification build refuse an
> RTM_DELACTION?
>
> tcf_del_notify() builds the notification before it deletes anything:
>
> net/sched/act_api.c:tcf_del_notify() {
>     ...
>         } else {
>                 skb = tcf_del_notify_msg(net, n, actions, portid, attr_size,
>                                          extack);
>                 if (IS_ERR(skb))
>                         return PTR_ERR(skb);
>         }
>
>         /* now do the delete */
>         ret = tcf_action_delete(net, actions);
>     ...
> }
>
> tcf_del_notify_msg() turns any tca_get_fill() failure into -EINVAL:
>
>         if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
>                          0, 2, extack) <= 0) {
>                 NL_SET_ERR_MSG(extack, "Failed to fill netlink TC action attributes");
>                 kfree_skb(skb);
>                 return ERR_PTR(-EINVAL);
>         }
>
> The full path is:
>
> tc_ctl_action(RTM_DELACTION)
>   tca_action_gd()
>     tcf_del_notify()
>       tcf_del_notify_msg()
>         tca_get_fill()
>           nla_nest_end_safe() < 0
>
> tcf_action_delete() is never reached.
>
> With the 1024-entry cap applied, "tc actions del action gate index 1
> action gate index 2" on two 1024-entry gate actions looks like it would
> fail with -EINVAL and leave both actions installed. The commit message
> says this two-action shape survives the cap.
>
> The cap patch later in this series only adds GATE_ENTRIES_MAX in
> parse_gate_list(). It does not touch tcf_del_notify(). Without the cap
> patch, a single gate action with about 1815 or more entries could not be
> deleted by index.
>
> This needs rtnl_notify_needed() to be true, meaning an RTNLGRP_TC
> listener or NLM_F_ECHO. rtnetlink allows non-root receive, so an
> unprivileged listener in the netns seems to be enough to make an admin's
> delete fail.
>
> The commit message says "A notification that cannot be serialised must
> not veto the state change the caller asked for", and tfilter_del_notify()
> is changed to match. tcf_reoffload_del_notify() in this file already
> drops a notification it cannot build:
>
>                 if (IS_ERR(skb))
>                         skb = NULL;
>
> Should tcf_del_notify() be changed the same way? tca_get_fill() returns a
> bare -1, so -EMSGSIZE would also need to be passed up. Otherwise the
> delete path cannot tell the structural overflow from other failures,
> which tfilter_del_notify() now relies on.
>
> [Severity: Medium]
> The same failure also shows up on the add side.
> tcf_action_add()->tcf_action_init() has already inserted or replaced the
> actions by the time tcf_add_notify()->tcf_add_notify_msg() calls
> tca_get_fill():
>
>         if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags,
>                          RTM_NEWACTION, 0, 0, extack) <= 0) {
>                 NL_SET_ERR_MSG(extack, "Failed to fill netlink attributes while adding TC action");
>                 kfree_skb(skb);
>                 return ERR_PTR(-EINVAL);
>         }
>
> The commit message notes this case and leaves it for later. However,
> before this patch an oversized set still filled (with wrapped lengths)
> and the add reported success. Isn't the "-EINVAL after the actions were
> created" result new for these sets?
>
> For example, with the cap applied, adding two 1024-entry gate actions in
> one request now returns -EINVAL with the actions installed. This happens
> when an RTNLGRP_TC listener exists or NLM_F_ECHO is set. A retry would
> then hit -EEXIST.
>
> >
> >       nlh->nlmsg_len = skb_tail_pointer(skb) - b;
> >
> > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> > index a9f54988561f..3f0ce567fa2d 100644
> > --- a/net/sched/cls_api.c
> > +++ b/net/sched/cls_api.c
>
> [ ... ]
>
> > @@ -2200,8 +2209,16 @@ static int tfilter_del_notify(struct net *net, struct sk_buff *oskb,
> >       skb = tfilter_notify_prep(net, oskb, n, tp, block, q, parent, fh,
> >                                 RTM_DELTFILTER, portid, rtnl_held, extack);
> >       if (IS_ERR(skb)) {
> > -             NL_SET_ERR_MSG(extack, "Failed to build del event notification");
> > -             return PTR_ERR(skb);
> > +             if (PTR_ERR(skb) != -EMSGSIZE) {
> > +                     NL_SET_ERR_MSG(extack, "Failed to build del event notification");
> > +                     return PTR_ERR(skb);
> > +             }
>
> [Severity: Medium]
> This is a pre-existing issue, not one introduced by this patch. This
> branch still returns without calling ->delete() when
> tfilter_notify_prep() fails with something other than -EMSGSIZE, such as
> -ENOBUFS from alloc_skb() or -EINVAL. Does that leak the flower filter
> reference taken during the lookup?
>
> In tc_del_tfilter(), fh comes from tp->ops->get(). For flower,
> fl_get()->__fl_get() takes a reference:
>
>         if (f && !refcount_inc_not_zero(&f->refcnt))
>                 f = NULL;
>
> fl_delete() normally drops it with __fl_put(f). On this error path,
> tc_del_tfilter() does:
>
>                 err = tfilter_del_notify(net, skb, n, tp, block, q, parent, fh,
>                                          &last, rtnl_held, extack);
>
>                 if (err)
>                         goto errout;
>
> errout releases tp, chain and block, but never calls tfilter_put(tp, fh).
> After that, even a later successful delete would not free the filter or
> the actions bound to it.
>
> The alloc_skb() failure seems realistic here. The retry loop grows the
> skb toward U16_MAX + NLMSG_GOODSIZE, which needs order 4-5 allocations.
>
> > +             /* The filter's dump cannot be represented in a u16 nest, so no
> > +              * notification can ever be built for it. Drop the notification
> > +              * rather than refusing to delete the filter.
> > +              */
>
> [Severity: Low]
> Does -EMSGSIZE always mean the dump cannot fit in a u16 nest?
> tcf_fill_node() starts with:
>
>         int ret = -EMSGSIZE;
>
> and returns that for every ->dump() failure. Some dumps fail for reasons
> that have nothing to do with size. One example is tcf_pedit_dump():
>
>         opt = kzalloc(s, GFP_ATOMIC);
>         if (unlikely(!opt)) {
>                 rcu_read_unlock();
>                 return -ENOBUFS;
>
> u32_dump() also allocates with kzalloc_flex(), and tcf_action_dump_1()
> reduces any error to -1.
>
> If such a failure repeats through every retry in tfilter_notify_prep()
> (about 17 with 4K pages), the loop returns ERR_PTR(-EMSGSIZE). This
> branch then drops an RTM_DELTFILTER event that could have been built. The
> comment in tfilter_notify_prep() only mentions structural overflow and
> genuine capacity exhaustion.
>
> > +             NL_SET_ERR_MSG(extack, "Filter deleted; del event notification could not be built");
> > +             return tp->ops->delete(tp, fh, last, rtnl_held, extack);
>
> [Severity: Low]
> The extack text is set before ->delete() runs. If ->delete() then fails
> without writing its own extack message, won't userspace get "Filter
> deleted" together with a negative errno?
>
> Flower, for example, runs without rtnl on an unlocked qdisc. Two
> concurrent RTM_DELTFILTER requests for the same oversized filter can both
> get through fl_get() and both hit -EMSGSIZE in prep. The second one then
> reaches __fl_delete():
>
>         if (f->deleted) {
>                 spin_unlock(&tp->lock);
>                 return -ENOENT;
>         }
>
> That path does not set extack. fw_delete() can return -EINVAL the same
> way.
>
> Could the message be set only after ->delete() returns 0?
>
> >       }
> >
> >       err = tp->ops->delete(tp, fh, last, rtnl_held, extack);
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-H19Z.v2.20261007062551%40mojatatu.com

[PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries

[Jamal Hadi Salim]

parse_gate_list() accepts _any number_ of TCA_GATE_ONE_ENTRY elements.
The only bound/cap is the enclosing nlattr u16 nla_len (~5460
minimal 12-byte entries). Each entry is a separate GFP_ATOMIC
allocation, and the dump of a large list grows one action's reply well
past NLMSG_GOODSIZE.

The bugs:
1. A single gate can make tcf_dump_walker() silently truncate
"tc actions ls"
2. Lack of capping has been demonstrated to be exploitable to
create an OOM through amplification of RTM_GETACTION replies.

So we need to provide an upper bound cap of TCA_GATE_ONE_ENTRY elements.

Some context:
The gate action offloads through FLOW_ACTION_GATE. The offload
interface passes the priority, base time, cycle time, cycle-time
extension, the complete entry count and an array holding each entry's
gate state, interval, IPV and maximum-octet value. The in-tree SJA1105
DSA driver forwards that complete list to sja1105_vl_gate() without any
entry cap check, and composes every supplied entry into the hardware
schedule table.

Act gate challenges:
SJA1105 and SJA1110 are two device families in the one sja1105 driver.
They expose different schedule-table capacities: SJA1105 provides up to
1024 schedule entries and SJA1110 up to 4096. These are total table
capacities shared with other schedules, so they are upper bounds, not a
guaranteed per-action budget. Software gate actions with _a lot more_
entries install and serialize today, so a low cap would reject working
configurations and would look like a UAPI breakage; but an infinite size
of entries is a bogus choice; 65 entries is the boundary configuration
verified during this work.

Netlink Challenges:
Netlink attributes record their total size, including the four-byte
header, in a 16-bit nla_len, so a nested attribute can describe at most
65532 bytes. A minimal accepted input entry is 12 bytes, which puts the
max we can fit at roughly 5460 entries; a dumped entry occupies 36 bytes,
or 40 bytes when it carries the gate-open flag.
1024 entries therefore serialize to 36868-40964 bytes, which fits
inside one nested attribute, while a 4096-entry schedule would need
roughly 144-160 KiB which _cannot fit_ at the moment due to the 16bit
length.

Note, Note: This represents challenges primarily with netlink.

The Cap:
Cap the list at 1024 entries. This value is based on the existing
SJA1105 schedule-table capacity and preserves the verified 65-entry
configuration while providing a substantial headroom, and keeps the
complete gate list inside the current 16-bit TLV format. It
deliberately declines to expose SJA1110's 1025-4096 range through
an interface that cannot serialize it, and deliberately rejects
software-only schedules above 1024.

This is an operational cap on a configuration the uAPI accepted before,
so it is net-next hardening; if we made this a stable backport then
it would start rejecting gate configurations that install today (even
though those settings would be totally bogus).

Caveat Emptor:
While this patch fixes the binding of per-action storage and the dump
amplification; it does not by itself fix tcf_dump_walker() truncation
whose skb is smaller than the reply for such an action (for example the
kernel clamps a dump skb via netlink_recvmsg()/netlink_dump() to
SKB_WITH_OVERHEAD(32768), so "tc actions ls" is already truncated below
the 1024 this cap admits, and you cannot dump the top of the range
without changing iproute2 code)
It also does not remove generic netlink receive-queue amplification.
A companion iproute2 change can enumerate actions with a terse dump and
then fetch each one with an indexed RTM_GETACTION.

Conditions to recreate the bug:
Cap net admin with CONFIG_NET_ACT_GATE=y. Install a gate
action with 1025 or more minimal TCA_GATE_ONE_ENTRY elements over a raw
netlink RTM_NEWACTION request. The cap rejects the list with -E2BIG and
an extack naming the limit; 1024 entries install and an indexed
RTM_GETACTION returns a well-formed reply.

Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/act_gate.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index 14801c604bd9..d8e8c2356bd5 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -18,6 +18,17 @@
 
 static struct tc_action_ops act_gate_ops;
 
+/* A netlink attribute records its total length, including the 4-byte
+ * header, in a u16 nla_len, so one nested attribute can describe at most
+ * 65532 bytes. tcf_gate_dump() emits each schedule entry as 36 bytes, or
+ * 40 with the gate-open flag, so 1024 entries serialize to 36868-40964
+ * bytes and still fit a single TCA_GATE_ENTRY_LIST nest, while 4096 would
+ * need ~144-160 KiB and cannot be represented. 1024 is also the schedule
+ * table capacity of SJA1105 (SJA1110 provides 4096). Cap the entry list
+ * at 1024.
+ */
+#define GATE_ENTRIES_MAX	1024
+
 static ktime_t gate_get_time(struct tcf_gate *gact)
 {
 	ktime_t mono = ktime_get();
@@ -277,6 +288,14 @@ static int parse_gate_list(struct nlattr *list_attr,
 			continue;
 		}
 
+		if (i >= GATE_ENTRIES_MAX) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "Too many schedule entries, at most %u are supported",
+					   GATE_ENTRIES_MAX);
+			err = -E2BIG;
+			goto release_list;
+		}
+
 		entry = kzalloc_obj(*entry, GFP_ATOMIC);
 		if (!entry) {
 			NL_SET_ERR_MSG(extack, "Not enough memory for entry");
-- 
2.43.0

Re: [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries

[Paul Moses]

On Wednesday, October 7th, 2026 at 5:29 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:

> parse_gate_list() accepts _any number_ of TCA_GATE_ONE_ENTRY elements.
> The only bound/cap is the enclosing nlattr u16 nla_len (~5460
> minimal 12-byte entries). Each entry is a separate GFP_ATOMIC
> allocation, and the dump of a large list grows one action's reply well
> past NLMSG_GOODSIZE.
> 
> The bugs:
> 1. A single gate can make tcf_dump_walker() silently truncate
> "tc actions ls"
> 2. Lack of capping has been demonstrated to be exploitable to
> create an OOM through amplification of RTM_GETACTION replies.
> 
> So we need to provide an upper bound cap of TCA_GATE_ONE_ENTRY elements.
> 
> Some context:
> The gate action offloads through FLOW_ACTION_GATE. The offload
> interface passes the priority, base time, cycle time, cycle-time
> extension, the complete entry count and an array holding each entry's
> gate state, interval, IPV and maximum-octet value. The in-tree SJA1105
> DSA driver forwards that complete list to sja1105_vl_gate() without any
> entry cap check, and composes every supplied entry into the hardware
> schedule table.
> 
> Act gate challenges:
> SJA1105 and SJA1110 are two device families in the one sja1105 driver.
> They expose different schedule-table capacities: SJA1105 provides up to
> 1024 schedule entries and SJA1110 up to 4096. These are total table
> capacities shared with other schedules, so they are upper bounds, not a
> guaranteed per-action budget. Software gate actions with _a lot more_
> entries install and serialize today, so a low cap would reject working
> configurations and would look like a UAPI breakage; but an infinite size
> of entries is a bogus choice; 65 entries is the boundary configuration
> verified during this work.
> 
> Netlink Challenges:
> Netlink attributes record their total size, including the four-byte
> header, in a 16-bit nla_len, so a nested attribute can describe at most
> 65532 bytes. A minimal accepted input entry is 12 bytes, which puts the
> max we can fit at roughly 5460 entries; a dumped entry occupies 36 bytes,
> or 40 bytes when it carries the gate-open flag.
> 1024 entries therefore serialize to 36868-40964 bytes, which fits
> inside one nested attribute, while a 4096-entry schedule would need
> roughly 144-160 KiB which _cannot fit_ at the moment due to the 16bit
> length.
> 
> Note, Note: This represents challenges primarily with netlink.
> 
> The Cap:
> Cap the list at 1024 entries. This value is based on the existing
> SJA1105 schedule-table capacity and preserves the verified 65-entry
> configuration while providing a substantial headroom, and keeps the
> complete gate list inside the current 16-bit TLV format. It
> deliberately declines to expose SJA1110's 1025-4096 range through
> an interface that cannot serialize it, and deliberately rejects
> software-only schedules above 1024.
> 
> This is an operational cap on a configuration the uAPI accepted before,
> so it is net-next hardening; if we made this a stable backport then
> it would start rejecting gate configurations that install today (even
> though those settings would be totally bogus).
> 
> Caveat Emptor:
> While this patch fixes the binding of per-action storage and the dump
> amplification; it does not by itself fix tcf_dump_walker() truncation
> whose skb is smaller than the reply for such an action (for example the
> kernel clamps a dump skb via netlink_recvmsg()/netlink_dump() to
> SKB_WITH_OVERHEAD(32768), so "tc actions ls" is already truncated below
> the 1024 this cap admits, and you cannot dump the top of the range
> without changing iproute2 code)
> It also does not remove generic netlink receive-queue amplification.
> A companion iproute2 change can enumerate actions with a terse dump and
> then fetch each one with an indexed RTM_GETACTION.
> 
> Conditions to recreate the bug:
> Cap net admin with CONFIG_NET_ACT_GATE=y. Install a gate
> action with 1025 or more minimal TCA_GATE_ONE_ENTRY elements over a raw
> netlink RTM_NEWACTION request. The cap rejects the list with -E2BIG and
> an extack naming the limit; 1024 entries install and an indexed
> RTM_GETACTION returns a well-formed reply.
> 
> Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> ---
>  net/sched/act_gate.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
> index 14801c604bd9..d8e8c2356bd5 100644
> --- a/net/sched/act_gate.c
> +++ b/net/sched/act_gate.c
> @@ -18,6 +18,17 @@
> 
>  static struct tc_action_ops act_gate_ops;
> 
> +/* A netlink attribute records its total length, including the 4-byte
> + * header, in a u16 nla_len, so one nested attribute can describe at most
> + * 65532 bytes. tcf_gate_dump() emits each schedule entry as 36 bytes, or
> + * 40 with the gate-open flag, so 1024 entries serialize to 36868-40964
> + * bytes and still fit a single TCA_GATE_ENTRY_LIST nest, while 4096 would
> + * need ~144-160 KiB and cannot be represented. 1024 is also the schedule
> + * table capacity of SJA1105 (SJA1110 provides 4096). Cap the entry list
> + * at 1024.
> + */
> +#define GATE_ENTRIES_MAX	1024
> +
>  static ktime_t gate_get_time(struct tcf_gate *gact)
>  {
>  	ktime_t mono = ktime_get();
> @@ -277,6 +288,14 @@ static int parse_gate_list(struct nlattr *list_attr,
>  			continue;
>  		}
> 
> +		if (i >= GATE_ENTRIES_MAX) {
> +			NL_SET_ERR_MSG_FMT(extack,
> +					   "Too many schedule entries, at most %u are supported",
> +					   GATE_ENTRIES_MAX);
> +			err = -E2BIG;
> +			goto release_list;
> +		}
> +
>  		entry = kzalloc_obj(*entry, GFP_ATOMIC);
>  		if (!entry) {
>  			NL_SET_ERR_MSG(extack, "Not enough memory for entry");
> --
> 2.43.0
> 
> 

If I remember correctly, act_gate was/is not fully taking into account how much size it actually
needs for dumps. Has this been fixed or worked around already? Sorry, I'd been meaning to swing 
back to this. I'll try to get caught up today.

https://lore.kernel.org/stable/20260130134220.305757-1-p@1g4.org/

Thanks,
Paul

Re: [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries

[Jamal Hadi Salim]

On Wed, Oct 7, 2026 at 10:23 AM Paul Moses <p@1g4.org> wrote:
>
> On Wednesday, October 7th, 2026 at 5:29 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> > parse_gate_list() accepts _any number_ of TCA_GATE_ONE_ENTRY elements.
> > The only bound/cap is the enclosing nlattr u16 nla_len (~5460
> > minimal 12-byte entries). Each entry is a separate GFP_ATOMIC
> > allocation, and the dump of a large list grows one action's reply well
> > past NLMSG_GOODSIZE.
> >
> > The bugs:
> > 1. A single gate can make tcf_dump_walker() silently truncate
> > "tc actions ls"
> > 2. Lack of capping has been demonstrated to be exploitable to
> > create an OOM through amplification of RTM_GETACTION replies.
> >
> > So we need to provide an upper bound cap of TCA_GATE_ONE_ENTRY elements.
> >
> > Some context:
> > The gate action offloads through FLOW_ACTION_GATE. The offload
> > interface passes the priority, base time, cycle time, cycle-time
> > extension, the complete entry count and an array holding each entry's
> > gate state, interval, IPV and maximum-octet value. The in-tree SJA1105
> > DSA driver forwards that complete list to sja1105_vl_gate() without any
> > entry cap check, and composes every supplied entry into the hardware
> > schedule table.
> >
> > Act gate challenges:
> > SJA1105 and SJA1110 are two device families in the one sja1105 driver.
> > They expose different schedule-table capacities: SJA1105 provides up to
> > 1024 schedule entries and SJA1110 up to 4096. These are total table
> > capacities shared with other schedules, so they are upper bounds, not a
> > guaranteed per-action budget. Software gate actions with _a lot more_
> > entries install and serialize today, so a low cap would reject working
> > configurations and would look like a UAPI breakage; but an infinite size
> > of entries is a bogus choice; 65 entries is the boundary configuration
> > verified during this work.
> >
> > Netlink Challenges:
> > Netlink attributes record their total size, including the four-byte
> > header, in a 16-bit nla_len, so a nested attribute can describe at most
> > 65532 bytes. A minimal accepted input entry is 12 bytes, which puts the
> > max we can fit at roughly 5460 entries; a dumped entry occupies 36 bytes,
> > or 40 bytes when it carries the gate-open flag.
> > 1024 entries therefore serialize to 36868-40964 bytes, which fits
> > inside one nested attribute, while a 4096-entry schedule would need
> > roughly 144-160 KiB which _cannot fit_ at the moment due to the 16bit
> > length.
> >
> > Note, Note: This represents challenges primarily with netlink.
> >
> > The Cap:
> > Cap the list at 1024 entries. This value is based on the existing
> > SJA1105 schedule-table capacity and preserves the verified 65-entry
> > configuration while providing a substantial headroom, and keeps the
> > complete gate list inside the current 16-bit TLV format. It
> > deliberately declines to expose SJA1110's 1025-4096 range through
> > an interface that cannot serialize it, and deliberately rejects
> > software-only schedules above 1024.
> >
> > This is an operational cap on a configuration the uAPI accepted before,
> > so it is net-next hardening; if we made this a stable backport then
> > it would start rejecting gate configurations that install today (even
> > though those settings would be totally bogus).
> >
> > Caveat Emptor:
> > While this patch fixes the binding of per-action storage and the dump
> > amplification; it does not by itself fix tcf_dump_walker() truncation
> > whose skb is smaller than the reply for such an action (for example the
> > kernel clamps a dump skb via netlink_recvmsg()/netlink_dump() to
> > SKB_WITH_OVERHEAD(32768), so "tc actions ls" is already truncated below
> > the 1024 this cap admits, and you cannot dump the top of the range
> > without changing iproute2 code)
> > It also does not remove generic netlink receive-queue amplification.
> > A companion iproute2 change can enumerate actions with a terse dump and
> > then fetch each one with an indexed RTM_GETACTION.
> >
> > Conditions to recreate the bug:
> > Cap net admin with CONFIG_NET_ACT_GATE=y. Install a gate
> > action with 1025 or more minimal TCA_GATE_ONE_ENTRY elements over a raw
> > netlink RTM_NEWACTION request. The cap rejects the list with -E2BIG and
> > an extack naming the limit; 1024 entries install and an indexed
> > RTM_GETACTION returns a well-formed reply.
> >
> > Reviewed-by: Victor Nogueira <victor@mojatatu.com>
> > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> > ---
> >  net/sched/act_gate.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
> > index 14801c604bd9..d8e8c2356bd5 100644
> > --- a/net/sched/act_gate.c
> > +++ b/net/sched/act_gate.c
> > @@ -18,6 +18,17 @@
> >
> >  static struct tc_action_ops act_gate_ops;
> >
> > +/* A netlink attribute records its total length, including the 4-byte
> > + * header, in a u16 nla_len, so one nested attribute can describe at most
> > + * 65532 bytes. tcf_gate_dump() emits each schedule entry as 36 bytes, or
> > + * 40 with the gate-open flag, so 1024 entries serialize to 36868-40964
> > + * bytes and still fit a single TCA_GATE_ENTRY_LIST nest, while 4096 would
> > + * need ~144-160 KiB and cannot be represented. 1024 is also the schedule
> > + * table capacity of SJA1105 (SJA1110 provides 4096). Cap the entry list
> > + * at 1024.
> > + */
> > +#define GATE_ENTRIES_MAX     1024
> > +
> >  static ktime_t gate_get_time(struct tcf_gate *gact)
> >  {
> >       ktime_t mono = ktime_get();
> > @@ -277,6 +288,14 @@ static int parse_gate_list(struct nlattr *list_attr,
> >                       continue;
> >               }
> >
> > +             if (i >= GATE_ENTRIES_MAX) {
> > +                     NL_SET_ERR_MSG_FMT(extack,
> > +                                        "Too many schedule entries, at most %u are supported",
> > +                                        GATE_ENTRIES_MAX);
> > +                     err = -E2BIG;
> > +                     goto release_list;
> > +             }
> > +
> >               entry = kzalloc_obj(*entry, GFP_ATOMIC);
> >               if (!entry) {
> >                       NL_SET_ERR_MSG(extack, "Not enough memory for entry");
> > --
> > 2.43.0
> >
> >
>
> If I remember correctly, act_gate was/is not fully taking into account how much size it actually
> needs for dumps. Has this been fixed or worked around already? Sorry, I'd been meaning to swing
> back to this. I'll try to get caught up today.
>
> https://lore.kernel.org/stable/20260130134220.305757-1-p@1g4.org/

Probably most of that is already in - re-run your tests on the current
kernel tree and if it still fails apply this patch on top and see if
it happens. We have quiet a few gate issues being reported and we are
systematically attacking them;  this and the ones Victor had already
sent are the highest prio. Others will follow later.

cheers,
jamal
>
> Thanks,
> Paul

Re: [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 (2) · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] With the 1024-entry cap, a single unprivileged RTM_GETACTION
  request can still make the kernel allocate about 1.3 MB of reply…
- [Medium] GATE_ENTRIES_MAX = 1024 still allows a single gate whose dump
  cannot fit in any netlink dump skb, so the silent 'tc actions ls'…
- [Low] The commit message calls the unbounded gate list 'bugs' and says
  it was 'demonstrated to be exploitable to create an OOM', but there is…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but batched
  action requests can carry several capped gates whose combined size…
- [Medium] This problem wasn't introduced by this patch, but the sja1105
  driver never checks the total number of hardware Schedule table…

--

net/sched: act_gate: cap the number of scheduling entries

This patch limits parse_gate_list() to 1024 TCA_GATE_ONE_ENTRY elements.
Longer lists are rejected with -E2BIG and an extack message. The value was
picked so that a fully dumped entry list still fits in one u16-length
TCA_GATE_ENTRY_LIST nest, and it matches the SJA1105 schedule table size.

> The bugs:
> 1. A single gate can make tcf_dump_walker() silently truncate
> "tc actions ls"
> 2. Lack of capping has been demonstrated to be exploitable to
> create an OOM through amplification of RTM_GETACTION replies.

[Severity: Low]
The commit message calls these bugs, but there is no Fixes: tag for the
commit that added act_gate:

a51c328df310 ("net: qos: introduce a gate control flow action")

Documentation/process/maintainer-netdev.rst says "for fixes the Fixes: tag
is required, regardless of the tree".

The commit message also says this is net-next hardening and should not be
backported, so leaving the tag out looks intentional.

Would a Fixes: tag together with "Cc: <stable+noautosel@kernel.org>" record
where the problem came from without triggering an automatic backport?

> diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
> index 14801c604bd9..d8e8c2356bd5 100644
> --- a/net/sched/act_gate.c
> +++ b/net/sched/act_gate.c
> @@ -18,6 +18,17 @@
>  
>  static struct tc_action_ops act_gate_ops;
>  
> +/* A netlink attribute records its total length, including the 4-byte
> + * header, in a u16 nla_len, so one nested attribute can describe at most
> + * 65532 bytes. tcf_gate_dump() emits each schedule entry as 36 bytes, or
> + * 40 with the gate-open flag, so 1024 entries serialize to 36868-40964
> + * bytes and still fit a single TCA_GATE_ENTRY_LIST nest, while 4096 would
> + * need ~144-160 KiB and cannot be represented. 1024 is also the schedule
> + * table capacity of SJA1105 (SJA1110 provides 4096). Cap the entry list
> + * at 1024.
> + */
> +#define GATE_ENTRIES_MAX	1024

[Severity: Medium]
This sizes the cap against the u16 nla_len of the nest. Should it also be
sized against the largest dump skb?

netlink_recvmsg() limits the dump buffer size:

net/netlink/af_netlink.c:netlink_recvmsg() {
    ...
	max_recvmsg_len = min_t(size_t, max_recvmsg_len,
				SKB_WITH_OVERHEAD(32768));
    ...
}

For RTM_GETACTION dumps, netlink_dump() allocates at most about 32.4 KB.
dumping_entry() emits 36 or 40 bytes per entry. A gate with more than about
810-900 entries therefore cannot be dumped, and the new cap still accepts
it.

When this happens, tcf_dump_walker() trims the action and returns without
an error:

net/sched/act_api.c:tcf_dump_walker() {
    ...
		if (err < 0) {
			index--;
			nlmsg_trim(skb, nest);
			goto done;
		}
    ...
}

If the gate is the first action in the skb, n_i is 0. tc_dump_action()
then returns a skb->len of 0, and netlink_dump() sends NLMSG_DONE with
success. If earlier actions fit, the next callback restarts at the gate's
index and ends the same way.

The result is that the gate and every later action in the IDR are missing
from "tc actions ls", and userspace gets no error.

The commit message lists this truncation first under "The bugs:". The
Caveat Emptor section then says the cap does not prevent it.

Should the cap be small enough that a single gate fits in a dump skb? If
not, should the commit message stop listing this truncation as something
the cap fixes?

[Severity: Medium]
This isn't a bug introduced by this patch, but the cap is based on the
SJA1105 schedule table size. Does the sja1105 driver check the total number
of Schedule table entries against the device capacity anywhere?

The cap only limits a single action. sja1105_vl_gate() copies num_entries
with no limit:

drivers/net/dsa/sja1105/sja1105_vl.c:sja1105_vl_gate() {
    ...
	rule->vl.num_entries = num_entries;
    ...
}

sja1105_init_scheduling() adds the taprio and gate entry counts together.
It then sets the table size without checking max_entry_count:

drivers/net/dsa/sja1105/sja1105_tas.c:sja1105_init_scheduling() {
    ...
	table->entry_count = num_entries;
    ...
}

sja1105_static_config_check_valid() does not reject an oversized Schedule
table either.

On SJA1105, the entry point address and subscheind[] fields are 10 bits
wide, so sja1105_packing() cannot encode schedule indices of 1024 or more.
A 1024-entry gate plus a taprio schedule or a second offloaded gate rule
would overflow the shared table.

The commit message already notes that these are shared total capacities
and that the driver has no entry cap check. Is a separate driver fix
planned?

[ ... ]

> @@ -277,6 +288,14 @@ static int parse_gate_list(struct nlattr *list_attr,
>  			continue;
>  		}
>  
> +		if (i >= GATE_ENTRIES_MAX) {
> +			NL_SET_ERR_MSG_FMT(extack,
> +					   "Too many schedule entries, at most %u are supported",
> +					   GATE_ENTRIES_MAX);
> +			err = -E2BIG;
> +			goto release_list;
> +		}
> +

[Severity: Medium]
This limits each gate to about 41 KB of fill size. Does it also limit the
size of an RTM_GETACTION reply? The commit message says:

  While this patch fixes the binding of per-action storage and the dump
  amplification

RTM_GETACTION skips the capability check in tc_ctl_action():

net/sched/act_api.c:tc_ctl_action() {
    ...
	if ((n->nlmsg_type != RTM_GETACTION) &&
	    !netlink_capable(skb, CAP_NET_ADMIN))
		return -EPERM;
    ...
}

tca_action_gd() accepts up to TCA_ACT_MAX_PRIO (32) TCA_ACT_TAB entries.
All of them can name the same gate index, because tcf_action_get_1() takes
another reference without checking for duplicates. For each entry it does:

	attr_size += tcf_action_fill_size(act);

tcf_gate_get_fill_size() budgets 40 bytes per entry. With one 1024-entry
gate in the netns, 32 copies add up to about 1.3 MB, and tcf_get_notify()
allocates that much:

	skb = alloc_skb(max(attr_size, NLMSG_GOODSIZE), GFP_KERNEL);

Once the total is above U16_MAX, tca_get_fill() always fails here:

	if (nla_nest_end_safe(skb, nest) < 0)
		goto out_nlmsg_trim;

and the skb is freed.

This does not lead to OOM. The doit runs under rtnl, so these allocations
are serialized and freed immediately.

Still, an unprivileged request of a few hundred bytes can repeatedly force
an order-9 GFP_KERNEL allocation while rtnl is held, and the buffer is
always discarded.

Could tca_action_gd()/tcf_get_notify() reject an attr_size larger than a
u16 TCA_ACT_TAB nest can hold, before calling alloc_skb()?

[Severity: Medium]
This is a pre-existing issue, but one capped gate takes about 41 KB in a
dump. Two of them in one batched request therefore exceed the u16
TCA_ACT_TAB nest in tca_get_fill():

	if (nla_nest_end_safe(skb, nest) < 0)
		goto out_nlmsg_trim;

For RTM_NEWACTION, tcf_action_add() has already installed the actions by
the time tcf_add_notify() fails. The caller gets -EINVAL even though the
actions were installed:

	ret = tcf_add_notify(net, n, actions, portid, attr_size, extack);

For RTM_DELACTION, tcf_del_notify() returns before tcf_action_delete() is
called, so nothing is deleted:

		skb = tcf_del_notify_msg(net, n, actions, portid, attr_size,
					 extack);
		if (IS_ERR(skb))
			return PTR_ERR(skb);

A batched RTM_GETACTION fails too.

The add and delete failures only happen when rtnl_notify_needed() is true,
i.e. there is an RTNLGRP_TC listener or NLM_F_ECHO is set. Whether they
occur depends on whether something like tc monitor is running.

The previous patch in this series, "net/sched: act_gate: reject oversized
dumps instead of wrapping them", turned the earlier wrapped length into
these failures and left the add-side -EINVAL for later. It fixed the filter
delete path, but tcf_del_notify() still blocks action deletes.

Is a follow-up planned for the action delete path?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-H19Z.v2.20261007062551%40mojatatu.com

Re: [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries

[Paul Moses]

> [Severity: Medium]
> This sizes the cap against the u16 nla_len of the nest. Should it also be
> sized against the largest dump skb?
> 
> netlink_recvmsg() limits the dump buffer size:
> 
> net/netlink/af_netlink.c:netlink_recvmsg() {
>     ...
> 	max_recvmsg_len = min_t(size_t, max_recvmsg_len,
> 				SKB_WITH_OVERHEAD(32768));
>     ...
> }
> 
> For RTM_GETACTION dumps, netlink_dump() allocates at most about 32.4 KB.
> dumping_entry() emits 36 or 40 bytes per entry. A gate with more than about
> 810-900 entries therefore cannot be dumped, and the new cap still accepts
> it.
> 
> When this happens, tcf_dump_walker() trims the action and returns without
> an error:
> 
> net/sched/act_api.c:tcf_dump_walker() {
>     ...
> 		if (err < 0) {
> 			index--;
> 			nlmsg_trim(skb, nest);
> 			goto done;
> 		}
>     ...
> }
> 
> If the gate is the first action in the skb, n_i is 0. tc_dump_action()
> then returns a skb->len of 0, and netlink_dump() sends NLMSG_DONE with
> success. If earlier actions fit, the next callback restarts at the gate's
> index and ends the same way.
> 
> The result is that the gate and every later action in the IDR are missing
> from "tc actions ls", and userspace gets no error.
> 
> The commit message lists this truncation first under "The bugs:". The
> Caveat Emptor section then says the cap does not prevent it.
> 
> Should the cap be small enough that a single gate fits in a dump skb? If
> not, should the commit message stop listing this truncation as something
> the cap fixes?
> 

This is the one I encountered, not that the cap should be smaller or that
this type of dump needs to work, it's confusing that it returns 0 and stops.

This needs more review with regard to other callers but to give you an idea.

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index e45a63be397c..4dfc050c48c4 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -589,18 +589,13 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
                tcf_action_update_hw_stats(p);
 
                nest = nla_nest_start_noflag(skb, n_i);
-               if (!nest) {
-                       index--;
+               if (!nest)
                        goto nla_put_failure;
-               }
                err = (act_flags & TCA_ACT_FLAG_TERSE_DUMP) ?
                        tcf_action_dump_terse(skb, p, true) :
                        tcf_action_dump_1(skb, p, 0, 0);
-               if (err < 0) {
-                       index--;
-                       nlmsg_trim(skb, nest);
-                       goto done;
-               }
+               if (err < 0)
+                       goto nla_put_failure;
                nla_nest_end(skb, nest);
                n_i++;
                if (!(act_flags & TCA_ACT_FLAG_LARGE_DUMP_ON) &&
@@ -616,10 +611,15 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
                if (act_flags & TCA_ACT_FLAG_LARGE_DUMP_ON)
                        cb->args[1] = n_i;
        }
-       return n_i;
+       return n_i ?: err;
 
 nla_put_failure:
+       index--;
        nla_nest_cancel(skb, nest);
+       err = -EMSGSIZE;
+       if (!n_i)
+               NL_SET_ERR_MSG(cb->extack,
+                              "TC action is too large for the dump buffer");
        goto done;
 }
 
@@ -2260,6 +2260,7 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
                msecs_since = nla_get_u32(tb[TCA_ROOT_TIME_DELTA]);
        }
 
+       ret = -EMSGSIZE;
        nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
                        cb->nlh->nlmsg_type, sizeof(*t), 0);
        if (!nlh)
@@ -2303,7 +2304,7 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
 out_module_put:
        module_put(a_o->owner);
        nlmsg_trim(skb, b);
-       return skb->len;
+       return ret;
 }
 
 static const struct rtnl_msg_handler tc_action_rtnl_msg_handlers[] __initconst = {

Re: [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries

[Jamal Hadi Salim]

On Sun, Oct 11, 2026 at 2:56 PM Paul Moses <p@1g4.org> wrote:
>
> > [Severity: Medium]
> > This sizes the cap against the u16 nla_len of the nest. Should it also be
> > sized against the largest dump skb?
> >
> > netlink_recvmsg() limits the dump buffer size:
> >
> > net/netlink/af_netlink.c:netlink_recvmsg() {
> >     ...
> >       max_recvmsg_len = min_t(size_t, max_recvmsg_len,
> >                               SKB_WITH_OVERHEAD(32768));
> >     ...
> > }
> >
> > For RTM_GETACTION dumps, netlink_dump() allocates at most about 32.4 KB.
> > dumping_entry() emits 36 or 40 bytes per entry. A gate with more than about
> > 810-900 entries therefore cannot be dumped, and the new cap still accepts
> > it.
> >
> > When this happens, tcf_dump_walker() trims the action and returns without
> > an error:
> >
> > net/sched/act_api.c:tcf_dump_walker() {
> >     ...
> >               if (err < 0) {
> >                       index--;
> >                       nlmsg_trim(skb, nest);
> >                       goto done;
> >               }
> >     ...
> > }
> >
> > If the gate is the first action in the skb, n_i is 0. tc_dump_action()
> > then returns a skb->len of 0, and netlink_dump() sends NLMSG_DONE with
> > success. If earlier actions fit, the next callback restarts at the gate's
> > index and ends the same way.
> >
> > The result is that the gate and every later action in the IDR are missing
> > from "tc actions ls", and userspace gets no error.
> >
> > The commit message lists this truncation first under "The bugs:". The
> > Caveat Emptor section then says the cap does not prevent it.
> >
> > Should the cap be small enough that a single gate fits in a dump skb? If
> > not, should the commit message stop listing this truncation as something
> > the cap fixes?
> >
>
> This is the one I encountered, not that the cap should be smaller or that
> this type of dump needs to work, it's confusing that it returns 0 and stops.
>

I will look closely tommorow - if i can reproduce it i will add an
extra patch to address this (already adding one for taprio with
similar issue i caught earlier today).


cheers,
jamal

> This needs more review with regard to other callers but to give you an idea.
>
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index e45a63be397c..4dfc050c48c4 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -589,18 +589,13 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
>                 tcf_action_update_hw_stats(p);
>
>                 nest = nla_nest_start_noflag(skb, n_i);
> -               if (!nest) {
> -                       index--;
> +               if (!nest)
>                         goto nla_put_failure;
> -               }
>                 err = (act_flags & TCA_ACT_FLAG_TERSE_DUMP) ?
>                         tcf_action_dump_terse(skb, p, true) :
>                         tcf_action_dump_1(skb, p, 0, 0);
> -               if (err < 0) {
> -                       index--;
> -                       nlmsg_trim(skb, nest);
> -                       goto done;
> -               }
> +               if (err < 0)
> +                       goto nla_put_failure;
>                 nla_nest_end(skb, nest);
>                 n_i++;
>                 if (!(act_flags & TCA_ACT_FLAG_LARGE_DUMP_ON) &&
> @@ -616,10 +611,15 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
>                 if (act_flags & TCA_ACT_FLAG_LARGE_DUMP_ON)
>                         cb->args[1] = n_i;
>         }
> -       return n_i;
> +       return n_i ?: err;
>
>  nla_put_failure:
> +       index--;
>         nla_nest_cancel(skb, nest);
> +       err = -EMSGSIZE;
> +       if (!n_i)
> +               NL_SET_ERR_MSG(cb->extack,
> +                              "TC action is too large for the dump buffer");
>         goto done;
>  }
>
> @@ -2260,6 +2260,7 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
>                 msecs_since = nla_get_u32(tb[TCA_ROOT_TIME_DELTA]);
>         }
>
> +       ret = -EMSGSIZE;
>         nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
>                         cb->nlh->nlmsg_type, sizeof(*t), 0);
>         if (!nlh)
> @@ -2303,7 +2304,7 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb)
>  out_module_put:
>         module_put(a_o->owner);
>         nlmsg_trim(skb, b);
> -       return skb->len;
> +       return ret;
>  }
>
>  static const struct rtnl_msg_handler tc_action_rtnl_msg_handlers[] __initconst = {
>
>
>

Re: [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries

[Jamal Hadi Salim]

On Sun, Oct 11, 2026 at 7:46 PM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> On Sun, Oct 11, 2026 at 2:56 PM Paul Moses <p@1g4.org> wrote:
> >
> > > [Severity: Medium]
> > > This sizes the cap against the u16 nla_len of the nest. Should it also be
> > > sized against the largest dump skb?
> > >
> > > netlink_recvmsg() limits the dump buffer size:
> > >
> > > net/netlink/af_netlink.c:netlink_recvmsg() {
> > >     ...
> > >       max_recvmsg_len = min_t(size_t, max_recvmsg_len,
> > >                               SKB_WITH_OVERHEAD(32768));
> > >     ...
> > > }
> > >
> > > For RTM_GETACTION dumps, netlink_dump() allocates at most about 32.4 KB.
> > > dumping_entry() emits 36 or 40 bytes per entry. A gate with more than about
> > > 810-900 entries therefore cannot be dumped, and the new cap still accepts
> > > it.
> > >
> > > When this happens, tcf_dump_walker() trims the action and returns without
> > > an error:
> > >
> > > net/sched/act_api.c:tcf_dump_walker() {
> > >     ...
> > >               if (err < 0) {
> > >                       index--;
> > >                       nlmsg_trim(skb, nest);
> > >                       goto done;
> > >               }
> > >     ...
> > > }
> > >
> > > If the gate is the first action in the skb, n_i is 0. tc_dump_action()
> > > then returns a skb->len of 0, and netlink_dump() sends NLMSG_DONE with
> > > success. If earlier actions fit, the next callback restarts at the gate's
> > > index and ends the same way.
> > >
> > > The result is that the gate and every later action in the IDR are missing
> > > from "tc actions ls", and userspace gets no error.
> > >
> > > The commit message lists this truncation first under "The bugs:". The
> > > Caveat Emptor section then says the cap does not prevent it.
> > >
> > > Should the cap be small enough that a single gate fits in a dump skb? If
> > > not, should the commit message stop listing this truncation as something
> > > the cap fixes?
> > >
> >
> > This is the one I encountered, not that the cap should be smaller or that
> > this type of dump needs to work, it's confusing that it returns 0 and stops.
> >
>
> I will look closely tommorow - if i can reproduce it i will add an
> extra patch to address this (already adding one for taprio with
> similar issue i caught earlier today).
>

I have reproduced it. TBH, this is something we already captured as a
followup from sashiko and planned to submit separately later. I will
make an exception this time since i started looking at fixing it
(starting with what you suggested).
The problem with sashiko is you get punished every time you submit a
patch, so it may look like tc is submitting a lot of fixes but that is
because it is a recursion: every time.
To put it in scripture terms: no good deed goes unpunished and woe
unto them who are not fixing the "pre-existing" issues for the
secondeth flood wave is coming unto them real sooneth when the kiddies
start poking at sashiko suggestions.

So by adding this one more patch (since I am working on it) it will
increase the sashiko attack surface and we are going to send one more
revision, but like i said will make an exception this time.

cheers,
jamal