Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] net/sched: act_gate: reject oversized dumps and cap entry count
@ 2026-10-07 10:29 Jamal Hadi Salim
  2026-10-07 10:29 ` [PATCH net-next 1/2] net/sched: act_gate: reject oversized dumps instead of wrapping them Jamal Hadi Salim
  2026-10-07 10:29 ` [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries Jamal Hadi Salim
  0 siblings, 2 replies; 5+ messages in thread
From: Jamal Hadi Salim @ 2026-10-07 10:29 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Victor Nogueira, Jiri Pirko, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Vladimir Oltean, Vladimir Oltean, Paul Moses, Edward Adam Davis,
	Davide Caratti, sashiko-bot

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


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

* [PATCH net-next 1/2] net/sched: act_gate: reject oversized dumps instead of wrapping them
  2026-10-07 10:29 [PATCH net-next 0/2] net/sched: act_gate: reject oversized dumps and cap entry count Jamal Hadi Salim
@ 2026-10-07 10:29 ` Jamal Hadi Salim
  2026-10-07 10:29 ` [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries Jamal Hadi Salim
  1 sibling, 0 replies; 5+ messages in thread
From: Jamal Hadi Salim @ 2026-10-07 10:29 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Victor Nogueira, Jiri Pirko, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Daniel Borkmann, Martin KaFai Lau, John Fastabend,
	Stanislav Fomichev, bpf, sashiko-bot

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


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

* [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries
  2026-10-07 10:29 [PATCH net-next 0/2] net/sched: act_gate: reject oversized dumps and cap entry count Jamal Hadi Salim
  2026-10-07 10:29 ` [PATCH net-next 1/2] net/sched: act_gate: reject oversized dumps instead of wrapping them Jamal Hadi Salim
@ 2026-10-07 10:29 ` Jamal Hadi Salim
  2026-10-07 14:23   ` Paul Moses
  1 sibling, 1 reply; 5+ messages in thread
From: Jamal Hadi Salim @ 2026-10-07 10:29 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, Victor Nogueira, Jiri Pirko, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Vladimir Oltean, Vladimir Oltean, Paul Moses, Edward Adam Davis,
	Davide Caratti, dsahern, stephen, sashiko-bot

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


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

* Re: [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries
  2026-10-07 10:29 ` [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries Jamal Hadi Salim
@ 2026-10-07 14:23   ` Paul Moses
  2026-10-08  7:51     ` Jamal Hadi Salim
  0 siblings, 1 reply; 5+ messages in thread
From: Paul Moses @ 2026-10-07 14:23 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev, Victor Nogueira, Jiri Pirko, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Vladimir Oltean, Vladimir Oltean, Edward Adam Davis,
	Davide Caratti, dsahern, stephen, sashiko-bot

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

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

* Re: [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries
  2026-10-07 14:23   ` Paul Moses
@ 2026-10-08  7:51     ` Jamal Hadi Salim
  0 siblings, 0 replies; 5+ messages in thread
From: Jamal Hadi Salim @ 2026-10-08  7:51 UTC (permalink / raw)
  To: Paul Moses
  Cc: netdev, Victor Nogueira, Jiri Pirko, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Vladimir Oltean, Vladimir Oltean, Edward Adam Davis,
	Davide Caratti, dsahern, stephen, sashiko-bot

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

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

end of thread, other threads:[~2026-10-08  7:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-07 10:29 [PATCH net-next 0/2] net/sched: act_gate: reject oversized dumps and cap entry count Jamal Hadi Salim
2026-10-07 10:29 ` [PATCH net-next 1/2] net/sched: act_gate: reject oversized dumps instead of wrapping them Jamal Hadi Salim
2026-10-07 10:29 ` [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries Jamal Hadi Salim
2026-10-07 14:23   ` Paul Moses
2026-10-08  7:51     ` 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