From f09db8b83e836f9554ec8e0f985c70699b3cb83f Mon Sep 17 00:00:00 2001 From: Victor Nogueira Date: Sat, 19 Sep 2026 20:04:27 -0300 Subject: [PATCH 1/3] net/sched: act_mirred: account for TCA_MIRRED_BLOCKID in get_fill_size tcf_mirred_get_fill_size budgets only TCA_MIRRED_PARMS, but tcf_mirred_dump also emits TCA_MIRRED_BLOCKID whenever the action was created with a block instead of a device, so the budget falls 8 bytes short of what the dump may produce for such actions. Account for TCA_MIRRED_BLOCKID unconditionally: reading tcfm_blockid to size it conditionally would race a concurrent replace, which can flip the action between device and block between sizing and dumping. Reported-by: Sashiko Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824153903.4143642-1-victor@mojatatu.com Co-developed-by: Jamal Hadi Salim Signed-off-by: Jamal Hadi Salim Signed-off-by: Victor Nogueira Link: https://patch.msgid.link/QDISC-RYTB.v1.20260918093205@mojatatu.com.1 Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/sched/act_mirred.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index 553342c55cf7..1c98e4d81ebe 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -607,7 +607,8 @@ tcf_mirred_get_dev(const struct tc_action *a, static size_t tcf_mirred_get_fill_size(const struct tc_action *act) { - return nla_total_size(sizeof(struct tc_mirred)); + return nla_total_size(sizeof(struct tc_mirred)) /* TCA_MIRRED_PARMS */ + + nla_total_size(sizeof(u32)); /* TCA_MIRRED_BLOCKID */ } static void tcf_offload_mirred_get_dev(struct flow_action_entry *entry, From 4ba61bfd4998074322cf2f34e6338151a1456ef6 Mon Sep 17 00:00:00 2001 From: Victor Nogueira Date: Sat, 19 Sep 2026 20:04:28 -0300 Subject: [PATCH 2/3] net/sched: add get_fill_size() to the five actions that lack one act_simple, act_mpls, act_nat, act_skbmod and act_connmark each have a tcf_*_dump but no .get_fill_size callback, so tcf_action_fill_size budgets only the shared attributes and omits the per-action TCA_*_PARMS and option bytes the dump emits. Add a callback to each, sized from the attributes its dump emits. Size every optional attribute unconditionally: reading the current flags to size it conditionally would race a concurrent replace, which can flip the attributes between sizing and dumping. Reported-by: Sashiko Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824153903.4143642-1-victor@mojatatu.com Co-developed-by: Jamal Hadi Salim Signed-off-by: Jamal Hadi Salim Signed-off-by: Victor Nogueira Link: https://patch.msgid.link/QDISC-RYTB.v1.20260918093205@mojatatu.com.2 Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/sched/act_connmark.c | 6 ++++++ net/sched/act_mpls.c | 11 +++++++++++ net/sched/act_nat.c | 6 ++++++ net/sched/act_simple.c | 7 +++++++ net/sched/act_skbmod.c | 9 +++++++++ 5 files changed, 39 insertions(+) diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c index c634af5edfcd..0b6ac6be60d2 100644 --- a/net/sched/act_connmark.c +++ b/net/sched/act_connmark.c @@ -236,12 +236,18 @@ static void tcf_connmark_cleanup(struct tc_action *a) kfree_rcu(parms, rcu); } +static size_t tcf_connmark_get_fill_size(const struct tc_action *act) +{ + return nla_total_size(sizeof(struct tc_connmark)); /* TCA_CONNMARK_PARMS */ +} + static struct tc_action_ops act_connmark_ops = { .kind = "connmark", .id = TCA_ID_CONNMARK, .owner = THIS_MODULE, .act = tcf_connmark_act, .dump = tcf_connmark_dump, + .get_fill_size = tcf_connmark_get_fill_size, .init = tcf_connmark_init, .cleanup = tcf_connmark_cleanup, .size = sizeof(struct tcf_connmark_info), diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c index 4ea8b2e08c3a..aa56521b7179 100644 --- a/net/sched/act_mpls.c +++ b/net/sched/act_mpls.c @@ -438,12 +438,23 @@ static int tcf_mpls_offload_act_setup(struct tc_action *act, void *entry_data, return 0; } +static size_t tcf_mpls_get_fill_size(const struct tc_action *act) +{ + return nla_total_size(sizeof(struct tc_mpls)) /* TCA_MPLS_PARMS */ + + nla_total_size(sizeof(u32)) /* TCA_MPLS_LABEL */ + + nla_total_size(sizeof(u8)) /* TCA_MPLS_TC */ + + nla_total_size(sizeof(u8)) /* TCA_MPLS_TTL */ + + nla_total_size(sizeof(u8)) /* TCA_MPLS_BOS */ + + nla_total_size(sizeof(u16)); /* TCA_MPLS_PROTO */ +} + static struct tc_action_ops act_mpls_ops = { .kind = "mpls", .id = TCA_ID_MPLS, .owner = THIS_MODULE, .act = tcf_mpls_act, .dump = tcf_mpls_dump, + .get_fill_size = tcf_mpls_get_fill_size, .init = tcf_mpls_init, .cleanup = tcf_mpls_cleanup, .offload_act_setup = tcf_mpls_offload_act_setup, diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c index abb332dee836..a201979ac7cf 100644 --- a/net/sched/act_nat.c +++ b/net/sched/act_nat.c @@ -313,12 +313,18 @@ static void tcf_nat_cleanup(struct tc_action *a) kfree_rcu(parms, rcu); } +static size_t tcf_nat_get_fill_size(const struct tc_action *act) +{ + return nla_total_size(sizeof(struct tc_nat)); /* TCA_NAT_PARMS */ +} + static struct tc_action_ops act_nat_ops = { .kind = "nat", .id = TCA_ID_NAT, .owner = THIS_MODULE, .act = tcf_nat_act, .dump = tcf_nat_dump, + .get_fill_size = tcf_nat_get_fill_size, .init = tcf_nat_init, .cleanup = tcf_nat_cleanup, .size = sizeof(struct tcf_nat), diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c index 8e69a919b4fe..80e5084b4dbd 100644 --- a/net/sched/act_simple.c +++ b/net/sched/act_simple.c @@ -79,6 +79,12 @@ static int reset_policy(struct tc_action *a, const struct nlattr *defdata, return 0; } +static size_t tcf_simp_get_fill_size(const struct tc_action *act) +{ + return nla_total_size(sizeof(struct tc_defact)) /* TCA_DEF_PARMS */ + + nla_total_size(SIMP_MAX_DATA); /* TCA_DEF_DATA */ +} + static const struct nla_policy simple_policy[TCA_DEF_MAX + 1] = { [TCA_DEF_PARMS] = { .len = sizeof(struct tc_defact) }, [TCA_DEF_DATA] = { .type = NLA_STRING, .len = SIMP_MAX_DATA }, @@ -204,6 +210,7 @@ static struct tc_action_ops act_simp_ops = { .owner = THIS_MODULE, .act = tcf_simp_act, .dump = tcf_simp_dump, + .get_fill_size = tcf_simp_get_fill_size, .cleanup = tcf_simp_release, .init = tcf_simp_init, .size = sizeof(struct tcf_defact), diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c index 7579cf1e0ff3..09e9dcf58253 100644 --- a/net/sched/act_skbmod.c +++ b/net/sched/act_skbmod.c @@ -279,12 +279,21 @@ nla_put_failure: return -1; } +static size_t tcf_skbmod_get_fill_size(const struct tc_action *act) +{ + return nla_total_size(sizeof(struct tc_skbmod)) /* TCA_SKBMOD_PARMS */ + + nla_total_size(ETH_ALEN) /* TCA_SKBMOD_DMAC */ + + nla_total_size(ETH_ALEN) /* TCA_SKBMOD_SMAC */ + + nla_total_size(sizeof(u16)); /* TCA_SKBMOD_ETYPE */ +} + static struct tc_action_ops act_skbmod_ops = { .kind = "skbmod", .id = TCA_ACT_SKBMOD, .owner = THIS_MODULE, .act = tcf_skbmod_act, .dump = tcf_skbmod_dump, + .get_fill_size = tcf_skbmod_get_fill_size, .init = tcf_skbmod_init, .cleanup = tcf_skbmod_cleanup, .size = sizeof(struct tcf_skbmod), From 54f5a4edf60966a7f7c13cf6ef8a036e95b25007 Mon Sep 17 00:00:00 2001 From: Victor Nogueira Date: Sat, 19 Sep 2026 20:04:29 -0300 Subject: [PATCH 3/3] net/sched: act_api: budget TCA_ROOT_EXT_WARN_MSG in notify skbs tca_get_fill may emit TCA_ROOT_EXT_WARN_MSG from extack->_msg. The string is whatever NL_SET_ERR_MSG and friends stored, so its length is bounded only by the caller, and nothing in the budget that tcf_add_notify_msg and tcf_del_notify_msg hand to alloc_skb accounts for it. The notify skb can therefore be sized smaller than what tca_get_fill goes on to write into it. The attribute reaches a successful add because tcf_action_init keeps going when an action that is not skip_sw fails to offload: err = tcf_action_offload_add(act, extack); if (tc_act_skip_sw(act->tcfa_flags) && err) goto err; The action is created while extack->_msg still holds the offload diagnostic, and tcf_add_notify echoes it back. A pedit action with mixed key commands (one SET and one ADD) takes that path: the non-bind tcf_pedit_offload_act_setup sets "Unsupported pedit command offload" and returns -EOPNOTSUPP. In practice, no underbudgeting has been observed because of this, and the gap is not easy to reach given some other spots account for more than necessary. However, for correctness, budget the attribute so the size handed to alloc_skb covers what tca_get_fill can write. Reported-by: Sashiko Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824153903.4143642-1-victor@mojatatu.com Co-developed-by: Jamal Hadi Salim Signed-off-by: Jamal Hadi Salim Signed-off-by: Victor Nogueira Link: https://patch.msgid.link/QDISC-RYTB.v1.20260918093205@mojatatu.com.3 Reviewed-by: Simon Horman Signed-off-by: Paolo Abeni --- net/sched/act_api.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/net/sched/act_api.c b/net/sched/act_api.c index 3f653721c45f..db06ddcf6ae6 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -475,6 +475,15 @@ static size_t tcf_action_full_attrs_size(size_t sz) + sz; } +/* tca_get_fill() may append TCA_ROOT_EXT_WARN_MSG from extack->_msg */ +static size_t tcf_action_warn_attr_size(const struct netlink_ext_ack *extack) +{ + if (unlikely(extack && extack->_msg)) + return nla_total_size(strlen(extack->_msg) + 1); + + return 0; +} + static size_t tcf_action_fill_size(const struct tc_action *act) { size_t sz = tcf_action_shared_attrs_size(act); @@ -1980,7 +1989,8 @@ static struct sk_buff *tcf_del_notify_msg(struct net *net, struct nlmsghdr *n, { struct sk_buff *skb; - skb = alloc_skb(max(attr_size, NLMSG_GOODSIZE), GFP_KERNEL); + skb = alloc_skb(max(attr_size + tcf_action_warn_attr_size(extack), + NLMSG_GOODSIZE), GFP_KERNEL); if (!skb) return ERR_PTR(-ENOBUFS); @@ -2078,7 +2088,8 @@ static struct sk_buff *tcf_add_notify_msg(struct net *net, struct nlmsghdr *n, { struct sk_buff *skb; - skb = alloc_skb(max(attr_size, NLMSG_GOODSIZE), GFP_KERNEL); + skb = alloc_skb(max(attr_size + tcf_action_warn_attr_size(extack), + NLMSG_GOODSIZE), GFP_KERNEL); if (!skb) return ERR_PTR(-ENOBUFS);