Re: [PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries
flat view
From: Paul Moses <hidden>
Date: 2026-10-11 18:56:20
Subsystem:
networking [general], tc subsystem, the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Jamal Hadi Salim, Jiri Pirko, Linus Torvalds
[Severity: Medium]
This sizes the cap against the u16 nla_len of the nest. Should it also be
sized against the largest dump skb?
netlink_recvmsg() limits the dump buffer size:
net/netlink/af_netlink.c:netlink_recvmsg() {
...
max_recvmsg_len = min_t(size_t, max_recvmsg_len,
SKB_WITH_OVERHEAD(32768));
...
}
For RTM_GETACTION dumps, netlink_dump() allocates at most about 32.4 KB.
dumping_entry() emits 36 or 40 bytes per entry. A gate with more than about
810-900 entries therefore cannot be dumped, and the new cap still accepts
it.
When this happens, tcf_dump_walker() trims the action and returns without
an error:
net/sched/act_api.c:tcf_dump_walker() {
...
if (err < 0) {
index--;
nlmsg_trim(skb, nest);
goto done;
}
...
}
If the gate is the first action in the skb, n_i is 0. tc_dump_action()
then returns a skb->len of 0, and netlink_dump() sends NLMSG_DONE with
success. If earlier actions fit, the next callback restarts at the gate's
index and ends the same way.
The result is that the gate and every later action in the IDR are missing
from "tc actions ls", and userspace gets no error.
The commit message lists this truncation first under "The bugs:". The
Caveat Emptor section then says the cap does not prevent it.
Should the cap be small enough that a single gate fits in a dump skb? If
not, should the commit message stop listing this truncation as something
the cap fixes?This is the one I encountered, not that the cap should be smaller or that this type of dump needs to work, it's confusing that it returns 0 and stops. This needs more review with regard to other callers but to give you an idea.
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index e45a63be397c..4dfc050c48c4 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c@@ -589,18 +589,13 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb, tcf_action_update_hw_stats(p); nest = nla_nest_start_noflag(skb, n_i); - if (!nest) { - index--; + if (!nest) goto nla_put_failure; - } err = (act_flags & TCA_ACT_FLAG_TERSE_DUMP) ? tcf_action_dump_terse(skb, p, true) : tcf_action_dump_1(skb, p, 0, 0); - if (err < 0) { - index--; - nlmsg_trim(skb, nest); - goto done; - } + if (err < 0) + goto nla_put_failure; nla_nest_end(skb, nest); n_i++; if (!(act_flags & TCA_ACT_FLAG_LARGE_DUMP_ON) &&
@@ -616,10 +611,15 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb, if (act_flags & TCA_ACT_FLAG_LARGE_DUMP_ON) cb->args[1] = n_i; } - return n_i; + return n_i ?: err; nla_put_failure: + index--; nla_nest_cancel(skb, nest); + err = -EMSGSIZE; + if (!n_i) + NL_SET_ERR_MSG(cb->extack, + "TC action is too large for the dump buffer"); goto done; }
@@ -2260,6 +2260,7 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb) msecs_since = nla_get_u32(tb[TCA_ROOT_TIME_DELTA]); } + ret = -EMSGSIZE; nlh = nlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq, cb->nlh->nlmsg_type, sizeof(*t), 0); if (!nlh)
@@ -2303,7 +2304,7 @@ static int tc_dump_action(struct sk_buff *skb, struct netlink_callback *cb) out_module_put: module_put(a_o->owner); nlmsg_trim(skb, b); - return skb->len; + return ret; } static const struct rtnl_msg_handler tc_action_rtnl_msg_handlers[] __initconst = {