summaryrefslogtreecommitdiff
path: root/net/sched/act_api.c
AgeCommit message (Collapse)Author
6 daysnet/sched: act_api: inherit filter offload flags only on new actionsAlexandre Cassen
tcf_action_init() gives the skip_sw and skip_hw flags of a filter to the actions it binds. An action that another filter created gets them too, they then apply to every filter that uses this action. When a skip_sw filter reuses the action of a software filter, the software filter silently stops running it. This happens even when the filter add fails on a later action. A skip_hw filter likewise prevents a later filter without flags from reusing the action. Give the flags only to the actions that the filter creates. The mismatch check still rejects a filter that reuses an action with a flag that the filter does not have. Discovered by manual code inspection after the Sashiko AI review [1] of a tunnel_key patch questioned how actions get the filter flags. Reproduced on netdevsim without kernel changes. The egress filter below runs in software only, since netdevsim offloads only ingress filters. Once the skip_sw filter reuses its drop action, the last command shows skip_sw on this action and the egress filter stops dropping packets: ethtool -K eth0 hw-tc-offload on tc qdisc add dev eth0 clsact tc filter add dev eth0 egress protocol ip flower dst_ip 192.0.2.1 \ action gact drop index 1 tc filter add dev eth0 ingress protocol ip flower skip_sw \ dst_ip 192.0.2.2 action gact drop index 1 tc actions get action gact index 1 Tested with the reproducer and the tc-testing action, matchall and flower tests. Fixes: 34d35b4edbbe ("net/sched: act_api: deny mismatched skip_sw/skip_hw flags for actions created by classifiers") Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009051930.3619697-1-acassen%40corp.free.fr [1] Signed-off-by: Alexandre Cassen <acassen@corp.free.fr> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/20261021193222.1318333-2-acassen@corp.free.fr Signed-off-by: Jakub Kicinski <kuba@kernel.org>
11 daysMerge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/netPaolo Abeni
Merge in late fixes in preparation for the net-next PR. Conflicts: net/netfilter/nf_conntrack_netlink.c cd37ab22a74e0 ("netfilter: nf_conntrack_netlink: fix inverted IPv6 tuple filter") 1ae2d094b0200 ("netfilter: ctnetlink: fix inverted IPv6 address match in dump filter") Adjacent changes: drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c 8830e65ed46de ("net: use assign_bit() where applicable") 07bd391eb957 ("octeontx2-af: Fix CQ enable state tracking after AQ writes") net/core/sock.c 31c5ea8a3296 ("ipv6: Remove IPV6_ADDRFORM.") 31b564f70d88 ("net: use the full socket in sk_mc_loop()") net/mptcp/protocol.h 0cb6910130a3 ("mptcp: remove thmac from subflow ctx") 7538fa33bb2f ("mptcp: fix subflow bitfield misuse") Signed-off-by: Paolo Abeni <pabeni@redhat.com>
12 daysnet/sched: act_api: report an oversized action dump instead of silently ↵Jamal Hadi Salim
truncating it 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> Link: https://patch.msgid.link/QDISC-H19Z.v3.r0.20261012193813-5@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
12 daysnet/sched: act_api: keep an add that created its actions from failingJamal 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> Link: https://patch.msgid.link/QDISC-H19Z.v3.r0.20261012193813-2@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
12 daysnet/sched: act_gate: reject oversized dumps instead of wrapping themJamal 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> Link: https://patch.msgid.link/QDISC-H19Z.v3.r0.20261012193813-1@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
13 daysnet/sched: act_api: reject duplicate actions in a batchJamal Hadi Salim
tca_action_gd() builds actions[] with one tcf_action_get_1() per nested TCA_ACT_TAB entry. Each successful lookup takes its own reference to the resolved action, so repeated TCA_ACT_INDEX entries in one request yield the same pointer in multiple slots. For RTM_DELACTION, tcf_action_delete() then consumes two references per slot: one in tcf_action_put() and one in tcf_idr_delete_index(). A duplicate therefore drives the refcount to zero mid-walk -- the delete frees the action and removes its IDR slot -- and the next slot calls tcf_action_put() on the freed action: refcount_t: underflow; use-after-free. WARNING: lib/refcount.c:87 at refcount_dec_not_one refcount_dec_and_mutex_lock __tcf_action_put tca_action_gd Three duplicate entries are enough. Reject a repeated action while a delete batch is built, dropping the reference the extra lookup took, and return -EINVAL. RTM_GETACTION balances its own references and keeps accepting duplicate entries. Conditions to recreate the bug: tc actions add action gact index 100 tc actions delete action gact index 100 action gact index 100 \ action gact index 100 Fixes: 16af6067392c ("net: sched: implement reference counted action release") Reported-by: Sashiko (gemini) <sashiko-bot@kernel.org> Link: https://sashiko.dev/#/patchset/20260824153903.4143642-1-victor@mojatatu.com Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://patch.msgid.link/QDISC-K1TH.v1.r3.20261012134653-1@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-10-15net/sched: act_api: take tcfa_lock for hardware stats updatesAlexandre Cassen
Actions without per CPU stats update their counters under tcfa_lock in the software path but tcf_action_update_stats() adds hardware stats to the same counters without it. A dump of a filter without skip_hw does this even when nothing is offloaded, so such an action loses counter updates. On 32-bit, readers holding tcfa_lock can also observe torn 64-bit values. Callers disable preemption around this update for the per CPU stats. PREEMPT_RT forbids taking a spinlock_t with preemption disabled. Disable BH in the per CPU path instead. On 32-bit, this also keeps a softirq on the same CPU from updating or reading these counters in the middle of the update. Discovered by manual code inspection while letting more actions honor TCA_ACT_FLAGS_NO_PERCPU_STATS. Reproduced in a VM without kernel changes, where the second action counts fewer packets than the first: ip link add d0 type dummy && ip link set d0 up tc qdisc add dev d0 clsact tc filter add dev d0 ingress matchall \ action gact pipe action gact pass no_percpu yes "filter show dev d0 ingress" | head -10000 > batch while :; do tc -s -b batch; done > /dev/null & samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh -i d0 -t 4 tc -s filter show dev d0 ingress The 32-bit issue in the per CPU path comes from the Sashiko AI review of v1 [1] and was not reproduced. Tested with the reproducer and the tc-testing action, matchall and flower tests with lockdep enabled. Also tested for regressions on ConnectX-7 with firmware 28.48.1000. Cc: stable+noautosel@kernel.org # high regression risk, mixed-use actions are not commonly used in practice Fixes: 5e174d5e73df ("net: sched: modify stats helper functions to support regular stats") Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261009051703.3615846-1-acassen%40corp.free.fr [1] Signed-off-by: Alexandre Cassen <acassen@corp.free.fr> Reviewed-by: Eric Dumazet <edumazet@kernel.org> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://patch.msgid.link/20261010152227.401987-1-acassen@corp.free.fr Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-10-15Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/netJakub Kicinski
Cross-merge networking fixes after downstream PR (net-7.3-rc8). No conflicts. Adjacent changes: drivers/net/ethernet/microchip/lan865x/lan865x.c 2a3f6d2a9ac3 ("microchip: lan865x: move setting of netops to fix crash") 15358c2c3dca ("net: ethernet: oa_tc6: return ERR_PTR from oa_tc6_init()") Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-10-15net/sched: fix use-after-free in __tcf_action_put()Jérémy Jean
__tcf_action_put() decrements the reference count before decrementing the binding count. Another thread can free the action between these operations, causing the binding-count update to access freed memory. KASAN reports it as: BUG: KASAN: slab-use-after-free in __tcf_action_put+0x327/0x340 Write of size 4 at addr ff11000000d46420 by task tc-put-pause/64 Hold the action IDR mutex across both counter updates and IDR removal, keeping cleanup outside the lock. Fixes: 16af6067392c ("net: sched: implement reference counted action release") Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/20261008181348.527035-2-Jeremy.Jean@oss.cyber.gouv.fr Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2026-09-24Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/netJakub Kicinski
Cross-merge networking fixes after downstream PR (net-7.3-rc5). Conflicts: drivers/net/mdio/mdio-realtek-rtl9300.c 89a8a1eef2d44 ("net: mdio: realtek-rtl9300: fix RTL931x C22 extended page selection") cc3cb8db1eef9 ("net: mdio: realtek-rtl9300: Add page tracking") https://lore.kernel.org/arUZOqy73bE2pp0w@sirena.org.uk net/8021q/vlan_dev.c cd5dd68267c4 ("vlan: ensure sufficient headroom in vlan_dev_hard_header()") ca6ff8dd70eb ("vlan: annotate data-races in vlan_dev_priv fields") Adjacent changes: net/ipv6/ip6_gre.c dd47bcf279f1 ("ip6_gre: Call ip6erspan_tunnel_unlink_md() in ip6erspan_changelink().") cce829e2aa1d ("ip6_gre: Protect ip6gre_net.tunnels[][] with mutex.") drivers/net/ethernet/meta/fbnic/fbnic_txrx.c b5d9e9d4d0c1 ("eth: fbnic: use the Rx queue napi pointer to find the napi vector") c0aca269ec07 ("eth: fbnic: Make Rx completion coalescing configurable") drivers/net/ethernet/stmicro/stmmac/stmmac_selftests.c d68acbf93531 ("net: stmmac: selftests: Support running selftests on DSA conduits") 85ca3292d7a3 ("net: stmmac: Remove ARP offload code") drivers/net/ethernet/wangxun/libwx/wx_hw.c 3173cba11701 ("net: libwx: fix races in Tx timestamp handling") 7042c8c193e5 ("net: libwx: rename wx_pf_flags to wx_flags") Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-09-22net/sched: act_api: budget TCA_ROOT_EXT_WARN_MSG in notify skbsVictor Nogueira
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 <sashiko-bot@kernel.org> Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260824153903.4143642-1-victor@mojatatu.com Co-developed-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Victor Nogueira <victor@mojatatu.com> Link: https://patch.msgid.link/QDISC-RYTB.v1.20260918093205@mojatatu.com.3 Reviewed-by: Simon Horman <horms@kernel.org> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2026-09-19net/sched: reject IDR error pointers when deleting actionsWeiming Shi
tcf_action_delete() drops the reference held by its lookup before calling tcf_idr_delete_index() with the saved action index. An unlocked classifier can remove that action and reserve the same IDR slot with ERR_PTR(-EBUSY) in between. tcf_idr_delete_index() only checks the lookup result for NULL. It therefore treats the reservation as a tc_action and dereferences tcfa_bindcnt. A hardware execution breakpoint was used to schedule the interleaving without changing the kernel source. KASAN reported this decoded trace: BUG: KASAN: null-ptr-deref in tca_action_gd+0x5b9/0x1010 Read of size 4 at addr 0000000000000010 by task poc/150 Oops: general protection fault, probably for non-canonical address 0xdffffc0000000002 RIP: tca_action_gd+0x5c0/0x1010: arch_atomic_read at arch/x86/include/asm/atomic.h:23 raw_atomic_read at include/linux/atomic/atomic-arch-fallback.h:457 atomic_read at include/linux/atomic/atomic-instrumented.h:33 tcf_idr_delete_index at net/sched/act_api.c:766 tcf_action_delete at net/sched/act_api.c:1859 tcf_del_notify at net/sched/act_api.c:2014 tca_action_gd at net/sched/act_api.c:2064 R13: 0000000000000010 R15: fffffffffffffff0 Kernel panic - not syncing: Fatal exception R15 contains ERR_PTR(-EBUSY), and adding the tcfa_bindcnt offset produces the address in R13. With the guard applied, the same reproducer returned -ENOENT without a KASAN report or panic. Treat error pointers as absent and return -ENOENT. Fixes: 0190c1d452a9 ("net: sched: atomically check-allocate action") Cc: stable@vger.kernel.org Reported-by: Xiang Mei <xmei5@asu.edu> Signed-off-by: Weiming Shi <bestswngs@gmail.com> Link: https://patch.msgid.link/20260914065123.4109709-2-bestswngs@gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-09-15net/sched: act_api: release tail references on DELACTION failureXuanqiang Luo
A batched RTM_DELACTION request takes a temporary reference on each action before attempting any deletion. tcf_action_delete() clears each processed slot and drops its temporary reference before attempting the deletion. If deletion fails, tca_action_gd() calls tcf_action_put_many() to release the remaining references, but its tcf_act_for_each_action() iterator stops at the first NULL slot. When a batch stops at an action bound to a filter, this leaks a reference on each subsequent action. A later delete of an unbound action can then return success without removing it from the IDR. Walk the full array in tcf_action_put_many() and skip NULL slots to release the references held on the unprocessed actions. Fixes: a0e947c9ccff ("net/sched: act_api: avoid non-contiguous action array") Cc: stable@vger.kernel.org Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn> Link: https://patch.msgid.link/20260910093413.34509-2-xuanqiang.luo@linux.dev Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-09-10net/sched: act_api: release all action references on NEWACTION failureXuanqiang Luo
When a batched RTM_NEWACTION request replaces an existing action, tcf_idr_check_alloc() takes a temporary reference on it. If a later action fails to initialize, tcf_action_destroy() uses strict release semantics to clean up the actions initialized so far. For an action bound to a filter, the strict check returns -EPERM without dropping the temporary reference. This error also makes tcf_action_destroy() return before releasing subsequent entries. Any new action initialized between the bound action and the failing entry is leaked together with its reserved IDR slot, preventing reuse of its index. Use tcf_idr_release() to drop each reference held by the batch without rejecting bound actions. This allows cleanup to continue through all initialized entries and preserves the module reference release when an action is destroyed. Explicit action deletion and flushing retain their separate bind-count checks. Fixes: 55334a5db5cd ("net_sched: act: refuse to remove bound action outside") Cc: stable@vger.kernel.org Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://patch.msgid.link/20260909070336.32979-2-xuanqiang.luo@linux.dev Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-08-28net/sched: act_api: fix skb sizing and action leak on reoffload deleteVictor Nogueira
tcf_reoffload_del_notify_msg() sizes the RTM_DELACTION skb with tcf_action_fill_size(action) alone. Unlike every other notification path it never wraps that in tcf_action_full_attrs_size(), so the nlmsg_put() header, struct tcamsg and the TCA_ACT_TAB nest that tca_get_fill() emits - 24 bytes on x86_64 - are not budgeted. As long as the single action stays well under NLMSG_GOODSIZE the floor in alloc_skb() hides this, but once its fill size crosses NLMSG_GOODSIZE the allocation is exactly 24 bytes short and tca_get_fill() runs out of tailroom. That is now easy to reach for an offloadable act_pedit with a large tcfp_nkeys, which commit 8e2efb3f45a5 ("net/sched: add get_fill_size callbacks for actions missing them") started accounting for properly. When that happens tcf_reoffload_del_notify() returns early, before tcf_idr_release_unsafe(), and tcf_action_reoffload_cb() discards the return value: if (tc_act_skip_sw(p->tcfa_flags) && !tc_act_in_hw(p)) tcf_reoffload_del_notify(net, p); The action has just lost its last hardware instance and is skip_sw, so it is left installed while processing no packets, and with no notification to tell userspace about it. An -ENOBUFS from alloc_skb() gets the same treatment. Fix this by budgeting the message header the way the add and delete paths do, and release the action even when the notification cannot be built - dropping the notification is strictly better than leaking a dead action, and there is no caller left to report the error to. Fixes: 13926d19a11e ("flow_offload: add reoffload process to update hw_count") Reported-by: Sashiko <sashiko-bot@kernel.org> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260810164357.1653956-1-victor%40mojatatu.com Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Victor Nogueira <victor@mojatatu.com> Reviewed-by: Pedro Tammela <pctammela@mojatatu.com> Link: https://patch.msgid.link/20260824153903.4143642-4-victor@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-08-28net/sched: act_api: size the RTM_GETACTION reply from the actionsVictor Nogueira
tca_action_gd() already walks every requested action and accumulates attr_size += tcf_action_fill_size(act), then wraps the result in tcf_action_full_attrs_size(). For RTM_DELACTION that value is handed to tcf_del_notify_msg(), which allocates max(attr_size, NLMSG_GOODSIZE). For RTM_GETACTION it is silently discarded and tcf_get_notify() allocates a fixed NLMSG_GOODSIZE skb instead. Any action whose dump exceeds that fixed budget therefore cannot be read back. For example, act_pedit overruns the budget with 32 actions of four munge keys each, act_police with 32 policers once the optional rate/peakrate/result/avrate attributes are present Fix this by passing attr_size through and allocate the reply the way the add and delete paths do. Note on exposure: RTM_GETACTION is the only one of the three action commands that is not capability checked - tc_ctl_action() requires CAP_NET_ADMIN for RTM_NEWACTION and RTM_DELACTION only - so this turns a fixed NLMSG_GOODSIZE reply into a user sized allocation on an unprivileged path. It is bounded by TCA_ACT_MAX_PRIO actions per request, and tca_action_gd() does not reject duplicate indices, so a single large action can be requested 32 times; an act_bpf program near BPF_MAXINSNS is about 32KB of dump, or roughly 1MB for one request. Creating such an action still requires CAP_NET_ADMIN, and the add and delete paths have sized their skbs this way since the Fixes commit. Should this ever need bounding, GFP_KERNEL_ACCOUNT would charge the reply to the caller's memcg. Fixes: 4e76e75d6aba ("net sched actions: calculate add/delete event message size") Reported-by: Sashiko <sashiko-bot@kernel.org> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260810164357.1653956-1-victor%40mojatatu.com Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Victor Nogueira <victor@mojatatu.com> Link: https://patch.msgid.link/20260824153903.4143642-3-victor@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-08-28net/sched: act_api: budget all shared attributes in notify skbsVictor Nogueira
tcf_action_shared_attrs_size() is supposed to return an upper bound on the netlink attributes every action dump emits outside of TCA_ACT_OPTIONS, so that tcf_add_notify_msg(), tcf_del_notify_msg() and friends can allocate an skb large enough for the reply. It has fallen behind the dump path and is now an underestimate for every single action. Attributes, such as, TCA_ACT_IN_HW_COUNT and TCA_STATS_BASIC_HW are emitted unconditionally and never accounted for. TCA_STATS_PKT64, TCA_ACT_USED_HW_STATS, TCA_STATS_RATE_EST, TCA_STATS_RATE_EST64 require specific conditions, but are also not accounted for. Fix the issue by budgeting all of them so that we have a legitimate upper bound. Even tough for of them require specific conditions, they are cheap so, to avoid overcomplicating, we opted to account for them unconditionally as well to account for a real worst case scenario. Fixes: 4e76e75d6aba ("net sched actions: calculate add/delete event message size") Reported-by: Sashiko <sashiko-bot@kernel.org> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260810164357.1653956-1-victor%40mojatatu.com Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Victor Nogueira <victor@mojatatu.com> Link: https://patch.msgid.link/20260824153903.4143642-2-victor@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-08-20net/sched: add get_fill_size callbacks for actions missing themVictor Nogueira
Several tc actions - act_police, act_bpf, act_pedit, act_ife, act_sample, act_ct, act_ctinfo and act_tunnel_key among them - provide no get_fill_size() callback, so tcf_action_fill_size() falls back to tcf_action_shared_attrs_size() which does not account for the action-specific netlink attributes emitted inside TCA_ACT_OPTIONS by their dump functions. When an RTM_NEWACTION request with NLM_F_ECHO (or an RTNLGRP_TC listener) creates several actions, tcf_add_notify_msg() allocates the echo skb from this underestimated size. When this happens, the act_api code fails to add all of the fields to the netlink message and, thus, fails to send it. Issue is that, when that happens, this failure doesn't stop the action instances from being added. So any user watching these events will be under the false impression that no actions were created at all. For example, act_pedit overruns with 32 actions of four munge keys each, act_police with 32 policers once the optional rate/peakrate/result/avrate attributes are present. To fix this, add the missing get_fill_size callbacks returning the worst-case size of each action's dump attributes, following the pattern used by act_gact/act_skbedit/act_vlan. Also widen the TCA_GACT_TM accounting in tcf_action_shared_attrs_size() to nla_total_size_64bit(), since actions dump their tcf_t with nla_put_64bit(), which may be preceded by an NLA_PAD attribute. Note: We only provided fixes for the actions we reproduced this bug with as of today. We can send a separate hardening patch for the remaining actions to net-next later. The other pre-existing issues, pointed out by Clashiko [1], will be fixed in upcoming patches. [1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260810164357.1653956-1-victor%40mojatatu.com Fixes: 4e76e75d6aba ("net sched actions: calculate add/delete event message size") Reported-by: Vega <vega@nebusec.ai> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Victor Nogueira <victor@mojatatu.com> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/20260816201327.2435335-1-victor@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-08-11net/sched: act_api: fix TOCTOU NULL deref on a->goto_chainJamal Hadi Salim
tcf_action_exec() handles TC_ACT_GOTO_CHAIN by first checking rcu_access_pointer(a->goto_chain) and then calling tcf_action_goto_chain_exec(), which does a second, independent rcu_dereference_bh(a->goto_chain) read and immediately dereferences chain->filter_chain. A concurrent tcf_action_set_ctrlact() (e.g. the gact replace path) can clear a->goto_chain between the two reads, so the second read returns NULL and tcf_action_goto_chain_exec() dereferences NULL. Fix the race by doing a single rcu_dereference_bh() read of a->goto_chain in tcf_action_exec(), checking it once for NULL, and passing the resulting chain pointer into tcf_action_goto_chain_exec(). This turns the split check/use into a single check/use on one value. Fixes: ee3bbfe806cd ("net/sched: let actions use RCU to access 'goto_chain'") Reported-by: vega@nebusec.ai Tested-by: Victor Nogueira <victor@mojatatu.com> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> Reviewed-by: Davide Caratti <dcaratti@redhat.com> Link: https://patch.msgid.link/20260809090928.868186-1-jhs@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-07-06net/sched: act_pedit: fix TOCTOU heap OOB write in tc offloadJamal Hadi Salim
There is a TOCTOU race condition in flower lockless approach between sizing a flow_rule buffer and filling it. zdi-disclosures@trendmicro.com reports: The cls_flower classifier operates with TCF_PROTO_OPS_DOIT_UNLOCKED (fl_change runs without RTNL), while RTM_NEWACTION holds RTNL, so the independent locking domains make the race reachable in practice. KASAN confirms: BUG: KASAN: slab-out-of-bounds in tcf_pedit_offload_act_setup+0x81b/0x930 Write of size 4 at addr ffff888001f27520 by task poc-toctou/312 The buggy address is located 0 bytes to the right of allocated 288-byte region [ffff888001f27400, ffff888001f27520) (cache kmalloc-512) Note: The result is a heap OOB write attacker-controlled content into the adjacent slab object (requires CAP_NET_ADMIN). The fix introduces reading tcfp_nkeys under act->tcfa_lock in all places using a new tcf_pedit_nkeys_locked() which replaces the old tcf_pedit_nkeys(). Additionally we close the remaining TOCTOU window between the sizing read and the fill reads by more careful accounting. Rather than silently truncating the key count, which leads to incorrect action semantics offloaded to hardware and secondary OOB writes if the remaining capacity is zero or consumed by prior actions, we enforce remaining capacity checks and return -ENOSPC if the required space exceeds the remaining capacity. Fixes: 71d0ed7079df ("net/act_pedit: Support using offset relative to the conventional network headers") Reported-by: zdi-disclosures@trendmicro.com Tested-by: Victor Nogueira <victor@mojatatu.com> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://patch.msgid.link/20260701161912.125355-1-jhs@mojatatu.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2026-06-04Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/netJakub Kicinski
Cross-merge networking fixes after downstream PR (net-7.1-rc7). Silent conflicts: net/wireless/nl80211.c cb9959ab5f99 ("wifi: cfg80211: enforce HE/EHT cap/oper consistency") a384ae969902 ("wifi: cfg80211: move AP HT/VHT/... operation to beacon info") https://lore.kernel.org/aiGJDaHV4UlCexIQ@sirena.org.uk Conflicts: drivers/net/wireless/intel/iwlwifi/mld/ap.c a342c99cb70d ("wifi: iwlwifi: mld: honor BSS_CHANGED_BEACON_ENABLED") 9bf1b409afc7 ("wifi: iwlwifi: mld: send tx power constraints before link activation") https://lore.kernel.org/ah2bfedhV45ZxMO8@sirena.org.uk drivers/net/wireless/intel/iwlwifi/pcie/drv.c 093305d801fa ("wifi: iwlwifi: pcie: simplify the resume flow if fast resume is not used") e2323929a68a ("wifi: iwlwifi: pcie: add debug print for resume flow if powered off") https://lore.kernel.org/ah2bfedhV45ZxMO8@sirena.org.uk Adjacent changes: drivers/net/ethernet/airoha/airoha_eth.c b38cae85d1c4 ("net: airoha: Fix use-after-free in metadata dst teardown") ec6c391bcca7 ("net: airoha: Introduce airoha_gdm_dev struct") drivers/net/ethernet/microchip/lan743x_main.c 8173d22b211f ("net: lan743x: permit VLAN-tagged packets up to configured MTU") e3c6508a46f5 ("net: lan743x: avoid netdev-based logging before netdev registration") Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-06-01net/sched: act_api: use RCU with deferred freeing for action lifecycleJamal Hadi Salim
When NEWTFILTER and DELFILTER are run concurrently it is possible to create a race with an associated action. Let's illustrate with CPU0 running NEWTFILTER and CPU1 running DELFILTER: 0: mutex_lock() <-- holds the idr lock 0: rcu_read_lock() 0: p = idr_find(idr, index) <-- action p is valid (RCU protects IDR) 0: mutex_unlock() <-- releases the idr lock 1: refcount_dec_and_mutex_lock() <-- refcnt 1->0, mutex held 1: idr_remove(idr, index) <-- Action removed from IDR 1: mutex_unlock() <-- mutex released allowing us to delete the action 1: tcf_action_cleanup(p); kfree(p) <-- Kfrees p immediately, no deferral 0: refcount_inc_not_zero(&p->tcfa_refcnt) <-- ouch, UAF p points to freed memory This patch fixes the race condition between NEWTFILTER and DELFILTER by adding struct rcu_head to tc_action used in the deferral and introducing a call_rcu() in the delete path to defer the final kfree(). Note: this is a revert of commit d7fb60b9cafb ("net_sched: get rid of tcfa_rcu") but also modernization/simplification to directly use kfree_rcu(). Let's illustrate the new restored code path: 0: rcu_read_lock() 1: refcount_dec_and_mutex_lock() <-- refcnt 1->0, mutex held 1: idr_remove(idr, index) 1: mutex_unlock() 1: call_rcu(&p->tcfa_rcu, tcf_action_rcu_free) <-- defer kfree after grace period 0: p = idr_find(idr, index) 0: refcount_inc_not_zero(&p->tcfa_refcnt) <-- fails, refcnt already 0 1: rcu_read_unlock() <-- release so freeing can run after grace period After CPU1 calls idr_remove(), the object is no longer reachable through the IDR. CPU0's subsequent idr_find() will return NULL, and even if it still held a stale pointer, the immediate kfree() is now deferred until after the RCU grace period, so no UAF can occur. Fixes: d7fb60b9cafb ("net_sched: get rid of tcfa_rcu") Suggested-by: Jakub Kicinski <kuba@kernel.org> Reported-by: Kyle Zeng <kylebot@openai.com> Tested-by: Victor Nogueira <victor@mojatatu.com> Tested-by: syzbot@syzkaller.appspotmail.com Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> Tested-by: Kyle Zeng <kylebot@openai.com> Reviewed-by: Pedro Tammela <pctammela@mojatatu.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Reviewed-by: Victor Nogueira <victor@mojatatu.com> Link: https://patch.msgid.link/20260531160812.68020-1-jhs@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-05-04net/sched: add qstats_cpu_drop_inc() helperEric Dumazet
1) Using this_cpu_inc() is better than going through this_cpu_ptr(): - Single instruction on x86. - Store tearing prevention. 2) Change tcf_action_update_stats() to use this_cpu_add(). 3) Add WRITE_ONCE() to __qdisc_qstats_drop() and qstats_drop_inc() in preparation for lockless "tc qdisc show". $ scripts/bloat-o-meter -t vmlinux.old vmlinux.new add/remove: 0/0 grow/shrink: 3/17 up/down: 72/-216 (-144) Function old new delta dualpi2_enqueue_skb 462 511 +49 tcf_ife_act 1061 1077 +16 taprio_enqueue 613 620 +7 codel_qdisc_enqueue 149 143 -6 tcf_vlan_act 684 676 -8 tcf_skbedit_act 626 618 -8 tcf_police_act 725 717 -8 tcf_mpls_act 1297 1289 -8 tcf_gate_act 310 302 -8 tcf_gact_act 222 214 -8 tcf_csum_act 2438 2430 -8 tcf_bpf_act 709 701 -8 tcf_action_update_stats 124 115 -9 pie_qdisc_enqueue 865 856 -9 pfifo_enqueue 116 107 -9 choke_enqueue 2069 2059 -10 plug_enqueue 139 128 -11 bfifo_enqueue 121 110 -11 tcf_nat_act 1501 1489 -12 gred_enqueue 1743 1668 -75 Total: Before=24388609, After=24388465, chg -0.00% Signed-off-by: Eric Dumazet <edumazet@google.com> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://patch.msgid.link/20260501135916.2566766-1-edumazet@google.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2026-02-21Convert 'alloc_obj' family to use the new default GFP_KERNEL argumentLinus Torvalds
This was done entirely with mindless brute force, using git grep -l '\<k[vmz]*alloc_objs*(.*, GFP_KERNEL)' | xargs sed -i 's/\(alloc_objs*(.*\), GFP_KERNEL)/\1)/' to convert the new alloc_obj() users that had a simple GFP_KERNEL argument to just drop that argument. Note that due to the extreme simplicity of the scripting, any slightly more complex cases spread over multiple lines would not be triggered: they definitely exist, but this covers the vast bulk of the cases, and the resulting diff is also then easier to check automatically. For the same reason the 'flex' versions will be done as a separate conversion. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2026-02-21treewide: Replace kmalloc with kmalloc_obj for non-scalar typesKees Cook
This is the result of running the Coccinelle script from scripts/coccinelle/api/kmalloc_objs.cocci. The script is designed to avoid scalar types (which need careful case-by-case checking), and instead replace kmalloc-family calls that allocate struct or union object instances: Single allocations: kmalloc(sizeof(TYPE), ...) are replaced with: kmalloc_obj(TYPE, ...) Array allocations: kmalloc_array(COUNT, sizeof(TYPE), ...) are replaced with: kmalloc_objs(TYPE, COUNT, ...) Flex array allocations: kmalloc(struct_size(PTR, FAM, COUNT), ...) are replaced with: kmalloc_flex(*PTR, FAM, COUNT, ...) (where TYPE may also be *VAR) The resulting allocations no longer return "void *", instead returning "TYPE *". Signed-off-by: Kees Cook <kees@kernel.org>
2026-01-06net/sched: act_api: avoid dereferencing ERR_PTR in tcf_idrinfo_destroyShivani Gupta
syzbot reported a crash in tc_act_in_hw() during netns teardown where tcf_idrinfo_destroy() passed an ERR_PTR(-EBUSY) value as a tc_action pointer, leading to an invalid dereference. Guard against ERR_PTR entries when iterating the action IDR so teardown does not call tc_act_in_hw() on an error pointer. Fixes: 84a7d6797e6a ("net/sched: acp_api: no longer acquire RTNL in tc_action_net_exit()") Link: https://syzkaller.appspot.com/bug?extid=8f1c492ffa4644ff3826 Reported-by: syzbot+8f1c492ffa4644ff3826@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=8f1c492ffa4644ff3826 Signed-off-by: Shivani Gupta <shivani07g@gmail.com> Link: https://patch.msgid.link/20260105005905.243423-1-shivani07g@gmail.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2025-09-02net_sched: act: remove tcfa_qstatsEric Dumazet
tcfa_qstats is currently only used to hold drops and overlimits counters. tcf_action_inc_drop_qstats() and tcf_action_inc_overlimit_qstats() currently acquire a->tcfa_lock to increment these counters. Switch to two atomic_t to get lock-free accounting. Signed-off-by: Eric Dumazet <edumazet@google.com> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://patch.msgid.link/20250901093141.2093176-1-edumazet@google.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2025-07-08net/sched: acp_api: no longer acquire RTNL in tc_action_net_exit()Eric Dumazet
tc_action_net_exit() got an rtnl exclusion in commit a159d3c4b829 ("net_sched: acquire RTNL in tc_action_net_exit()") Since then, commit 16af6067392c ("net: sched: implement reference counted action release") made this RTNL exclusion obsolete for most cases. Only tcf_action_offload_del() might still require it. Move the rtnl locking into tcf_idrinfo_destroy() when an offload action is found. Most netns do not have actions, yet deleting them is adding a lot of pressure on RTNL, which is for many the most contended mutex in the kernel. We are moving to a per-netns 'rtnl', so tc_action_net_exit() will not be able to grab 'rtnl' a single time for a batch of netns. Before the patch: perf probe -a rtnl_lock perf record -e probe:rtnl_lock -a /bin/bash -c 'unshare -n "/bin/true"; sleep 1' [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.305 MB perf.data (25 samples) ] After the patch: perf record -e probe:rtnl_lock -a /bin/bash -c 'unshare -n "/bin/true"; sleep 1' [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.304 MB perf.data (9 samples) ] Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Vlad Buslov <vladbu@nvidia.com> Cc: Jiri Pirko <jiri@resnulli.us> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Link: https://patch.msgid.link/20250702071230.1892674-1-edumazet@google.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2025-04-15tc: Return an error if filters try to attach too many actionsToke Høiland-Jørgensen
While developing the fix for the buffer sizing issue in [0], I noticed that the kernel will happily accept a long list of actions for a filter, and then just silently truncate that list down to a maximum of 32 actions. That seems less than ideal, so this patch changes the action parsing to return an error message and refuse to create the filter in this case. This results in an error like: # ip link add type veth # tc qdisc replace dev veth0 root handle 1: fq_codel # tc -echo filter add dev veth0 parent 1: u32 match u32 0 0 $(for i in $(seq 33); do echo action pedit munge ip dport set 22; done) Error: Only 32 actions supported per filter. We have an error talking to the kernel Instead of just creating a filter with 32 actions and dropping the last one. This is obviously a change in UAPI. But seeing as creating more than 32 filters has never actually *worked*, it seems that returning an explicit error is better, and any use cases that get broken by this were already broken just in more subtle ways. [0] https://lore.kernel.org/r/20250407105542.16601-1-toke@redhat.com Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com> Link: https://patch.msgid.link/20250409145523.164506-1-toke@redhat.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2024-10-25Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/netPaolo Abeni
Cross-merge networking fixes after downstream PR. No conflicts and no adjacent changes. Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2024-10-23net/sched: act_api: unexport tcf_action_dump_1()Vladimir Oltean
This isn't used outside act_api.c, but is called by tcf_dump_walker() prior to its definition. So move it upwards and make it static. Simultaneously, reorder the variable declarations so that they follow the networking "reverse Christmas tree" coding style. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Simon Horman <horms@kernel.org> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> Link: https://patch.msgid.link/20241017161934.3599046-1-vladimir.oltean@nxp.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2024-10-23net/sched: act_api: deny mismatched skip_sw/skip_hw flags for actions ↵Vladimir Oltean
created by classifiers tcf_action_init() has logic for checking mismatches between action and filter offload flags (skip_sw/skip_hw). AFAIU, this is intended to run on the transition between the new tc_act_bind(flags) returning true (aka now gets bound to classifier) and tc_act_bind(act->tcfa_flags) returning false (aka action was not bound to classifier before). Otherwise, the check is skipped. For the case where an action is not standalone, but rather it was created by a classifier and is bound to it, tcf_action_init() skips the check entirely, and this means it allows mismatched flags to occur. Taking the matchall classifier code path as an example (with mirred as an action), the reason is the following: 1 | mall_change() 2 | -> mall_replace_hw_filter() 3 | -> tcf_exts_validate_ex() 4 | -> flags |= TCA_ACT_FLAGS_BIND; 5 | -> tcf_action_init() 6 | -> tcf_action_init_1() 7 | -> a_o->init() 8 | -> tcf_mirred_init() 9 | -> tcf_idr_create_from_flags() 10 | -> tcf_idr_create() 11 | -> p->tcfa_flags = flags; 12 | -> tc_act_bind(flags)) 13 | -> tc_act_bind(act->tcfa_flags) When invoked from tcf_exts_validate_ex() like matchall does (but other classifiers validate their extensions as well), tcf_action_init() runs in a call path where "flags" always contains TCA_ACT_FLAGS_BIND (set by line 4). So line 12 is always true, and line 13 is always true as well. No transition ever takes place, and the check is skipped. The code was added in this form in commit c86e0209dc77 ("flow_offload: validate flags of filter and actions"), but I'm attributing the blame even earlier in that series, to when TCA_ACT_FLAGS_SKIP_HW and TCA_ACT_FLAGS_SKIP_SW were added to the UAPI. Following the development process of this change, the check did not always exist in this form. A change took place between v3 [1] and v4 [2], AFAIU due to review feedback that it doesn't make sense for action flags to be different than classifier flags. I think I agree with that feedback, but it was translated into code that omits enforcing this for "classic" actions created at the same time with the filters themselves. There are 3 more important cases to discuss. First there is this command: $ tc qdisc add dev eth0 clasct $ tc filter add dev eth0 ingress matchall skip_sw \ action mirred ingress mirror dev eth1 which should be allowed, because prior to the concept of dedicated action flags, it used to work and it used to mean the action inherited the skip_sw/skip_hw flags from the classifier. It's not a mismatch. Then we have this command: $ tc qdisc add dev eth0 clasct $ tc filter add dev eth0 ingress matchall skip_sw \ action mirred ingress mirror dev eth1 skip_hw where there is a mismatch and it should be rejected. Finally, we have: $ tc qdisc add dev eth0 clasct $ tc filter add dev eth0 ingress matchall skip_sw \ action mirred ingress mirror dev eth1 skip_sw where the offload flags coincide, and this should be treated the same as the first command based on inheritance, and accepted. [1]: https://lore.kernel.org/netdev/20211028110646.13791-9-simon.horman@corigine.com/ [2]: https://lore.kernel.org/netdev/20211118130805.23897-10-simon.horman@corigine.com/ Fixes: 7adc57651211 ("flow_offload: add skip_hw and skip_sw to control if offload the action") Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Simon Horman <horms@kernel.org> Reviewed-by: Ido Schimmel <idosch@nvidia.com> Tested-by: Ido Schimmel <idosch@nvidia.com> Link: https://patch.msgid.link/20241017161049.3570037-1-vladimir.oltean@nxp.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2024-10-15net: sched: Use rtnl_register_many().Kuniyuki Iwashima
We will remove rtnl_register() in favour of rtnl_register_many(). When it succeeds, rtnl_register_many() guarantees all rtnetlink types in the passed array are supported, and there is no chance that a part of message types is not supported. Let's use rtnl_register_many() instead. Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://patch.msgid.link/20241014201828.91221-5-kuniyu@amazon.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2024-06-20Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/netJakub Kicinski
Cross-merge networking fixes after downstream PR. Conflicts: drivers/net/ethernet/broadcom/bnxt/bnxt.c 1e7962114c10 ("bnxt_en: Restore PTP tx_avail count in case of skb_pad() error") 165f87691a89 ("bnxt_en: add timestamping statistics support") No adjacent changes. Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2024-06-18net/sched: act_api: fix possible infinite loop in tcf_idr_check_alloc()David Ruth
syzbot found hanging tasks waiting on rtnl_lock [1] A reproducer is available in the syzbot bug. When a request to add multiple actions with the same index is sent, the second request will block forever on the first request. This holds rtnl_lock, and causes tasks to hang. Return -EAGAIN to prevent infinite looping, while keeping documented behavior. [1] INFO: task kworker/1:0:5088 blocked for more than 143 seconds. Not tainted 6.9.0-rc4-syzkaller-00173-g3cdb45594619 #0 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:kworker/1:0 state:D stack:23744 pid:5088 tgid:5088 ppid:2 flags:0x00004000 Workqueue: events_power_efficient reg_check_chans_work Call Trace: <TASK> context_switch kernel/sched/core.c:5409 [inline] __schedule+0xf15/0x5d00 kernel/sched/core.c:6746 __schedule_loop kernel/sched/core.c:6823 [inline] schedule+0xe7/0x350 kernel/sched/core.c:6838 schedule_preempt_disabled+0x13/0x30 kernel/sched/core.c:6895 __mutex_lock_common kernel/locking/mutex.c:684 [inline] __mutex_lock+0x5b8/0x9c0 kernel/locking/mutex.c:752 wiphy_lock include/net/cfg80211.h:5953 [inline] reg_leave_invalid_chans net/wireless/reg.c:2466 [inline] reg_check_chans_work+0x10a/0x10e0 net/wireless/reg.c:2481 Fixes: 0190c1d452a9 ("net: sched: atomically check-allocate action") Reported-by: syzbot+b87c222546179f4513a7@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=b87c222546179f4513a7 Signed-off-by: David Ruth <druth@chromium.org> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://lore.kernel.org/r/20240614190326.1349786-1-druth@chromium.org Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2024-06-06net: use unrcu_pointer() helperEric Dumazet
Toke mentioned unrcu_pointer() existence, allowing to remove some of the ugly casts we have when using xchg() for rcu protected pointers. Also make inet_rcv_compat const. Signed-off-by: Eric Dumazet <edumazet@google.com> Cc: Toke Høiland-Jørgensen <toke@redhat.com> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com> Link: https://lore.kernel.org/r/20240604111603.45871-1-edumazet@google.com Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2024-02-02net/sched: Load modules via their aliasMichal Koutný
The cls_,sch_,act_ modules may be loaded lazily during network configuration but without user's awareness and control. Switch the lazy loading from canonical module names to a module alias. This allows finer control over lazy loading, the precedent from commit 7f78e0351394 ("fs: Limit sys_mount to only request filesystem modules.") explains it already: Using aliases means user space can control the policy of which filesystem^W net/sched modules are auto-loaded by editing /etc/modprobe.d/*.conf with blacklist and alias directives. Allowing simple, safe, well understood work-arounds to known problematic software. By default, nothing changes. However, if a specific module is blacklisted (its canonical name), it won't be modprobe'd when requested under its alias (i.e. kernel auto-loading). It would appear as if the given module was unknown. The module can still be loaded under its canonical name, which is an explicit (privileged) user action. Signed-off-by: Michal Koutný <mkoutny@suse.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20240201130943.19536-4-mkoutny@suse.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2024-01-07net/sched: simplify tc_action_load_ops parametersPedro Tammela
Instead of using two bools derived from a flags passed as arguments to the parent function of tc_action_load_ops, just pass the flags itself to tc_action_load_ops to simplify its parameters. Reviewed-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: David S. Miller <davem@davemloft.net>
2024-01-03net/sched: introduce ACT_P_BOUND return codePedro Tammela
Bound actions always return '0' and as of today we rely on '0' being returned in order to properly skip bound actions in tcf_idr_insert_many. In order to further improve maintainability, introduce the ACT_P_BOUND return code. Actions are updated to return 'ACT_P_BOUND' instead of plain '0'. tcf_idr_insert_many is then updated to check for 'ACT_P_BOUND'. Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://lore.kernel.org/r/20231229132642.1489088-1-pctammela@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2023-12-20net: sched: Add initial TC error skb drop reasonsVictor Nogueira
Continue expanding Daniel's patch by adding new skb drop reasons that are idiosyncratic to TC. More specifically: - SKB_DROP_REASON_TC_COOKIE_ERROR: An error occurred whilst processing a tc ext cookie. - SKB_DROP_REASON_TC_CHAIN_NOTFOUND: tc chain lookup failed. - SKB_DROP_REASON_TC_RECLASSIFY_LOOP: tc exceeded max reclassify loop iterations Signed-off-by: Victor Nogueira <victor@mojatatu.com> Reviewed-by: Simon Horman <horms@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
2023-12-20net: sched: Move drop_reason to struct tc_skb_cbVictor Nogueira
Move drop_reason from struct tcf_result to skb cb - more specifically to struct tc_skb_cb. With that, we'll be able to also set the drop reason for the remaining qdiscs (aside from clsact) that do not have access to tcf_result when time comes to set the skb drop reason. Signed-off-by: Victor Nogueira <victor@mojatatu.com> Acked-by: Daniel Borkmann <daniel@iogearbox.net> Reviewed-by: Simon Horman <horms@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
2023-12-13net/sched: act_api: skip idr replace on bound actionsPedro Tammela
tcf_idr_insert_many will replace the allocated -EBUSY pointer in tcf_idr_check_alloc with the real action pointer, exposing it to all operations. This operation is only needed when the action pointer is created (ACT_P_CREATED). For actions which are bound to (returned 0), the pointer already resides in the idr making such operation a nop. Even though it's a nop, it's still not a cheap operation as internally the idr code walks the idr and then does a replace on the appropriate slot. So if the action was bound, better skip the idr replace entirely. Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Reviewed-by: Vlad Buslov <vladbu@nvidia.com> Link: https://lore.kernel.org/r/20231211181807.96028-3-pctammela@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2023-12-13net/sched: act_api: rely on rcu in tcf_idr_check_allocPedro Tammela
Instead of relying only on the idrinfo->lock mutex for bind/alloc logic, rely on a combination of rcu + mutex + atomics to better scale the case where multiple rtnl-less filters are binding to the same action object. Action binding happens when an action index is specified explicitly and an action exists which such index exists. Example: tc actions add action drop index 1 tc filter add ... matchall action drop index 1 tc filter add ... matchall action drop index 1 tc filter add ... matchall action drop index 1 tc filter ls ... filter protocol all pref 49150 matchall chain 0 filter protocol all pref 49150 matchall chain 0 handle 0x1 not_in_hw action order 1: gact action drop random type none pass val 0 index 1 ref 4 bind 3 filter protocol all pref 49151 matchall chain 0 filter protocol all pref 49151 matchall chain 0 handle 0x1 not_in_hw action order 1: gact action drop random type none pass val 0 index 1 ref 4 bind 3 filter protocol all pref 49152 matchall chain 0 filter protocol all pref 49152 matchall chain 0 handle 0x1 not_in_hw action order 1: gact action drop random type none pass val 0 index 1 ref 4 bind 3 When no index is specified, as before, grab the mutex and allocate in the idr the next available id. In this version, as opposed to before, it's simplified to store the -EBUSY pointer instead of the previous alloc + replace combination. When an index is specified, rely on rcu to find if there's an object in such index. If there's none, fallback to the above, serializing on the mutex and reserving the specified id. If there's one, it can be an -EBUSY pointer, in which case we just try again until it's an action, or an action. Given the rcu guarantees, the action found could be dead and therefore we need to bump the refcount if it's not 0, handling the case it's in fact 0. As bind and the action refcount are already atomics, these increments can happen without the mutex protection while many tcf_idr_check_alloc race to bind to the same action instance. In case binding encounters a parallel delete or add, it will return -EAGAIN in order to try again. Both filter and action apis already have the retry machinery in-place. In case it's an unlocked filter it retries under the rtnl lock. Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Reviewed-by: Vlad Buslov <vladbu@nvidia.com> Link: https://lore.kernel.org/r/20231211181807.96028-2-pctammela@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2023-12-11net/sched: act_api: conditional notification of eventsPedro Tammela
As of today tc-action events are unconditionally built and sent to RTNLGRP_TC. As with the introduction of rtnl_notify_needed we can check before-hand if they are really needed. Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20231208192847.714940-6-pctammela@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2023-12-11net/sched: act_api: don't open code max()Pedro Tammela
Use max() in a couple of places that are open coding it with the ternary operator. Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20231208192847.714940-5-pctammela@mojatatu.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2023-12-05net/sched: act_api: use tcf_act_for_each_action in tcf_idr_insert_manyPedro Tammela
The actions array is contiguous, so stop processing whenever a NULL is found. This is already the assumption for tcf_action_destroy[1], which is called from tcf_actions_init. [1] https://elixir.bootlin.com/linux/v6.7-rc3/source/net/sched/act_api.c#L1115 Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2023-12-05net/sched: act_api: stop loop over ops array on NULL in tcf_action_initPedro Tammela
The ops array is contiguous, so stop processing whenever a NULL is found Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2023-12-05net/sched: act_api: avoid non-contiguous action arrayPedro Tammela
In tcf_action_add, when putting the reference for the bound actions it assigns NULLs to just created actions passing a non contiguous array to tcf_action_put_many. Refactor the code so the actions array is always contiguous. Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2023-12-05net/sched: act_api: use tcf_act_for_each_actionPedro Tammela
Use the auxiliary macro tcf_act_for_each_action in all the functions that expect a contiguous action array Suggested-by: Marcelo Ricardo Leitner <mleitner@redhat.com> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com> Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
2023-11-06net, sched: Fix SKB_NOT_DROPPED_YET splat under debug configJamal Hadi Salim
Getting the following splat [1] with CONFIG_DEBUG_NET=y and this reproducer [2]. Problem seems to be that classifiers clear 'struct tcf_result::drop_reason', thereby triggering the warning in __kfree_skb_reason() due to reason being 'SKB_NOT_DROPPED_YET' (0). Fixed by disambiguating a legit error from a verdict with a bogus drop_reason [1] WARNING: CPU: 0 PID: 181 at net/core/skbuff.c:1082 kfree_skb_reason+0x38/0x130 Modules linked in: CPU: 0 PID: 181 Comm: mausezahn Not tainted 6.6.0-rc6-custom-ge43e6d9582e0 #682 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc37 04/01/2014 RIP: 0010:kfree_skb_reason+0x38/0x130 [...] Call Trace: <IRQ> __netif_receive_skb_core.constprop.0+0x837/0xdb0 __netif_receive_skb_one_core+0x3c/0x70 process_backlog+0x95/0x130 __napi_poll+0x25/0x1b0 net_rx_action+0x29b/0x310 __do_softirq+0xc0/0x29b do_softirq+0x43/0x60 </IRQ> [2] ip link add name veth0 type veth peer name veth1 ip link set dev veth0 up ip link set dev veth1 up tc qdisc add dev veth1 clsact tc filter add dev veth1 ingress pref 1 proto all flower dst_mac 00:11:22:33:44:55 action drop mausezahn veth0 -a own -b 00:11:22:33:44:55 -q -c 1 Ido reported: [...] getting the following splat [1] with CONFIG_DEBUG_NET=y and this reproducer [2]. Problem seems to be that classifiers clear 'struct tcf_result::drop_reason', thereby triggering the warning in __kfree_skb_reason() due to reason being 'SKB_NOT_DROPPED_YET' (0). [...] [1] WARNING: CPU: 0 PID: 181 at net/core/skbuff.c:1082 kfree_skb_reason+0x38/0x130 Modules linked in: CPU: 0 PID: 181 Comm: mausezahn Not tainted 6.6.0-rc6-custom-ge43e6d9582e0 #682 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc37 04/01/2014 RIP: 0010:kfree_skb_reason+0x38/0x130 [...] Call Trace: <IRQ> __netif_receive_skb_core.constprop.0+0x837/0xdb0 __netif_receive_skb_one_core+0x3c/0x70 process_backlog+0x95/0x130 __napi_poll+0x25/0x1b0 net_rx_action+0x29b/0x310 __do_softirq+0xc0/0x29b do_softirq+0x43/0x60 </IRQ> [2] #!/bin/bash ip link add name veth0 type veth peer name veth1 ip link set dev veth0 up ip link set dev veth1 up tc qdisc add dev veth1 clsact tc filter add dev veth1 ingress pref 1 proto all flower dst_mac 00:11:22:33:44:55 action drop mausezahn veth0 -a own -b 00:11:22:33:44:55 -q -c 1 What happens is that inside most classifiers the tcf_result is copied over from a filter template e.g. *res = f->res which then implicitly overrides the prior SKB_DROP_REASON_TC_{INGRESS,EGRESS} default drop code which was set via sch_handle_{ingress,egress}() for kfree_skb_reason(). Commit text above copied verbatim from Daniel. The general idea of the patch is not very different from what Ido originally posted but instead done at the cls_api codepath. Fixes: 54a59aed395c ("net, sched: Make tc-related drop reason more flexible") Reported-by: Ido Schimmel <idosch@idosch.org> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> Link: https://lore.kernel.org/netdev/ZTjY959R+AFXf3Xy@shredder Reviewed-by: Simon Horman <horms@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>