[PATCH net-next v3 0/5] net/sched: reject oversized action dumps and cap unbounded entry lists

[PATCH net-next v3 0/5] net/sched: reject oversized action dumps and cap unbounded entry lists

[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, bounds the tfilter_notify_prep() retry,
and stops a notification that cannot be built from vetoing a filter or
action delete. Patch 2 stops an action add whose actions were already
committed from being reported as a failure when the notification cannot
be built. Patch 3 caps the number of gate scheduling entries at 1024,
rejected with -E2BIG and an extack message. Patch 4 caps the number of
taprio schedule entries at 1024 in the same way. Patch 5 makes the action
dump report an action that cannot be serialized into one dump skb as
-EMSGSIZE instead of returning success and silently dropping the rest of
the list.

Patches 3 and 4 each bound an unbounded userspace-supplied entry list that
a reviewer reading the gate cap would ask about next: the qdisc-side
sibling and the SJA1105 schedule-table producer. Patch 5 closes the
silent-truncation that the cap admits but cannot reach. They are folded
into this posting so the family is fixed in one patchset rather than a
stream of follow-ups; each stays a separate commit, so any one can drop
out without touching the others.

A pre-existing filter reference-count leak in cls_flower, found alongside
this work, is being sent to net as its own fix with a Fixes: tag; it is not
part of this series.

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.

This is hardening, not a regression fix, so it targets net-next.

v1 -> v2:
  - 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 (now 3/5), 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

v2 -> v3:
  - Patch 1: the action-delete path no longer lets an unbuildable
    RTM_DELACTION notification veto the delete.  tca_get_fill() and
    tcf_del_notify_msg() pass -EMSGSIZE up, and tcf_del_notify() drops the
    notification and proceeds with the delete.
  - Patch 1: the filter-delete path drops an unbuildable notification and
    proceeds with the delete; its "Filter deleted" extack is set only after
    ->delete() has returned 0.
  - Patch 1: the retry bound is documented as part of the same change as
    the checked closes, and -EMSGSIZE is noted as the general
    tcf_fill_node() outcome rather than a u16 overflow only.
  - Patch 2: the action-add path no longer reports a failure after the
    actions are committed.  tcf_action_add() drops a notification that
    cannot be built and returns success, so an RTM_NEWACTION that created
    its actions is no longer reported back as -EINVAL (mirrors
    tc_new_tfilter(), which never fails a filter create on a notify
    failure).
  - Sashiko reviews of v2:
    https://sashiko.dev/#/patchset/QDISC-H19Z.v2.20261007062551%40mojatatu.com
    https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-H19Z.v2.20261007062551%40mojatatu.com
  - Fold the qdisc-side sibling into the series as patch 4/5: cap the
    taprio schedule entry list at 1024 (TAPRIO_ENTRIES_MAX) in
    parse_sched_list(), rejected with -E2BIG and an extack.  Same
    unbounded-entry-list family as the gate cap, grounded in the SJA1105
    schedule-table capacity, and a separate commit.
  - Fold the dump path into the series as patch 5/5: tcf_dump_walker() and
    tc_dump_action() now report an action that cannot be serialized into a
    dump skb as -EMSGSIZE (with an extack) instead of returning success and
    quietly dropping the rest of the list.  Reported and suggested by Paul
    Moses.

Jamal Hadi Salim (5):
  net/sched: act_gate: reject oversized dumps instead of wrapping them
  net/sched: act_api: keep an add that created its actions from failing
  net/sched: act_gate: cap the number of scheduling entries
  net/sched: sch_taprio: cap the number of schedule entries
  net/sched: act_api: report an oversized action dump instead of
    silently truncating it

 net/sched/act_api.c      | 87 ++++++++++++++++++++++++++++------------
 net/sched/act_gate.c     | 18 ++++++++-
 net/sched/cls_api.c      | 38 +++++++++++++-----
 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 +-
 net/sched/sch_taprio.c   | 14 +++++++
 13 files changed, 141 insertions(+), 46 deletions(-)

-- 
2.43.0

[PATCH net-next v3 1/5] 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 a plain nla_nest_end() also warns (DEBUG_NET_WARN_ON_ONCE warns
once per call site), 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.

The retry bound and the checked closes are one change: it is
nla_nest_end_safe() that turns an unrepresentable dump into -EMSGSIZE,
so a backport has to take both together if either is taken.

Context note:
A delete notification that cannot be built must not veto the state
change the caller asked for. Two delete paths built the notification
before the state change and propagated the build failure.
tfilter_del_notify() builds the filter's delete notification before
calling ->delete(), 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.
tcf_del_notify() has the same shape for RTM_DELACTION: it builds the
notification before tcf_action_delete(), so an action list whose
TCA_ACT_TAB exceeds a u16 nest could not be deleted either. In both cases
drop the unbuildable notification instead: on -EMSGSIZE the delete
proceeds. This needs tca_get_fill() and tcf_del_notify_msg() to pass
-EMSGSIZE up instead of collapsing it to -1/-EINVAL, and tca_get_fill()
reports any inner tcf_action_dump() failure as -EMSGSIZE too
(tcf_gate_dump() and tcf_action_dump_1() return plain -1 after one of
their checked closes fails), so a single oversized action's delete is not
vetoed either. On the delete paths the rule is scoped to notification
BUILD failures: a
delivery failure after the state change (rtnetlink_send() failing once
->delete() has already committed) still reports an error to the caller,
as it did before this series. The extack that notes a dropped filter
event is set only after ->delete() has returned 0.
-EMSGSIZE here is the general tcf_fill_node() outcome for any ->dump()
that does not fit, not only a u16 nest overflow; the effect of the new
branch is limited to dropping a best-effort notification.

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).
The same shape as two standalone gate actions is hit on the action path
by RTM_DELACTION with NLM_F_ECHO; before this change the fill fails and
the delete is refused, after it the actions are removed and only the
notification is dropped.
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
per-action cap added by "net/sched: act_gate: cap the number of scheduling
entries": 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://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-H19Z.v2.20261007062551%40mojatatu.com
Link: https://lore.kernel.org/netdev/QDISC-H19Z.v2.20261007062551@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>
---
v1 -> v2:
- Retarget to net-next and drop Fixes:/Cc: stable, as it is hardening
  rather than a regression.
- 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

v2 -> v3:
- Patch 1: the action-delete path no longer lets an unbuildable
  RTM_DELACTION notification veto the delete.  tca_get_fill() and
  tcf_del_notify_msg() pass -EMSGSIZE up, and tcf_del_notify() drops the
  notification and proceeds with the delete.
- Patch 1: the filter-delete path drops an unbuildable notification and
  proceeds with the delete; its "Filter deleted" extack is set only after
  ->delete() has returned 0.
- Patch 1: the retry bound is documented as part of the same change as
  the checked closes, and -EMSGSIZE is noted as the general
  tcf_fill_node() outcome rather than a u16 overflow only.
- Sashiko reviews of v2:
  https://sashiko.dev/#/patchset/QDISC-H19Z.v2.20261007062551%40mojatatu.com
  https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-H19Z.v2.20261007062551%40mojatatu.com
 net/sched/act_api.c      | 52 ++++++++++++++++++++++++++++++----------
 net/sched/act_gate.c     |  3 ++-
 net/sched/cls_api.c      | 38 +++++++++++++++++++++--------
 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, 89 insertions(+), 34 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 6e48b4bc2d75..d57e897cd363 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;
@@ -1673,27 +1676,42 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
 	struct nlmsghdr *nlh;
 	unsigned char *b = skb_tail_pointer(skb);
 	struct nlattr *nest;
+	int err;
 
 	nlh = nlmsg_put(skb, portid, seq, event, sizeof(*t), flags);
-	if (!nlh)
+	if (!nlh) {
+		err = -EMSGSIZE;
 		goto out_nlmsg_trim;
+	}
 	t = nlmsg_data(nlh);
 	t->tca_family = AF_UNSPEC;
 	t->tca__pad1 = 0;
 	t->tca__pad2 = 0;
 
 	if (extack && extack->_msg &&
-	    nla_put_string(skb, TCA_ROOT_EXT_WARN_MSG, extack->_msg))
+	    nla_put_string(skb, TCA_ROOT_EXT_WARN_MSG, extack->_msg)) {
+		err = -EMSGSIZE;
 		goto out_nlmsg_trim;
+	}
 
 	nest = nla_nest_start_noflag(skb, TCA_ACT_TAB);
-	if (!nest)
+	if (!nest) {
+		err = -EMSGSIZE;
 		goto out_nlmsg_trim;
+	}
 
-	if (tcf_action_dump(skb, actions, bind, ref, false) < 0)
+	err = tcf_action_dump(skb, actions, bind, ref, false);
+	if (err < 0) {
+		/* Report any inner dump failure as -EMSGSIZE so
+		 * tcf_del_notify() drops the notification, not the delete.
+		 */
+		err = -EMSGSIZE;
 		goto out_nlmsg_trim;
+	}
 
-	nla_nest_end(skb, nest);
+	err = nla_nest_end_safe(skb, nest);
+	if (err < 0)
+		goto out_nlmsg_trim;
 
 	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
 
@@ -1701,7 +1719,7 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
 
 out_nlmsg_trim:
 	nlmsg_trim(skb, b);
-	return -1;
+	return err;
 }
 
 static int
@@ -1988,17 +2006,19 @@ static struct sk_buff *tcf_del_notify_msg(struct net *net, struct nlmsghdr *n,
 					  struct netlink_ext_ack *extack)
 {
 	struct sk_buff *skb;
+	int err;
 
 	skb = alloc_skb(max(attr_size + tcf_action_warn_attr_size(extack),
 			    NLMSG_GOODSIZE), GFP_KERNEL);
 	if (!skb)
 		return ERR_PTR(-ENOBUFS);
 
-	if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
-			 0, 2, extack) <= 0) {
+	err = tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
+			   0, 2, extack);
+	if (err <= 0) {
 		NL_SET_ERR_MSG(extack, "Failed to fill netlink TC action attributes");
 		kfree_skb(skb);
-		return ERR_PTR(-EINVAL);
+		return ERR_PTR(err ? err : -EINVAL);
 	}
 
 	return skb;
@@ -2016,8 +2036,14 @@ static int tcf_del_notify(struct net *net, struct nlmsghdr *n,
 	} else {
 		skb = tcf_del_notify_msg(net, n, actions, portid, attr_size,
 					 extack);
-		if (IS_ERR(skb))
-			return PTR_ERR(skb);
+		if (IS_ERR(skb)) {
+			/* Such a dump can never be serialized: drop the
+			 * notification rather than refuse the delete.
+			 */
+			if (PTR_ERR(skb) != -EMSGSIZE)
+				return PTR_ERR(skb);
+			skb = NULL;
+		}
 	}
 
 	/* now do the delete */
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..bd0d635fe3fe 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2149,11 +2149,16 @@ 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 u16 nest caps the dump, so once the skb is larger than
+		 * any valid message it can never serialize. Stop at the
+		 * bound instead of looping until alloc_skb() fails.
+		 */
+		if (size > U16_MAX + NLMSG_GOODSIZE)
+			return ERR_PTR(-EMSGSIZE);
+		size += NLMSG_GOODSIZE;
+		goto retry;
 	}
 	return skb;
 }
@@ -2200,8 +2205,18 @@ 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);
+		}
+		/* No notification can ever be built for this filter: drop
+		 * it rather than refuse the delete.
+		 */
+		err = tp->ops->delete(tp, fh, last, rtnl_held, extack);
+		if (!err)
+			NL_SET_ERR_MSG(extack,
+				       "Filter deleted; del event notification could not be built");
+		return err;
 	}
 
 	err = tp->ops->delete(tp, fh, last, rtnl_held, extack);
@@ -3529,7 +3544,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 +3553,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 +3582,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 dca812a75269..6f8b4a98bb84 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -663,7 +663,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

[PATCH net-next v3 2/5] net/sched: act_api: keep an add that created its actions from failing

[Jamal Hadi Salim]

Spotted while fixing the dump sizing reported by Sashiko. This references
commit cfa165cbfbed ("net/sched: act_gate: budget the per-entry list in
get_fill_size"), which is only one way to trigger the issue.

The issue:
tcf_action_add() returns the result of tcf_add_notify(), so an
RTM_NEWACTION is reported back to user space as a failure when the event
notification cannot be built, even though tcf_action_init() has already
inserted the actions into the idr. The realistic trigger is an action
list whose TCA_ACT_TAB exceeds a u16 nest (the notification can never be
built), or a memory-pressure -ENOBUFS from the notification's alloc_skb().

The Fix:
Drop the notification failure on the add path: the created actions
stay installed and the add reports success, with an extack noting that
the event notification could not be sent. The extack is set on a
successful ack, so iproute2 reports it as a warning; it replaces any
warning tcf_action_init() left there. This needs no idr/refcount
audit - tca_put_bound_many() already runs unconditionally after the
notification, so this path releases exactly what the success path does
and only the returned result changes. A create that ignores a
notification failure is already the filter-side norm: tc_new_tfilter()
does not look at tfilter_notify()'s result.

This is net-next hardening: it corrects the reported result of an add
that already took effect, not a regression.

Conditions to recreate the bug:
CONFIG_NET_CLS_ACT=y; CONFIG_NET_ACT_GATE=y. Send an RTM_NEWACTION whose
actions create an aggregate TCA_ACT_TAB larger than a u16 nest, so the
notification cannot be built; before this change the add reports -EINVAL
although the actions were installed, after it the add reports success and
the actions are present.

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://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-H19Z.v2.20261007062551%40mojatatu.com
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/act_api.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index d57e897cd363..949cbc4523a9 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -2167,12 +2167,17 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
 	if (ret < 0)
 		return ret;
 
-	ret = tcf_add_notify(net, n, actions, portid, attr_size, extack);
+	/* The add has already been committed, so a notification failure
+	 * must not turn it into a reported failure (cf. tc_new_tfilter()).
+	 */
+	if (tcf_add_notify(net, n, actions, portid, attr_size, extack))
+		NL_SET_ERR_MSG(extack,
+			       "Action created; event notification could not be sent");
 
 	/* only put bound actions */
 	tca_put_bound_many(actions, init_res);
 
-	return ret;
+	return 0;
 }
 
 static const struct nla_policy tcaa_policy[TCA_ROOT_MAX + 1] = {
-- 
2.43.0

[PATCH net-next v3 3/5] 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" ("net/sched: act_api: report an oversized action dump
instead of silently truncating it" makes such an action report -EMSGSIZE
instead of dropping the rest of the list).
2. Lack of capping pins memory for as long as the action lives: the
largest representable minimal list is roughly 5460 entries, each a
separate GFP_ATOMIC allocation, with a per-entry offload array on top in
full offload. The held-reply RTM_GETACTION amplification demonstrated in
the original posting of this series (Link below) is a generic netlink
property this cap does not remove; the bound added here is the install-side
pin.

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; a 65-entry configuration was verified
during this work to install and dump well-formed.

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: 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.

Why 1024 despite the dump budget:
1024 is a SJA1105 schedule-table capacity, not a dump-budget value. The
kernel clamps a dump skb to SKB_WITH_OVERHEAD(32768), so only ~800-890
entries of this list fit "tc actions ls"; the top of the admitted band
therefore installs but is not listed. That is accepted deliberately:
a unicast RTM_GETACTION reply is pre-sized from the action's advertised
size (budgeted per entry since the parent fix), so "tc actions get"
enumerates the whole band, and "net/sched: act_api: report an oversized
action dump instead of silently truncating it" makes the list view fail
loudly with -EMSGSIZE and an extack rather than silently truncating. A
filter whose action list contains such a gate also stops "tc filter show"
for that chain with -EMSGSIZE (loud), through tc_dump_tfilter()'s
no-progress path.

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).

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.

Link: https://lore.kernel.org/netdev/QDISC-H19Z.v1.20261001053234@mojatatu.com/
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/act_gate.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index 14801c604bd9..711cf2443540 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -18,6 +18,13 @@
 
 static struct tc_action_ops act_gate_ops;
 
+/* A nested attribute's u16 nla_len caps it at 65532 bytes. tcf_gate_dump()
+ * emits each entry as 36 bytes (40 with the gate-open flag), so 4096 entries
+ * would need ~144-160 KiB and cannot be represented. 1024 matches the
+ * SJA1105 schedule-table capacity (SJA1110 provides 4096).
+ */
+#define GATE_ENTRIES_MAX	1024U
+
 static ktime_t gate_get_time(struct tcf_gate *gact)
 {
 	ktime_t mono = ktime_get();
@@ -277,6 +284,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

[PATCH net-next v3 4/5] net/sched: sch_taprio: cap the number of schedule entries

[Jamal Hadi Salim]

parse_sched_list() accepts _any number_ of TCA_TAPRIO_SCHED_ENTRY
elements. The only bound is the enclosing nlattr u16 nla_len. A minimal
accepted entry is 12 bytes (the entry nest plus
TCA_TAPRIO_SCHED_ENTRY_INTERVAL), so that u16 admits roughly 5460
entries - protocol-bounded amplification, not truly unbounded, but far
past any hardware schedule.

The bug:
Each entry is a separate sched_entry allocation (368 bytes, rounded up
to the kmalloc-512 bucket) and, in full offload, is also composed into a
per-entry offload array, so the largest representable minimal list pins
roughly 2.7 MiB for the lifetime of the schedule.

Some context:
The schedule list is not dumped the way the gate action list is: a
taprio dump is bounded by the carrying skb (the 32K dump skb, and
NLMSG_GOODSIZE for notify and unicast get), which keeps the plain
nla_nest_end() closes from ever seeing a wrapped length. But the
input-side amplification is the same class as the act_gate entry list,
and the offload interface composes every supplied entry into the
hardware schedule table without a capacity check.

The top of the accepted band cannot be read back: a schedule above
roughly 900 entries does not fit the 32K dump skb, so "tc qdisc show"
fails loudly with -EMSGSIZE for it (as the action-dump patch does). The
pre-existing silent gaps - no change notification above about 100
entries, and no unicast get reply - are tracked separately.

The Cap:
Cap the list at 1024 entries. This matches the SJA1105 schedule-table
capacity (SJA1110 provides 4096) and the act_gate entry cap, and keeps
the list inside the current 16-bit TLV format.

This is an operational cap on a configuration the uAPI accepted before,
so it is net-next hardening; a stable backport would start rejecting
taprio configurations that install today.

Conditions to recreate the bug:
CAP_NET_ADMIN with CONFIG_NET_SCH_TAPRIO=y. Install a taprio qdisc with
more than 1024 minimal TCA_TAPRIO_SCHED_ENTRY elements over a raw
netlink RTM_NEWQDISC request. The cap rejects the list with -E2BIG and
an extack naming the limit; 1024 entries install.

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

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 39ac5b97aa3a..8e7fb6d0fc00 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -49,6 +49,13 @@ static struct static_key_false taprio_have_working_mqprio;
  */
 #define TAPRIO_PICOS_PER_BYTE_MIN 17
 
+/* Each entry is a separate 368-byte sched_entry and, in full offload, is
+ * also composed into a per-entry offload array. The u16 nla_len admits
+ * roughly 5460 minimal 12-byte entries, pinning ~2 MiB; 1024 matches the
+ * SJA1105 schedule-table capacity (SJA1110 provides 4096) and the act_gate cap.
+ */
+#define TAPRIO_ENTRIES_MAX	1024U
+
 struct sched_entry {
 	/* Durations between this GCL entry and the GCL entry where the
 	 * respective traffic class gate closes
@@ -1105,6 +1112,13 @@ static int parse_sched_list(struct taprio_sched *q, struct nlattr *list,
 			continue;
 		}
 
+		if (i >= TAPRIO_ENTRIES_MAX) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "Too many schedule entries, at most %u are supported",
+					   TAPRIO_ENTRIES_MAX);
+			return -E2BIG;
+		}
+
 		entry = kzalloc_obj(*entry);
 		if (!entry) {
 			NL_SET_ERR_MSG(extack, "Not enough memory for entry");
-- 
2.43.0

[PATCH net-next v3 5/5] net/sched: act_api: report an oversized action dump instead of silently truncating it

[Jamal Hadi Salim]

The issue:
tcf_dump_walker() trims the half-built nest and returns the number of
actions dumped so far as soon as one action does not fit. When the action
at the current index cannot be serialized even into an *empty* dump skb,
that count is zero, so tc_dump_action() returns skb->len (zero) and
netlink sends NLMSG_DONE with no error: "tc actions ls" silently drops
that action and every action after it.

That case is reachable inside the gate entry cap this series adds: a gate
entry serializes to 36 bytes (40 with the gate-open flag) while a dump skb
is clamped to SKB_WITH_OVERHEAD(32768) by netlink_recvmsg()/netlink_dump(),
so the top of the range the cap admits installs but cannot be dumped -
and, before this change, fails silently.

The Fix:
tcf_dump_walker() already has to distinguish the two cases: a walk that
filled the skb before completing the current action must keep returning a
positive count so netlink retries that action in a fresh skb, while an
action that cannot be serialized even into an empty skb must return
-EMSGSIZE. So return n_i when anything was dumped and -EMSGSIZE (with an
extack) when nothing was, and let tc_dump_action() propagate the walker's
error code instead of overwriting it with skb->len. That mirrors
tc_dump_tfilter(), which returns the walker error when a retry made no
progress (if (skb->len == 0 && err) return err;), and tc_dump_chain()
carries the same idiom. This is netlink's documented contract: -EMSGSIZE
with data in the skb means "more to dump", -EMSGSIZE with an empty skb is
delivered to user space.

Context note:
The shared nla_put_failure label stamps -EMSGSIZE for any inner
tcf_action_dump_1()/terse failure, not only a size failure, consistent
with how every other nla_put_failure label in the file reports; the
-EMSGSIZE the action dump now returns is therefore not a strict size
assertion.

The cap stays a memory bound; this only makes an action the cap admits but
the dump buffer cannot carry fail loudly instead of silently truncating
the list. This is a long-standing silent failure, not a regression, so it
is net-next hardening.

Reported-by: Paul Moses <p@1g4.org>
Closes: https://lore.kernel.org/netdev/vPFrhbkwAui8zFPcGQTUDazQfsFDlr5GYuaYQ5BxJcrDiT6x1EljEze4cYadHqbeUW2ivLdpa8eC_cXC6ceACswgb-lW47aSVkHvHjhn_X8=@1g4.org/
Suggested-by: Paul Moses <p@1g4.org>
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/act_api.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 949cbc4523a9..55d553713bd3 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -599,18 +599,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) &&
@@ -626,10 +621,21 @@ 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;
+
+	/* A positive count means the skb filled up before this action was
+	 * complete, so netlink retries it in a fresh skb. An action that
+	 * does not fit even in an empty dump skb (nothing dumped) must be
+	 * reported as -EMSGSIZE by netlink instead of ending the dump.
+	 */
+	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;
 }
 
@@ -2345,7 +2351,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 = {
-- 
2.43.0

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 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] In this commit, when an RTM_NEWACTION notification is needed,
  tcf_add_notify_msg() (net/sched/act_api.c) turns the new -EMSGSIZE…
- [Medium] The commit message describes several failures as pre-existing
  when they only appear once this patch adds nla_nest_end_safe().
- [Low] tcf_del_notify() (net/sched/act_api.c) now treats every -EMSGSIZE
  from tcf_del_notify_msg() as "Such a dump can never be serialized".

--

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

This patch closes the action and classifier nests with nla_nest_end_safe(),
so a dump larger than a u16 nest fails with -EMSGSIZE instead of writing a
wrapped length. It also puts a bound on the tfilter_notify_prep() retry
loop. On the filter and action delete paths, an -EMSGSIZE notification
build failure now drops the notification and lets the delete go ahead.

Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824153903.4143642-1-victor@mojatatu.com
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-H19Z.v2.20261007062551%40mojatatu.com
Link: https://lore.kernel.org/netdev/QDISC-H19Z.v2.20261007062551@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/

> 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 same shape as two standalone gate actions is hit on the action path
> by RTM_DELACTION with NLM_F_ECHO; before this change the fill fails and
> the delete is refused, after it the actions are removed and only the
> notification is dropped.

[Severity: Medium]
Does this describe the behaviour before the patch correctly? Before this
patch, nla_nest_end() cannot fail:

include/net/netlink.h:nla_nest_end() {
    ...
	DEBUG_NET_WARN_ON_ONCE(diff > U16_MAX);
	start->nla_len = diff;
	return skb->len;
}

Also, net/sched had no nla_nest_end_safe() callers before this patch.

Before the patch, tfilter_notify_prep() grows the skb until the dump fits.
It then succeeds, and the TCA_OPTIONS length is wrapped. It does not spin
until alloc_skb() fails.

On the action side, tcf_gate_get_fill_size() already budgets each entry.
So tcf_del_notify_msg() gets a large enough skb, and tca_get_fill()
succeeds with a wrapped TCA_ACT_TAB. The delete goes through rather than
being refused.

The Context note has the same issue. It says "a filter whose dump cannot
be represented in a u16 nest could be created but never removed" and "an
action list whose TCA_ACT_TAB exceeds a u16 nest could not be deleted
either". Both behaviours only appear once the new checked closes are in
place.

The only defect that exists before the patch seems to be the wrapped
(structurally corrupt) reply or notification, plus the DEBUG_NET warning.
The retry bound and the delete path changes look like follow-ons that the
new nla_nest_end_safe() calls require.

Could the commit message be reworded? As written, a backporter might
conclude that current kernels refuse deletes or spin in
tfilter_notify_prep().

> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 6e48b4bc2d75d..4b4b5d37d2206 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c

[ ... ]

> @@ -1673,27 +1676,42 @@ static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],

[ ... ]

> -	if (tcf_action_dump(skb, actions, bind, ref, false) < 0)
> +	err = tcf_action_dump(skb, actions, bind, ref, false);
> +	if (err < 0) {
> +		/* Report any inner dump failure as -EMSGSIZE so
> +		 * tcf_del_notify() drops the notification, not the delete.
> +		 */
> +		err = -EMSGSIZE;
>  		goto out_nlmsg_trim;
> +	}
>  
> -	nla_nest_end(skb, nest);
> +	err = nla_nest_end_safe(skb, nest);
> +	if (err < 0)
> +		goto out_nlmsg_trim;

[Severity: Medium]
How does the RTM_NEWACTION path handle this new failure? tca_get_fill()
can now fail once TCA_ACT_TAB exceeds U16_MAX. On the add side,
tcf_add_notify_msg() still turns that into -EINVAL:

net/sched/act_api.c:tcf_add_notify_msg() {
    ...
	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);
	}
    ...
}

tcf_add_notify() returns that error, and tcf_action_add() passes it to
userspace:

tc_ctl_action(RTM_NEWACTION)
  tcf_action_add()
    tcf_action_init()        <- actions already created or replaced
    tcf_add_notify()
      tcf_add_notify_msg()
        tca_get_fill()
          nla_nest_end_safe() -> -EMSGSIZE -> -EINVAL

This happens when an RTNLGRP_TC listener exists or NLM_F_ECHO is set. The
add then reports failure for a change that was already committed. A retry
gets -EEXIST, and with NLM_F_REPLACE the old configuration is already gone.

Before this patch the fill could not fail here and the add returned 0. The
delete side is handled in this patch, but the add side is not.

The later patch in this series, "net/sched: act_api: keep an add that
created its actions from failing", resolves this. With it, tcf_action_add()
ignores the tcf_add_notify() result, sets the extack "Action created; event
notification could not be sent", and returns 0. Since this patch alone
exposes the problem, would it make sense to fold that change in here or
order it earlier?

A smaller leftover remains at the end of the series. tcf_get_notify()
(RTM_GETACTION) still maps any tca_get_fill() failure, now including the
new -EMSGSIZE, to -EINVAL:

net/sched/act_api.c:tcf_get_notify() {
    ...
	if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, event,
			 0, 1, NULL) <= 0) {
		NL_SET_ERR_MSG(extack, "Failed to fill netlink attributes while adding TC action");
		kfree_skb(skb);
		return -EINVAL;
	}
    ...
}

The filter GET path returns -EMSGSIZE for the same condition, and the
extack text still talks about adding a TC action.

[ ... ]

> @@ -2016,8 +2036,14 @@ static int tcf_del_notify(struct net *net, struct nlmsghdr *n,
>  	} else {
>  		skb = tcf_del_notify_msg(net, n, actions, portid, attr_size,
>  					 extack);
> -		if (IS_ERR(skb))
> -			return PTR_ERR(skb);
> +		if (IS_ERR(skb)) {
> +			/* Such a dump can never be serialized: drop the
> +			 * notification rather than refuse the delete.
> +			 */
> +			if (PTR_ERR(skb) != -EMSGSIZE)
> +				return PTR_ERR(skb);
> +			skb = NULL;
> +		}
>  	}

[Severity: Low]
Is the "can never be serialized" claim accurate for every -EMSGSIZE that
reaches this branch? tcf_del_notify_msg() allocates a single skb of
max(attr_size + warn, NLMSG_GOODSIZE) and calls tca_get_fill() once. Unlike
tfilter_notify_prep(), it has no grow-and-retry loop.

tca_get_fill() also reports every tcf_action_dump() error as -EMSGSIZE.
That includes transient failures, for example in tcf_pedit_dump():

net/sched/act_pedit.c:tcf_pedit_dump() {
    ...
	opt = kzalloc(s, GFP_ATOMIC);
	if (unlikely(!opt)) {
		rcu_read_unlock();
		return -ENOBUFS;
	}
    ...
}

That error becomes -1 in tcf_action_dump_1() and tcf_action_dump(), and
tca_get_fill() then reports it as -EMSGSIZE. An ops->get_fill_size()
underestimate, the bug class fixed by cfa165cbfbed, would also land here.

Before this patch these failures refused the delete with -EINVAL. Now the
delete commits, and RTNLGRP_TC listeners such as tc monitor never see the
RTM_DELACTION event. NLM_F_ECHO callers get no echo, and nothing signals
the lost event.

Should the comment be reworded, or should transient failures be kept apart
from the size-overflow case? This code is unchanged at the end of the
series.

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

Re: [PATCH net-next v3 2/5] net/sched: act_api: keep an add that created its actions from failing

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message says the change is "net-next hardening ... not
  a regression".
- [Low] The "Conditions to recreate the bug" section leaves out a
  required precondition.

--

net/sched: act_api: keep an add that created its actions from failing

This commit changes tcf_action_add() so it no longer passes the result of
tcf_add_notify() back to user space. After this change, an RTM_NEWACTION
whose actions are already in the idr reports success even when the event
notification cannot be built or sent. The failure is reported as an extack
warning instead.

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

> This is net-next hardening: it corrects the reported result of an add
> that already took effect, not a regression.

[Severity: Low]
Is "not a regression" accurate? The message names a "realistic trigger"
and uses it in its reproducer: an action list whose aggregate TCA_ACT_TAB
exceeds a u16 nest. That trigger seems to make the add fail only after the
preceding patch in this series, "net/sched: act_gate: reject oversized
dumps instead of wrapping them".

Before that patch, tca_get_fill(), tcf_action_dump() and tcf_gate_dump()
all closed their nests with plain nla_nest_end(). nla_nest_end() cannot
fail: it warns and writes a wrapped length. Since cfa165cbfbed,
tcf_gate_get_fill_size() budgets every entry, so the skb was large enough.
An oversized add therefore returned 0, although its notification was
corrupt.

The preceding patch switched those closes to nla_nest_end_safe(), so
tca_get_fill() now fails:

net/sched/act_api.c:tca_get_fill() {
    ...
	err = nla_nest_end_safe(skb, nest);
	if (err < 0)
		goto out_nlmsg_trim;
    ...
}

tcf_add_notify_msg() then turns that into -EINVAL:

net/sched/act_api.c:tcf_add_notify_msg() {
    ...
	if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags,
			 RTM_NEWACTION, 0, 0, extack) <= 0) {
		...
		return ERR_PTR(-EINVAL);
	}
    ...
}

Before this patch, tcf_action_add() returned that error to user space even
though tcf_action_init() had already inserted the actions into the idr.

This opens a one-commit bisect window. In it, an RTM_NEWACTION with
oversized aggregate gate actions installs the actions but reports -EINVAL.
This requires an RTNLGRP_TC listener or NLM_F_ECHO. A retry with the same
index then fails with -EEXIST.

The preceding patch fixed this "build failure vetoes the request" pattern
for both delete paths but not for the add path.

Should this change be squashed into the preceding patch, or ordered before
it, with the message saying so? An in-series Fixes: tag would not work,
because the hash is not stable yet.

There are also triggers that predate this series: -ENOBUFS from
alloc_skb(), or any action whose get_fill_size underestimates. They cause
the same user-visible symptom that cfa165cbfbed described. That commit
went in as a net fix with a Fixes: tag.

If this stays a standalone patch, is a Fixes: tag for the long-standing
error propagation in tcf_action_add() worth adding?

> Conditions to recreate the bug:
> CONFIG_NET_CLS_ACT=y; CONFIG_NET_ACT_GATE=y. Send an RTM_NEWACTION whose
> actions create an aggregate TCA_ACT_TAB larger than a u16 nest, so the
> notification cannot be built; before this change the add reports -EINVAL
> although the actions were installed, after it the add reports success and
> the actions are present.

[Severity: Low]
Does this reproducer leave out a required condition? tcf_add_notify() only
builds the notification when rtnl_notify_needed() is true:

net/sched/act_api.c:tcf_add_notify() {
    ...
	if (!rtnl_notify_needed(net, n->nlmsg_flags, RTNLGRP_TC)) {
		skb = NULL;
	} else {
    ...
}

rtnl_notify_needed() is defined as:

	return (nlflags & NLM_F_ECHO) || rtnl_has_listeners(net, group);

With a NULL skb, rtnetlink_maybe_send() returns 0. So if there is no
RTNLGRP_TC listener (for example, no tc monitor running) and the request
does not set NLM_F_ECHO, the oversized RTM_NEWACTION returns 0 even before
this change.

The preceding patch in the series states this condition for the delete
paths ("only when a notification was actually needed (rtnl_notify_needed():
an RTNLGRP_TC listener or NLM_F_ECHO)"). Could the same condition be added
here?

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

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

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Concern (rejected after checking): the commit message relies on
  "net/sched: act_api: report an oversized action dump instead of…

--

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

This commit caps the number of TCA_GATE_ONE_ENTRY elements that
parse_gate_list() accepts at GATE_ENTRIES_MAX (1024). Larger lists are
rejected with -E2BIG and an extack message. The limit is based on the
SJA1105 schedule-table capacity and on the 16-bit nla_len limit of a
nested attribute.

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

> Why 1024 despite the dump budget:
> 1024 is a SJA1105 schedule-table capacity, not a dump-budget value. The
> kernel clamps a dump skb to SKB_WITH_OVERHEAD(32768), so only ~800-890
> entries of this list fit "tc actions ls"; the top of the admitted band
> therefore installs but is not listed. That is accepted deliberately:
> a unicast RTM_GETACTION reply is pre-sized from the action's advertised
> size (budgeted per entry since the parent fix), so "tc actions get"
> enumerates the whole band, and "net/sched: act_api: report an oversized
> action dump instead of silently truncating it" makes the list view fail
> loudly with -EMSGSIZE and an extack rather than silently truncating.

[Severity: Low]
This is a minor point about the commit message, not about the code.

The paragraph reads as if the -EMSGSIZE behaviour is already in place.
The referenced commit is the last patch of this series, though. At this
commit, tcf_dump_walker() in net/sched/act_api.c still drops an action
that does not fit:

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

So at this bisect point, a gate with roughly 890-1024 entries is still
silently left out of "tc actions ls". That is not a regression: the
truncation already happened before this patch, when gates of any size
were allowed. This patch only narrows the range that triggers it, and
the final patch of the series makes it fail loudly.

Could the message say that the act_api change comes later in this
series?

Could "the parent fix" also name the commit it means, presumably
cfa165cbfbed, which added the per-entry get_fill_size budget?

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

Re: [PATCH net-next v3 4/5] net/sched: sch_taprio: cap the number of schedule entries

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 0

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] sch_taprio: hardware capability regression for SJA1110 due to
  1024 entry cap

--

> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> --- a/net/sched/sch_taprio.c
> +++ b/net/sched/sch_taprio.c
> @@ -49,6 +49,13 @@ static struct static_key_false taprio_have_working_mqprio;
>   */
>  #define TAPRIO_PICOS_PER_BYTE_MIN 17
>  
> +/* Each entry is a separate 368-byte sched_entry and, in full offload, is
> + * also composed into a per-entry offload array. The u16 nla_len admits
> + * roughly 5460 minimal 12-byte entries, pinning ~2 MiB; 1024 matches the
> + * SJA1105 schedule-table capacity (SJA1110 provides 4096) and the act_gate cap.
> + */
> +#define TAPRIO_ENTRIES_MAX	1024U
> +
>  struct sched_entry {
>  	/* Durations between this GCL entry and the GCL entry where the
>  	 * respective traffic class gate closes

[Severity: Medium]
The commit message notes that the SJA1110 schedule table holds 4096
entries, but TAPRIO_ENTRIES_MAX is set to 1024.

Before this patch, a user could install and offload a taprio schedule
with anywhere from 1025 up to the SJA1110 hardware limit. With this
patch, parse_sched_list() fails those same configurations with -E2BIG
before the driver ever sees them. So a working full-offload setup on
SJA1110 stops installing after an upgrade. The commit message calls
this net-next hardening, but it is still a user-visible regression for
that hardware.

Why was 1024 chosen over 4096? Is it only to match the act_gate cap?
The allocation concern in the changelog would still be well bounded at
4096. Drivers that offload already have to check the entry count
against their own table size, so a software-side cap at the largest
known hardware capacity seems sufficient.

If 1024 is intentional, for example because schedules above roughly
900 entries cannot be dumped anyway, could the changelog say that
explicitly? It should state that SJA1110 users with more than 1024
entries will be broken by this change.

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

Re: [PATCH net-next v3 5/5] net/sched: act_api: report an oversized action dump instead of silently truncating it

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The patch has no Fixes: tag for the old silent-truncation path in
  tcf_dump_walker() (`index--; nlmsg_trim; goto done; return n_i`).
- [Low] The new comment above `return n_i ?: err;` in tcf_dump_walker()
  (net/sched/act_api.c:625-630) does not match the code or af_netlink's…

--

net/sched: act_api: report an oversized action dump instead of silently truncating it

When an action does not fit even into an empty dump skb, tcf_dump_walker()
now returns -EMSGSIZE with an extack instead of 0. tc_dump_action() now
passes the walker's error up instead of returning skb->len. Before this
change, "tc actions ls" silently left out that action and every action
after it.

Closes: https://lore.kernel.org/netdev/vPFrhbkwAui8zFPcGQTUDazQfsFDlr5GYuaYQ5BxJcrDiT6x1EljEze4cYadHqbeUW2ivLdpa8eC_cXC6ceACswgb-lW47aSVkHvHjhn_X8=@1g4.org/

> That case is reachable inside the gate entry cap this series adds: a gate
> entry serializes to 36 bytes (40 with the gate-open flag) while a dump skb
> is clamped to SKB_WITH_OVERHEAD(32768) by netlink_recvmsg()/netlink_dump(),
> so the top of the range the cap admits installs but cannot be dumped -
> and, before this change, fails silently.

[ ... ]

> This is a long-standing silent failure, not a regression, so it
> is net-next hardening.

[Severity: Low]
Would a Fixes: tag for the old silent truncation path in tcf_dump_walker()
be useful here, even though this goes to net-next?

The commit message ties the trigger to the gate entry cap in this series.
The same truncation can also be hit without the series. act_bpf_policy in
net/sched/act_bpf.c allows:

	[TCA_ACT_BPF_OPS]	= { .type = NLA_BINARY,
				    .len = sizeof(struct sock_filter) * BPF_MAXINSNS },

That is 32768 bytes of ops, which is more than SKB_WITH_OVERHEAD(32768).
So a single classic bpf action can already be too large for the dump skb.

Leaving out Fixes: may be on purpose, because the patch turns a silent
success into an error and is kept out of stable. If so, could the commit
message say that the problem is not limited to the gate series?

> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index fd7f60e0f62ba..33dba34364541 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c

[ ... ]

> @@ -626,10 +621,21 @@ 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;
> +
> +	/* A positive count means the skb filled up before this action was
> +	 * complete, so netlink retries it in a fresh skb. An action that
> +	 * does not fit even in an empty dump skb (nothing dumped) must be
> +	 * reported as -EMSGSIZE by netlink instead of ending the dump.
> +	 */
> +	return n_i ?: err;

[Severity: Low]
Is this comment right for every path that reaches this return in
tcf_dump_walker()?

A positive n_i also comes back when the idr walk finishes after dumping
every action. It also comes back when the loop stops here:

		if (!(act_flags & TCA_ACT_FLAG_LARGE_DUMP_ON) &&
		    n_i >= TCA_ACT_MAX_PRIO)
			goto done;

The skb did not fill up in either case. After a normal finish there is
also nothing left to retry.

The second sentence says -EMSGSIZE is reported "instead of ending the
dump". In netlink_dump() in net/netlink/af_netlink.c, a negative return
with an empty skb fails this check:

	if (nlk->dump_done_errno > 0 ||
	    skb_tailroom(skb) < nlmsg_total_size(sizeof(nlk->dump_done_errno))) {

It then goes on to netlink_dump_done(), which sends NLMSG_DONE with the
error and extack, and the dump ends. So the dump still ends, but now with
an error instead of silently.

Could the comment be reworded to match the return contract that
tc_dump_action() now relies on?

[ ... ]

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

Re: [PATCH net-next v3 5/5] net/sched: act_api: report an oversized action dump instead of silently truncating it

[Jamal Hadi Salim]

On Wed, Oct 14, 2026 at 5:15 AM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
>
> Low priority / nit picks for your consideration:
>
> - [Low] The patch has no Fixes: tag for the old silent-truncation path in
>   tcf_dump_walker() (`index--; nlmsg_trim; goto done; return n_i`).
> - [Low] The new comment above `return n_i ?: err;` in tcf_dump_walker()
>   (net/sched/act_api.c:625-630) does not match the code or af_netlink's…
>

Since all feedback on patches 1-5 are nitpicks on commit log or
comment I will not be making any changes unless something substantial
shows up.

cheers,
jamal
> --
>
> net/sched: act_api: report an oversized action dump instead of silently truncating it
>
> When an action does not fit even into an empty dump skb, tcf_dump_walker()
> now returns -EMSGSIZE with an extack instead of 0. tc_dump_action() now
> passes the walker's error up instead of returning skb->len. Before this
> change, "tc actions ls" silently left out that action and every action
> after it.
>
> Closes: https://lore.kernel.org/netdev/vPFrhbkwAui8zFPcGQTUDazQfsFDlr5GYuaYQ5BxJcrDiT6x1EljEze4cYadHqbeUW2ivLdpa8eC_cXC6ceACswgb-lW47aSVkHvHjhn_X8=@1g4.org/
>
> > That case is reachable inside the gate entry cap this series adds: a gate
> > entry serializes to 36 bytes (40 with the gate-open flag) while a dump skb
> > is clamped to SKB_WITH_OVERHEAD(32768) by netlink_recvmsg()/netlink_dump(),
> > so the top of the range the cap admits installs but cannot be dumped -
> > and, before this change, fails silently.
>
> [ ... ]
>
> > This is a long-standing silent failure, not a regression, so it
> > is net-next hardening.
>
> [Severity: Low]
> Would a Fixes: tag for the old silent truncation path in tcf_dump_walker()
> be useful here, even though this goes to net-next?
>
> The commit message ties the trigger to the gate entry cap in this series.
> The same truncation can also be hit without the series. act_bpf_policy in
> net/sched/act_bpf.c allows:
>
>         [TCA_ACT_BPF_OPS]       = { .type = NLA_BINARY,
>                                     .len = sizeof(struct sock_filter) * BPF_MAXINSNS },
>
> That is 32768 bytes of ops, which is more than SKB_WITH_OVERHEAD(32768).
> So a single classic bpf action can already be too large for the dump skb.
>
> Leaving out Fixes: may be on purpose, because the patch turns a silent
> success into an error and is kept out of stable. If so, could the commit
> message say that the problem is not limited to the gate series?
>
> > diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> > index fd7f60e0f62ba..33dba34364541 100644
> > --- a/net/sched/act_api.c
> > +++ b/net/sched/act_api.c
>
> [ ... ]
>
> > @@ -626,10 +621,21 @@ 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;
> > +
> > +     /* A positive count means the skb filled up before this action was
> > +      * complete, so netlink retries it in a fresh skb. An action that
> > +      * does not fit even in an empty dump skb (nothing dumped) must be
> > +      * reported as -EMSGSIZE by netlink instead of ending the dump.
> > +      */
> > +     return n_i ?: err;
>
> [Severity: Low]
> Is this comment right for every path that reaches this return in
> tcf_dump_walker()?
>
> A positive n_i also comes back when the idr walk finishes after dumping
> every action. It also comes back when the loop stops here:
>
>                 if (!(act_flags & TCA_ACT_FLAG_LARGE_DUMP_ON) &&
>                     n_i >= TCA_ACT_MAX_PRIO)
>                         goto done;
>
> The skb did not fill up in either case. After a normal finish there is
> also nothing left to retry.
>
> The second sentence says -EMSGSIZE is reported "instead of ending the
> dump". In netlink_dump() in net/netlink/af_netlink.c, a negative return
> with an empty skb fails this check:
>
>         if (nlk->dump_done_errno > 0 ||
>             skb_tailroom(skb) < nlmsg_total_size(sizeof(nlk->dump_done_errno))) {
>
> It then goes on to netlink_dump_done(), which sends NLMSG_DONE with the
> error and extack, and the dump ends. So the dump still ends, but now with
> an error instead of silently.
>
> Could the comment be reworded to match the return contract that
> tc_dump_action() now relies on?
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-H19Z.v3.r0.20261012193813%40mojatatu.com