This list has been moved to per-netns, remove onstack list which is not
used anymore.
Fixes: b343ededb3 ("netfilter: nf_tables: move set_update_list to nftables per-netns")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
.commit call for sets is skipped if set->dead flag is set on, but this
flag is set on later in the commit path.
This also reintroduces the bug fixed in commit 7315dc1e12
("netfilter: nf_tables: skip set commit for deleted/destroyed sets").
Fixes: 1e3b9e1c77 ("netfilter: nf_tables: call set ops .commit when building new ruleset blob")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Both the dynset and newsetelem path clone the existing set expressions
when setting set element expressions if no override expressions are
provided. This results in a double clone, once to clone the template set
expressions then another clone on the new element. Add a flag to
annotate if userspace provides a override expression (ie. expression of
the same type of the set but different configuration), otherwise borrow
the existing expression from the set. Add conditionals to release
expression iif they represent an override. Use this new override_exprs
flag to dump the dynset expression override to userspace.
This simplifies the existing logic and it also fixes a bug with the
connlimit expression which results in a module refcount imbalance
WARNING splat when resorting on the default set expressions.
Fixes: 65038428b2 ("netfilter: nf_tables: allow to specify stateful expression in set definition")
Fixes: fca05d4d61 ("netfilter: nft_dynset: honor stateful expressions in set definition")
Reported-by: Xingyuan Mo <hdthky0@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Several xtables extension still use pr_err() or pr_info() without
ratelimit.
For xt_cgroup, while at this, remove redundant "xt_cgroup:" prefix
since pr_fmt is already set on.
Fixes: c38c4597e4 ("netfilter: implement xt_cgroup cgroup2 path match")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Remove pr_debug() for these xtables extensions, these have no use
these days. Still, turn pr_debug() into pr_info_ratelimited() in the
.checkentry path since this helps provide a hint via dmesg in legacy
iptables.
Exception is xt_IDLETIMER in the module init path, where pr_err() is
used.
Add missing pr_fmt() definition in xt_REDIRECT, xt_NETMAP and
xt_MASQUERADE.
Add missing \n to several pr_debug() that were translated to use
pr_info_ratelimited().
Link: https://patch.msgid.link/cover.1786933680.git.rakukuip@gmail.com/
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Since pipapo_get_avx2() uses YMM registers, execute vzeroupper before
returning from it. This is needed to avoid degrading the performance of
any later SSE code that may happen to be executed.
Fixes: 7400b06396 ("nft_set_pipapo: Introduce AVX2-based lookup implementation")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Allocate the chain blob before the ruleset offload to reduce chances of
entering an inconsistent state where the offloaded ruleset in the nic
and the software ruleset differ.
Fixes: c9626a2cbd ("netfilter: nf_tables: add hardware offload support")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Replace WARN_ON_ONCE with DEBUG_NET_WARN_ON_ONCE in __nf_conncount_add.
The function handles count limit breaches safely by returning
-EOVERFLOW, so a production backtrace is not needed. This prevents
unnecessary system panics when panic_on_warn=1 is enabled in production
systems.
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Replace WARN_ON calls with DEBUG_NET_WARN_ON_ONCE in the default switch
blocks of nf_tproxy_get_sock_v4 and v6. Unsupported transport protocols
are already safely handled by returning a NULL socket pointer. This
prevents unnecessary system panics when panic_on_warn=1 is enabled in
production systems.
Link: https://patch.msgid.link/cover.1786968834.git.zhilinz@nebusec.ai/
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
The cdc_devs[] quirk table special-cases the Mac CDC-NCM private
interface personality only for USB product ID 0x1905.
Some MacBook Pro models (e.g. M1 Max) connected over a
USB4/Thunderbolt 3/4 cable to a host whose Thunderbolt controller
lacks PCIe tunneling support (no NHI function, USB4-only mode)
present themselves with product ID 0x1902 instead,
using the same descriptor layout as 0x1905: a Communications
control interface with zero endpoints (no interrupt/status endpoint)
paired with a CDC Data interface, at
interface numbers 0 and 2.
Because 0x1902 is unmatched, these devices fall through to the
generic cdc_ncm_info driver_info, which sets FLAG_LINK_INTR and
therefore requires an interrupt endpoint on the control interface.
Apple's private NCM interface never provides one, so cdc_ncm_bind()
fails outright:
cdc_ncm 2-1:1.0: bind() failure
cdc_ncm 2-1:1.2: bind() failure
and no network device is created, breaking Ethernet-over-USB4
between the Mac and any USB4 host lacking Thunderbolt PCIe
tunneling.
Add matching entries for 0x1902 alongside the existing 0x1905
ones, reusing apple_private_interface_info as with the other Mac
ID.
Signed-off-by: Mehrdad Afshari <mehrdad@signeen.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Nikolay Aleksandrov says:
====================
bridge/vxlan: fix reading neigh ha without synchronization
Neigh ha address must be read using the seqlock to get a stable snapshot.
Both the bridge and vxlan read it directly and can see partial updates.
I reproduced both issues with running neigh updates and exercising these
paths in parallel and saw partial addresses, e.g. updating between
neigh A: 02:00:00:00:00:00 neigh B: fe:ff:ff:ff:ff:ff was able to observe
02:00:ff:ff:ff:ff and fe:ff:00:00:00:00 in packets. Noticed this initially
in the bridge, then checked vxlan and its arp/neigh_reduce functions have
the same bug, route_shortcircuit is doing the right thing already.
====================
Link: https://patch.msgid.link/20260818150756.890025-1-razor@blackwall.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
smc_llc_flow_stop() resets a flow struct with a blind memset:
spin_lock_bh(&lgr->llc_flow_lock);
memset(flow, 0, sizeof(*flow));
flow->type = SMC_LLC_FLOW_NONE;
spin_unlock_bh(&lgr->llc_flow_lock);
If flow->qentry is non-NULL at this point the pointer is overwritten without the
allocation being freed, leaking one kmalloc object.
A late-arriving duplicate CONFIRM_LINK or ADD_LINK_CONT message can set
flow->qentry after the legitimate message has been consumed by the waiter via
smc_llc_flow_qentry_clr() (which NULLs the pointer but leaves flow->type
non-zero) but before the flow completes and smc_llc_flow_stop() runs. In that
window the duplicate is stashed into flow->qentry, and then lost when
smc_llc_flow_stop() zeros the struct.
Call smc_llc_flow_qentry_del() inside the lock before the memset.
smc_llc_flow_qentry_del() already checks flow->qentry before freeing, so the
normal case where no entry is pending is a no-op.
Fixes: 555da9af82 ("net/smc: add event-based llc_flow framework")
Reviewed-by: Hidayath Khan <hidayath@linux.ibm.com>
Signed-off-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
Link: https://patch.msgid.link/20260818073943.1108383-1-mjambigi@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When smc_llc_event_handler() transitions the local LLC flow from
SMC_LLC_FLOW_REQ_ADD_LINK to SMC_LLC_FLOW_ADD_LINK on arrival of an ADD_LINK
request, it calls smc_llc_flow_qentry_set() unconditionally:
if (lgr->llc_flow_lcl.type == SMC_LLC_FLOW_REQ_ADD_LINK) {
lgr->llc_flow_lcl.type = SMC_LLC_FLOW_ADD_LINK;
smc_llc_flow_qentry_set(&lgr->llc_flow_lcl, qentry);
...
}
A CONFIRM_LINK or ADD_LINK_CONT arriving while flow->type is
SMC_LLC_FLOW_REQ_ADD_LINK is stashed into flow->qentry via the
SMC_LLC_CONFIRM_LINK / SMC_LLC_ADD_LINK_CONT handler (which stores into
flow->qentry for any non-NONE flow type). When the subsequent ADD_LINK
arrives, the REQ_ADD_LINK branch overwrites flow->qentry with the new pointer
without first freeing the stashed allocation, leaking one kmalloc object.
The stashed entry has no consumer: smc_llc_wait() is only called from
llc_add_link_work, which is not yet scheduled while the flow type remains
REQ_ADD_LINK. No waiter is sleeping on llc_msg_waiter at this point.
It is safe to unconditionally free any stashed qentry before
the overwrite.
Call smc_llc_flow_qentry_del() before smc_llc_flow_qentry_set() in the
REQ_ADD_LINK branch. smc_llc_flow_qentry_del() already checks flow->qentry
before freeing, so the normal path where no entry is stashed is a no-op.
Fixes: b4ba4652b3 ("net/smc: extend LLC layer for SMC-Rv2")
Reviewed-by: Hidayath Khan <hidayath@linux.ibm.com>
Signed-off-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
Link: https://patch.msgid.link/20260818073107.466506-1-mjambigi@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
In phylink_inband_caps(), the PCS returned by mac_select_pcs is only
checked if NULL but mac_select_pcs can also return an error pointer.
This can cause a kernel panic as phylink_pcs_inband_caps() only checks
if passed PCS is not NULL and directly dereference ops from the phylink_pcs
struct.
Use the IS_ERR_OR_NULL macro to address both case where the returned
PCS can be NULL or an error pointer and prevent a kernel panic.
Cc: stable@vger.kernel.org
Fixes: df874f9e52 ("net: phylink: add pcs_inband_caps() method")
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
Link: https://patch.msgid.link/20260817213009.13924-1-ansuelsmth@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Koichiro Den says:
====================
net: ntb_netdev: Fix TX completion and error handling
This small series fixes several TX buffer ownership and queue handling
bugs in ntb_netdev and ntb_transport.
Patch 4 first appeared in my "NTB: Add direct TX/RX using PCI endpoint
DMA" series. Sashiko later reported the same pre-existing leak while
reviewing another series, so I moved the fix here. See:
https://lore.kernel.org/r/20260815032932.151F11F000E9@smtp.kernel.org/
The ntb_transport fixes affect buffer ownership and queue handling in
ntb_netdev, the only in-tree ntb_transport_client implementation, so the
patches need to go in together. I am targeting the net tree for the
series.
====================
Link: https://patch.msgid.link/20260817053519.4135287-1-den@valinux.co.jp
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
ntb_process_tx() handles an oversized buffer by calling tx_handler()
with a NULL data pointer and returning success. ntb_netdev therefore
neither frees the skb in its completion callback nor takes its enqueue
error path, leaking it.
Reject oversized buffers in ntb_transport_tx_enqueue() before acquiring
a queue entry and return -EMSGSIZE. The caller retains ownership of the
buffer, and the preceding netdev patch frees the skb when enqueue
returns this permanent error.
Fixes: fce8a7bb5b ("PCI-Express Non-Transparent Bridge Support")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Link: https://patch.msgid.link/20260817053519.4135287-5-den@valinux.co.jp
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Commit f195a1a6fe ("ntb: Drop packets when qp link is down") meant to
make ntb_transport_tx_enqueue() drop packets submitted while the QP link
is down, but it only returns 0 without consuming the packet. Zero means
success by this function's contract, so ntb_netdev reports NETDEV_TX_OK
and forgets the skb: nothing queued it, nothing frees it, and it leaks,
one skb for every transmit racing a link-down.
Return -ENOLINK instead, restoring the contract that a non-zero return
leaves the buffer owned by the caller. With the preceding patch,
ntb_netdev frees the skb on non-retryable enqueue failures and returns
NETDEV_TX_OK, so a packet racing with link-down is dropped without leaking
or entering a busy retry loop.
Fixes: f195a1a6fe ("ntb: Drop packets when qp link is down")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Link: https://patch.msgid.link/20260817053519.4135287-4-den@valinux.co.jp
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Currently, ntb_netdev returns NETDEV_TX_BUSY for every enqueue error. It
also increments the drop and error counters while leaving the skb owned
by the qdisc, and may return BUSY with the subqueue still awake.
Retrying a permanent error cannot succeed either.
The unconditional BUSY return and premature accounting date back to the
initial driver. The error-path queue stop was later removed without
changing that return value. The current flow-control code includes a
resource check, but ntb_netdev does not honor its result before enqueue.
Honor the resource check before enqueue. For -EAGAIN and -EBUSY, stop
the subqueue, arm the existing reaper timer, and return BUSY without
touching the skb. For other errors, free the skb, increment tx_dropped,
and return NETDEV_TX_OK.
Fixes: 548c237c0a ("net: Add support for NTB virtual ethernet device")
Fixes: d723485cb4 ("ntb_netdev: remove tx timeout")
Fixes: e74bfeedad ("NTB: Add flow control to the ntb_netdev")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Link: https://patch.msgid.link/20260817053519.4135287-3-den@valinux.co.jp
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
ntb_tx_copy_callback() invokes the client callback before returning the
entry to tx_free_q. The callback may wake a stopped client queue, only
for the next enqueue to find no local entry and return -EBUSY. The window
is narrow, but the retry is unnecessary.
Save the callback data and length, then return the entry to tx_free_q
before invoking the client. A completion callback then means both the
client buffer and transport entry are ready for reuse.
Fixes: fce8a7bb5b ("PCI-Express Non-Transparent Bridge Support")
Cc: stable@vger.kernel.org
Signed-off-by: Koichiro Den <den@valinux.co.jp>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Link: https://patch.msgid.link/20260817053519.4135287-2-den@valinux.co.jp
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
sctp_rcv() resolves the transport once per packet and leaves it in
chunk->transport. The lookup reference, or the one sctp_add_backlog() takes
if the socket is owned by userspace, keeps it around until the chunk has
been processed.
An authenticated ASCONF DEL-IP can remove it in the meantime.
sctp_assoc_rm_peer() takes the transport out of the association and calls
sctp_transport_free(), which tags it dead and drops the reference the
association held. There is a window on both paths: the packet can sit on
the socket backlog, and on the direct path the lookup completes before
bh_lock_sock().
The DATA chunk in that packet puts the removed transport back into
asoc->peer.last_data_from. Once the packet is done that reference goes
away and the transport is freed by RCU, so the next delayed SACK carries
the pointer into the SACK chunk and sctp_outq_select_transport() reads the
freed transport's state.
Drop the chunk in sctp_inq_push(), next to the existing rcvr->dead check.
Both paths reach it with the association's socket lock held. The peer
retransmits it.
Fixes: 1da177e4c3 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
Acked-by: Xin Long <lucien.xin@gmail.com>
Link: https://patch.msgid.link/aoUJHQmxL0LFIMCw@v4bel
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
gred_enqueue(), bfifo_enqueue() and plug_enqueue() admit a packet when the
current backlog plus the packet length fits within the queue limit:
sch->qstats.backlog + qdisc_pkt_len(skb) <= sch->limit (gred default VQ)
gred_backlog+qdisc_pkt_len(skb) <= q->limit (gred configured VQ)
sch->qstats.backlog + qdisc_pkt_len(skb) <= sch->limit (bfifo)
sch->qstats.backlog + skb->len <= q->limit (plug)
sch->qstats.backlog and q->backlog are u32, and qdisc_pkt_len()/skb->len
are unsigned int, so all sums are computed in 32 bits and wrap at 2^32.
Once the true backlog exceeds 4 GiB the wrapped sum becomes small and
admission keeps succeeding, so the queue grows without bound and the kernel
can be driven to OOM.
Promote the sums to u64 so admission stops once the true backlog exceeds
the limit. The limit is u32, so the bounded queue stays below 2^32 and
the stored u32 backlog never wraps.
The bug can only be reproduced as root (albeit with ridiculous setup):
attach a gred (or bfifo/plug) qdisc with a limit near 4 GiB,
leaving the default VQ unconfigured (for gred), and drive >4 GiB of
queued traffic (e.g. via a size table / stab to inflate qdisc_pkt_len,
or sustained high-rate traffic). The u32 backlog+len sum wraps at 2^32,
admission keeps succeeding, and the queue grows unboundedly to OOM.
Fixes: a3eb95f891 ("net_sched: gred: add TCA_GRED_LIMIT attribute")
Reported-by: vega@nebusec.ai
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260818095927.15901-1-jhs@mojatatu.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
AF_PACKET TX_RING skbs keep a raw pointer to their ring frame. The skb
page references preserve page-backed ring blocks after pg_vec is freed,
but they do not preserve a vmalloc mapping.
tpacket_destruct_skb() currently drops the pending reference before
writing the timestamp and TP_STATUS_AVAILABLE to the frame. Move the
decrement after those stores. The smp_wmb() in __packet_set_status()
orders the frame stores before the decrement.
Also recheck pending TX frames under pg_vec_lock before non-closing
ring replacement, so a racing send cannot add a pending skb between
the initial check and the ring swap.
Ring allocation can produce a mixture of page-backed and vmalloc-backed
blocks. Allocate deferred-work storage during TX ring setup when the
first vmalloc-backed block is encountered, and keep its pointer in the
pg_vec allocation header. If allocation fails, return -ENOMEM from ring
setup. On socket close, a non-NULL pointer identifies a vmalloc-backed
vector without a scan. If TX skbs remain, defer the whole vector to
system_long_wq.
After pg_vec is detached, a late destructor can skip the pending
decrement. Use socket write-memory accounting as the deferred lifetime
gate instead: an skb remains charged through its final sock_wfree(),
after all ring-frame accesses. The delayed work retains a socket
reference and reschedules itself until no TX skbs remain.
Move pending_refcnt release to packet_sock_destruct() so late skb
destructors and deferred cleanup can safely use it after
packet_release(). Page-backed teardown remains synchronous, and no lock
is added to the TX completion hot path.
Fixes: b013840810 ("packet: use percpu mmap tx frame pending refcount")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/netdev/20260721015824.45829-1-kylebot@openai.com/
Suggested-by: Eric Dumazet <edumazet@google.com>
Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Link: https://patch.msgid.link/20260816235646.76500-1-kylebot@openai.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Abhijit Gangurde says:
====================
Extend the net/ionic firmware identity structure to expose
the rcq_sign_bit field from the RDMA LIF identity.
* 'ionic_rcq_shared' of https://github.com/abhijitG-xlnx/linux:
net: ionic: Fetch RCQ sign bit from firmware
====================
Link: https://patch.msgid.link/
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Toggling hardware VLAN TX offload (NETIF_F_HW_VLAN_CTAG_TX or
NETIF_F_HW_VLAN_STAG_TX) on a lower device invokes vlan_transfer_features(),
which dynamically changed vlandev->hard_header_len.
This causes two issues:
1. Lockless TX paths (e.g. packet_snd in af_packet.c, ip6_finish_output2)
read dev->hard_header_len without holding RTNL lock. Mutating
hard_header_len dynamically under RTNL creates a data race where upper
layers reserve insufficient headroom based on a stale hard_header_len,
resulting in skb_under_panic when vlan_dev_hard_header() is called.
2. In addition, vlan_transfer_features() updated hard_header_len without
updating header_ops, causing a mismatch between allocated headroom
and header creation.
Always setting dev->hard_header_len = real_dev->hard_header_len and
dev->needed_headroom = real_dev->needed_headroom + VLAN_HLEN unconditionally
ensures:
- dev->hard_header_len remains 100% static and immutable at real_dev->hard_header_len,
eliminating all dynamic runtime updates and data races on hard_header_len.
- Upper layers allocating skbs via LL_RESERVED_SPACE() will always reserve
sufficient headroom for software VLAN tag insertion (real_dev->hard_header_len +
real_dev->needed_headroom + VLAN_HLEN).
- vlandev inherits real_dev->needed_tailroom so underlying trailer/padding/ICV
requirements are honored.
- AF_PACKET SOCK_RAW network header offsets remain correctly aligned at
real_dev->hard_header_len.
- vlan_header_ops is used unconditionally.
Note to stable teams: Make sure to backport these commits:
e16e960d55 ("ipvlan: inherit needed_headroom and needed_tailroom from phy_dev")
cef51860be ("macvlan: inherit needed_headroom and needed_tailroom from lowerdev")
Fixes: 1da177e4c3 ("Linux-2.6.12-rc2")
Reported-by: Tangxin Xie <xietangxin@h-partners.com>
Closes: https://lore.kernel.org/netdev/99d678ae-c7b2-4b44-b534-b8320679deb3@h-partners.com/
Cc: <stable@vger.kernel.org> # 3.19: e16e960d55: ipvlan: inherit needed_headroom and needed_tailroom from phy_dev
Cc: <stable@vger.kernel.org> # 3.19: cef51860be: macvlan: inherit needed_headroom and needed_tailroom from lowerdev
Cc: <stable@vger.kernel.org> # 3.19
Signed-off-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20260811085246.2267779-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The network and transport header fields in struct sk_buff are 16-bit
offsets from skb->head, and U16_MAX is reserved as the unset transport
header value. batadv_tvlv_call_handler() sets both fields from a received
multicast TVLV without checking whether the TVLV end is representable.
If the end offset exceeds the field's range, skb_set_transport_header()
truncates it so that the transport header precedes the network header.
The negative difference is then returned by skb_network_header_len() as
a large u32. batadv_mcast_forw_packet() consequently accepts an oversized
multicast tracker and accesses memory beyond the skb data.
Add skb_set_transport_header_careful(), an offset-aware counterpart to
skb_reset_transport_header_careful(), which validates the final
head-relative offset before assigning it. Use the new helper in
batadv_tvlv_call_handler() and reject unrepresentable TVLVs before
setting the network header.
Fixes: 07afe1ba28 ("batman-adv: mcast: implement multicast packet reception and forwarding")
Cc: stable@vger.kernel.org
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Co-developed-by: David Lee <david.lee@trailofbits.com>
Signed-off-by: David Lee <david.lee@trailofbits.com>
Acked-by: Sven Eckelmann <sven@narfation.org>
Link: https://patch.msgid.link/20260817084955.944189-1-david.lee@trailofbits.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
End.DX4 and End.DT4 decapsulate an IPv4 packet through
decap_and_validate() and send it directly to IPv4 routing. The inner
packet therefore bypasses ip_rcv_core(), which normally clears IPCB
before IPv4 interprets skb->cb.
The skb instead retains IP6CB data from the outer packet. IP6CB and
IPCB use the same skb->cb storage, so IP6CB(skb)->lastopt overlaps
IPCB(skb)->opt.optlen and srr, while IP6CB(skb)->nhoff overlaps rr and
ts.
The sender can make the stale optlen byte nonzero with a valid outer
extension-header chain. The reproducers put an eight-byte Destination
Options header immediately after the 40-byte IPv6 header and before the
Segment Routing Header. ipv6_destopt_rcv() records the sender-controlled
Destination Options offset in both lastopt and nhoff, setting them to
40. On the reproduced little-endian x86-64 kernel, IPv4 therefore sees
optlen = 40 and rr = 40.
Both tcp_v4_save_options() and __ip_options_echo() skip option copying
when optlen is zero. Here optlen is 40, so the TCP SYN path allocates
room for 40 bytes of option data and calls __ip_options_echo(). The
stale rr value makes that function read inner packet byte 41 as the
Record Route option length. The reproducers set that sender-controlled
byte to 255, so __ip_options_echo() copies 255 bytes into the 40-byte
option-data area.
Separate End.DX4 and End.DT4 reproducers on the unpatched v7.2-rc5
kernel both produced:
BUG: KASAN: slab-out-of-bounds in __ip_options_echo()
Write of size 255
The relevant End.DX4 call path is:
__ip_options_echo
tcp_v4_route_req
tcp_conn_request
tcp_v4_conn_request
tcp_rcv_state_process
tcp_v4_do_rcv
tcp_v4_rcv
ip_protocol_deliver_rcu
ip_local_deliver_finish
ip_local_deliver
input_action_end_dx4_finish
input_action_end_dx4
The relevant End.DT4 call path is:
__ip_options_echo
tcp_v4_route_req
tcp_conn_request
tcp_v4_conn_request
tcp_rcv_state_process
tcp_v4_do_rcv
tcp_v4_rcv
ip_protocol_deliver_rcu
ip_local_deliver_finish
ip_local_deliver
input_action_end_dt4
tcp_v4_save_options() is inlined into the tcp_v4_route_req() path, so
it does not appear as a separate frame.
When decap_and_validate() handles IPPROTO_IPIP, save the ingress
interface from IP6CB, clear IPCB, and restore the saved value. Doing
this in the common decapsulation path covers End.DX4, End.DT4, and
End.DT46's IPv4 arm.
Use IP6CB(skb)->iif rather than skb->skb_iif. These actions run after
l3mdev processing, which can replace skb_iif with the L3 master;
IP6CB iif still records the receiving interface set at IPv6 ingress.
Fixes: 891ef8dd2a ("ipv6: sr: implement additional seg6local actions")
Cc: stable@vger.kernel.org
Suggested-by: Andrea Mayer <andrea.mayer@uniroma2.it>
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Co-developed-by: David Lee <david.lee@trailofbits.com>
Signed-off-by: David Lee <david.lee@trailofbits.com>
Reviewed-by: Andrea Mayer <andrea.mayer@uniroma2.it>
Link: https://patch.msgid.link/20260817085839.946321-1-david.lee@trailofbits.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The inetpeer rate limiting system stores peer entries in a Red-Black tree
keyed deterministically on the remote IP address. Because tree lookups walk
the RB-tree using standard lexicographical comparisons (inetpeer_addr_cmp),
an off-path adversary can predict the exact topology of the tree and the
sequence of nodes traversed during lookups (the gc_stack candidate list).
By combining deterministic tree traversal with aggressive garbage collection
(triggered when tree size exceeds inet_peer_threshold), an attacker can
selectively force the eviction of targeted inet_peer nodes. When an evicted
node is subsequently re-created upon receiving a new packet, its rate-limiting
token bucket (rate_tokens, rate_last) is reset to full capacity. This creates
a side-channel primitive allowing off-path attackers to bypass IP-keyed ICMP
rate limits and infer open UDP ports (similar to SAD DNS style attacks).
Mitigate this by randomizing the RB-tree node comparison logic using SipHash
with a secret key (inetpeer_hash_key) initialized via net_get_random_once().
Nodes are ordered in the tree by SipHash(addr, key) rather than raw IP
addresses. Because the secret key is unknown to external entities, the tree
layout and lookup traversal paths are unpredictable to off-path adversaries,
breaking the deterministic eviction gadget.
Cache the computed 64-bit SipHash (hash) in struct inet_peer and compute the
target hash (dhash) once at the beginning of inet_getpeer() to avoid recomputing
SipHash at every step of the RB-tree walk.
Fixes: b145425f26 ("inetpeer: remove AVL implementation in favor of RB tree")
Reported-by: Michael Blunt <michaelbblunt@gmail.com>
Suggested-by: Michael Blunt <michaelbblunt@gmail.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20260818151213.3953963-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
IPv6 input attaches a non-refcounted (NOREF) dst to skbs under RCU.
When an ingress multicast packet misses MFC lookup,
ip6mr_cache_unresolved() places the skb onto the unresolved queue,
escaping the receive-side RCU grace period.
If the underlying route is deleted and freed, and the MFC queue is later
resolved with a wrong parent interface, ip6_mr_forward() invokes
ip6mr_cache_report(..., MRT6MSG_WRONGMIF), which executes
dst_clone(skb_dst(pkt)) on the freed dst entry, triggering a slab
use-after-free.
Report packets queued to mroute6_sk (a raw socket) and netlink
notifications do not require an attached dst entry.
Fix this by:
1. Removing dst_clone() in ip6mr_cache_report() and ensuring report skbs
do not hold a dst.
2. Dropping skb_dst before queuing unresolved skbs in
ip6mr_cache_unresolved(), matching the fact that multicast
forwarding resolves outgoing routes anew via ip6_route_output().
Fixes: 67f415dd29 ("ipv6: convert rx data path to not take refcnt on dst")
Reported-by: Zero Day Initiative <zdi-disclosures@trendmicro.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>
Link: https://patch.msgid.link/20260818172755.4083692-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
mptcp_token_join_cookie_init_state() restores remote_nonce, local_nonce,
backup, join_id, token and msk from the saved cookie entry when rebuilding
the request socket for a MP_JOIN 4th-ACK handled under SYN cookies, but it
does not restore local_id, even though the SYN path saved it.
subflow_ulp_clone() then reads that uninitialized field and stores it as
the joined subflow's address-ID. Because the request-sock slab is
SLAB_TYPESAFE_BY_RCU and not zeroed on allocation, the value is the stale
byte of a previously freed request socket, which an off-path peer can
influence by sending concurrent MP_JOIN SYNs. This corrupts the path
manager's id-based subflow bookkeeping for the connection.
Restore subflow_req->local_id from the cookie entry, as done for the other
fields.
Fixes: 9466a1cceb ("mptcp: enable JOIN requests even if cookies are in use")
Cc: stable@vger.kernel.org
Signed-off-by: Harshit Varu <harshitvaru666@gmail.com>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Link: https://patch.msgid.link/20260815115205.197151-1-harshitvaru666@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The unified ROM parser reads directory, product, and data-descriptor fields
from the firmware file. Existing validation forms table and data ends with
unchecked additions and multiplications. Malformed values can wrap before
they are compared with the firmware size. The parser also dereferences
typed pointers at firmware-controlled offsets.
Valid descriptor extents alone are insufficient for the consumers. The
loader reads a fixed-size bootloader regardless of its declared size, the
version parser assumes a 17-byte tail, and a partial final firmware word is
read as a full u64. A truncated image can therefore make the driver read
beyond the firmware allocation during validation or loading.
Replace the pointer-returning parser with bounded range helpers. Validate
table entry sizes, descriptor indices, section ranges, the fixed
bootloader load length, and the version tail before exposing any section.
Read all file fields with unaligned little-endian accessors and assemble a
partial final word from only the bytes that remain. Apply the same range
checks to the legacy image before reading its fixed fields.
Fixes: af19b49152 ("qlcnic: Qlogic ethernet driver for CNA devices")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260816052109.4607-1-pengpeng@iscas.ac.cn
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
I found that trying to read /sys/kernel/debug/ref_tracker/* causes NULL
pointer dereference crash when alloc_netdev_mqs() via unshare() returned
NULL, for commit 9ba74e6c9e ("net: add networking namespace refcount
tracker") added ref_tracker_dir_exit(&net->refcnt_tracker) to only
__put_net() path whereas commit 65b584f536 ("ref_tracker: automatically
register a file in debugfs for a ref_tracker_dir") added
ref_tracker_dir_debugfs() to ref_tracker_dir_init() path.
Since preinit_net() calls ref_tracker_dir_init(&net->refcnt_tracker) and
ref_tracker_dir_init(&net->notrefcnt_tracker), we need to make sure that
both ref_tracker_dir_exit(&net->refcnt_tracker) and
ref_tracker_dir_exit(&net->notrefcnt_tracker) are called before
net_passive_dec() schedules for kmem_cache_free() via net_complete_free().
ref_tracker_dir_exit(&net->refcnt_tracker) is called via put_net() when
ns_ref_put() returned true. But put_net() is not called when copy_net_ns()
fails. Therefore, call ref_tracker_dir_exit() from net_passive_dec() if
put_net() is not yet called.
Link: https://sashiko.dev/#/patchset/b06ce35d-e7bc-47a5-8e0a-e82be7e4dd08%40I-love.SAKURA.ne.jp
Fixes: 9ba74e6c9e ("net: add networking namespace refcount tracker")
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Link: https://patch.msgid.link/64254d80-9248-466c-8108-95f43bd71117@I-love.SAKURA.ne.jp
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
bnx2x_init_firmware() frees bp->init_ops, bp->init_data and
bp->init_ops_offsets in its error path without setting them to NULL.
The cleanup function bnx2x_release_firmware() frees the same three
pointers unconditionally, so if init_firmware fails and
release_firmware is later called (e.g. from __bnx2x_remove or through
the function state machine), all three are freed a second time.
Set each pointer to NULL after kfree() in the error path so that the
subsequent kfree(NULL) in bnx2x_release_firmware() is a safe no-op.
Fixes: 94a78b79cb ("bnx2x: Separated FW from the source.")
Cc: stable@vger.kernel.org
Signed-off-by: Jiangshan Yi <yijiangshan@kylinos.cn>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260815122149.951215-1-yijiangshan@kylinos.cn
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
rt6_multipath_rebalance() calculates the total eligible nexthop weight
in one pass and programs upper bounds in a second pass. Since
RTM_NEWROUTE is RTNL-free, a concurrent
ignore_routes_with_linkdown update can make the first pass return zero
while the second sees an eligible nexthop, causing
rt6_upper_bound_set() to divide by zero.
UBSAN: division-overflow in net/ipv6/route.c:4845:17
Oops: divide error: 0000 [#1] SMP KASAN NOPTI
rt6_upper_bound_set() net/ipv6/route.c:4845
rt6_multipath_rebalance()
fib6_add_rt2node()
ip6_route_multipath_add()
inet6_rtm_newroute()
Skip upper-bound calculation when the first pass reports a zero total.
This respects the lock-free performance considerations here and solves
insecure scenarios.
Fixes: bd11ff421d ("ipv6: Get rid of RTNL for SIOCDELRT and RTM_DELROUTE.")
Reported-by: AutonomousCodeSecurity@microsoft.com
Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Reported-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Link: https://patch.msgid.link/20260817013237.2797-1-blbllhy@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
In netdevsim, receive queues (struct nsim_rq) embed their own struct
napi_struct. When queue reset is performed (e.g. via queue_reset
debugfs), nsim_queue_start() swaps in a newly allocated struct nsim_rq,
and nsim_queue_mem_free() later deletes and frees the old one.
However, nsim_queue_start() failed to update the queue-to-NAPI mapping
via netif_queue_set_napi(). As a result, dev->_rx[idx].napi continued to
point to the old NAPI struct. After the old queue was freed, a subsequent
queue dump via Netlink (NETDEV_CMD_QUEUE_GET) triggered a KASAN
slab-use-after-free read in nla_put_napi_id() when accessing
rxq->napi->napi_id.
Fix this by calling netif_queue_set_napi() in nsim_queue_start() to
associate the new NAPI with the RX queue, and clear the association
with netif_queue_set_napi(..., NULL) in nsim_del_napi() during teardown.
Fixes: 5bc8e8dbef ("netdevsim: add queue management API support")
Reported-by: syzbot+483a6efbc4882c1201ee@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6a82c3d4.f7a79266.2f965f.0024.GAE@google.com/T/#u
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Link: https://patch.msgid.link/20260817082511.2300402-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
mctp_route_lookup() uses rt->dev without holding a reference on it.
mctp_route_lookup_single() returns the route under RCU only, so the
route's device can be torn down concurrently: mctp_dev_put() drops the
last reference and synchronously kfree()s mdev->addrs. mctp_dev_saddr()
then reads rt->dev->addrs[0], giving a use-after-free reachable by an
unprivileged local AF_MCTP user on the receive/forwarding path (no
CAP_NET_RAW required):
BUG: KASAN: slab-use-after-free in mctp_route_lookup
Read of size 1 at addr ... by task mctp_uaf/...
mctp_route_lookup
mctp_pkttype_receive
Freed by task ...:
kfree
mctp_dev_put
mctp_dev_notify
In the same window mctp_dst_from_route() -> mctp_dev_hold() also
increments a refcount that has already reached zero
("refcount_t: addition on 0 ... mctp_dev_hold").
This reintroduces the use-after-free class of CVE-2023-3439: the source
address lookup was moved ahead of the point where the destination takes
its device reference.
Take a reference with refcount_inc_not_zero() before touching rt->dev,
skip a device that is already dead, and drop the reference once the
destination has taken its own.
Fixes: 22cb45afd2 ("net: mctp: perform source address lookups when we populate our dst")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
Link: https://patch.msgid.link/20260813022102.2792032-1-qwe.aldo@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
ipa_remove() takes a runtime PM reference before accessing IPA hardware
during teardown. If a concurrent modem start or stop keeps
ipa_modem_stop() busy across both attempts, the callback intentionally
returns without releasing the remaining resources because proceeding
with teardown could crash. That return also skips the matching
pm_runtime_put_noidle(), leaving the callback's usage-count reference
held.
Drop only this runtime PM reference before returning.
pm_runtime_put_noidle() does not request an idle transition, so the
hardware and resources retained on this exceptional path remain
untouched while the usage count stays balanced.
This issue was found by a static analysis checker and confirmed by
manual source review.
Fixes: 923a6b6984 ("net: ipa: get clock in ipa_probe()")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
Reviewed-by: Alex Elder <elder@riscstar.com>
Link: https://patch.msgid.link/20260815151737.3758320-1-ruoyuw560@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Marek Czernohous says:
====================
forcedeth: two register-window bounds fixes
Two bounds fixes in forcedeth, both in the same shape: a loop that walks
the register window one step too far. They are independent of each other
and touch different functions.
1/2 nv_suspend() and nv_resume() save and restore the non-PCI config
space with i <= register_size/sizeof(u32). On a VER3 device that is
exactly the length of saved_config_space[], so the last iteration
reads and writes one element past the array, and on resume it
writel()s that element one dword past the length the driver mapped.
UBSAN catches it.
2/2 nv_tx_timeout() dumps the window in rows of eight dwords but only
bounds the row's starting offset, so the final row reads between 12
and 28 bytes past register_size, on every one of the three supported
window sizes.
Neither is a regression. Both are long standing, and 1/2 in particular
is not new to the list:
- The identical off-by-one in nv_get_regs() was fixed by commit
ba9aa13428 ("forcedeth: fix buffer overflow") in 2012. The two
loops in this patch were missed at the time.
- The suspend and resume side was then reported on LKML in September
2013 by Marc Weber, with the same analysis and the same
one-character fix. Sergei Shtylyov replied asking for the patch
inline rather than attached, and the thread ended there.
So this is not a new discovery. It is the same bug at the two sites the
2012 fix did not reach, finally sent in the form the list asks for.
How bad is it, stated plainly
1/2 writes one u32 past the end of a declared array, on a suspend
path, on every suspend of a VER3 device. That is an out-of-bounds
store, it is what UBSAN reports, and with CONFIG_UBSAN_TRAP=y it is a
trap that aborts the running kernel code. That is the stable case, and
I think it stands on its own: memory safety, reproduced on hardware,
one character to fix, no behavioural change for anyone else.
What I will not claim is drama beyond that. The element it lands in is
np->name_rx, a scratch string that nv_request_irq() rewrites with
sprintf() before it is ever used, so on a kernel without UBSAN_TRAP
nothing observable is corrupted. The patch says which member and why,
so you can judge the severity yourself instead of taking my word.
The MMIO side of both patches is milder still. ioremap() rounds the
requested length up to page granularity, so these accesses stay inside
the page the CPU has mapped and no fault is expected on any
architecture with PAGE_SIZE >= 4K. What they leave is the window the
driver asked for. 2/2 is only that, and carries no stable tag.
Behaviour change in 2/2, so it is not buried in the patch
The partial trailing row of the debug dump is no longer printed: 16
bytes for VER1, 20 for VER2, 4 for VER3. That is a deliberate trade
against open-coding a second, narrower dump in a debug-only path. If
you would rather keep those registers, a short remainder loop on top
is the obvious follow-up.
Testing
Reference hardware: Apple Macmini3,1 (MCP79 chipset), forcedeth
driving the onboard NIC.
1/2 is reproduced and fixed on that machine. One point of method
first: UBSAN reports each source location only once per module load,
so a quiet second suspend proves nothing. Both runs below are the
first S3 cycle after a fresh load of the module in question.
stock module, first S3 after load: 2 splats, one per loop
patched module, first S3 after load: none
The patched module was built, stripped, installed and reloaded, with
the md5 of the running module checked against the installed one. The
link came back, the DHCP lease was restored and ping showed no loss.
That measurement was taken on 2026-08-04 on a 7.1.6 based kernel. The
stock half has since been reproduced again on 7.1.8, most recently on
2026-08-13, reporting line 6225 from pci_pm_suspend and line 6240
from pci_pm_resume.
I have not repeated the patched half on net/main itself. The runtime
measurements come from a distro kernel on the reference hardware,
which is the only machine I have with this NIC; the series itself is
based on and built against net/main.
2/2 has no runtime test. Its path sits behind the debug_tx_timeout
module parameter and needs a genuine TX timeout, which I cannot force
safely on this machine. It rests on the arithmetic in the patch and
on the build below.
Build: allmodconfig with W=1 on x86_64, whole tree, zero compiler
warnings and zero errors; forcedeth.c specifically produces none.
That took about 30 hours on the two cores I have, which is why I say
it plainly rather than in passing.
I have not run allyesconfig. If you want that too, say so and I will
queue it before reposting rather than claim a build I did not do.
Two checkpatch notes on 1/2, both deliberate
"Prefer a maximum 75 chars per line" fires on a line that is quoted
UBSAN output. The splat is trimmed, and 1/2 says what was cut, but I
did not rewrap the lines that remain: reflowing diagnostic output to
satisfy a heuristic makes it harder to match against a real log.
Two "spaces preferred around that '/'" CHECKs fire on
register_size/sizeof(u32). That spacing is what the file already uses,
including in nv_get_regs(), which is otherwise the same loop. Adding
spaces would leave the two lines I touch inconsistent with their
neighbourhood, so I kept the change to the one character that is
wrong. Happy to do it the other way round if you prefer.
AI assistance
Per Documentation/process/coding-assistants.rst: this work is AI
assisted. I use Claude (claude-opus-5) as a coding and analysis
assistant. Both patches carry an Assisted-by trailer accordingly, and
no Signed-off-by is added by the tool.
Nature of the assistance: the assistant did the code archaeology and
most of the drafting. I described the symptom, asked for the mechanism
to be traced in the source rather than guessed, and asked for each
claim to be backed by a file and a line. The UBSAN output and the S3
measurements are from the machine, not model output.
It is also what found the 2012 fix and the 2013 report above, on a
second pass over an earlier draft of this posting that claimed the bug
had never been reported. That claim was wrong and would have wasted
your time, so it seems worth saying that the checking pass is part of
the process here and not a flourish. I reviewed the result, I
understand the code, and I take responsibility for it.
====================
Link: https://patch.msgid.link/178682367884.3748309.5288746298966501007@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
nv_tx_timeout() dumps the register window in rows of eight dwords:
for (i = 0; i <= np->register_size; i += 32) {
netdev_info(dev, "%3x: %08x ... %08x\n", i,
readl(base + i + 0), ..., readl(base + i + 28));
The loop bound only checks the row's starting offset, so the final row
reads a full 32 bytes from a position that is below the end of the window
but too close to it. base is mapped with exactly that length:
np->base = ioremap(addr, np->register_size);
so the tail of that row is read from beyond the length the driver asked
for. Per variant, the last iteration reads past register_size by:
NV_PCI_REGSZ_VER1 (0x270): row 0x260 reads to 0x27f, 16 bytes over
NV_PCI_REGSZ_VER2 (0x2d4): row 0x2c0 reads to 0x2df, 12 bytes over
NV_PCI_REGSZ_VER3 (0x604): row 0x600 reads to 0x61f, 28 bytes over
This happens on every supported device, not just one of them. Note that
it is not a consequence of the sizes being odd: with i <= register_size
the offending row is reached whatever the size, and a size that were a
multiple of 32 would overrun by a full row rather than by a remainder.
To be precise about the severity: the reads stay inside the BAR. Memory
BAR sizes are powers of two, the driver only accepts a region with
pci_resource_len() >= register_size (forcedeth.c:5757-5762), and the
next power of two at or above each register_size already covers the
offending row: 0x400 for 0x270 and 0x2d4, 0x800 for 0x604. ioremap()
also rounds the mapped length up to page granularity, so the reads land
inside the mapping the CPU has as well. What they leave is the window
the driver asked for, not the BAR and not the mapping. That is still a
driver reading registers it did not ask for, and it is trivial to
avoid, but nobody should expect a fault from it.
Changing <= to < is not enough: register_size is a length and every size
above is larger than its last row start, so i still reaches the offending
row. Check that the whole row fits instead.
The trade-off is that a partial trailing row is no longer dumped: 16 bytes
for VER1, 20 for VER2, 4 for VER3. That seemed preferable to reading
outside the requested window, and to open-coding a second, narrower dump
for the remainder in what is a debug-only path. Extending the dump to
cover the tail can be done on top if anyone misses those registers.
Only reachable with the debug_tx_timeout module parameter, which defaults
to false. It has not been observed at runtime: forcing a genuine TX
timeout on the reference machine is not something I can do safely, so this
rests on the arithmetic above and on a build test, not on a reproduction.
UBSAN does not catch it either, since these are MMIO reads rather than an
array access. It was found by reading the function while fixing the
saved_config_space off-by-one in nv_suspend() and nv_resume().
The dump was introduced with a fixed 0x400 bound while ioremap() mapped
only NV_PCI_REGSZ (0x270), so it read about 0x190 bytes too far from the
start. Commit 86a0f04387 ("[PATCH] forcedeth: fix initialization")
later replaced 0x400 with np->register_size, which shrank the overrun to
the remainder but did not remove it.
Fixes: c2dba06dae ("[PATCH] forcedeth: rewritten tx irq handling")
Signed-off-by: Marek Czernohous <marek@czernohous.de>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev>
Link: https://patch.msgid.link/178682367886.3748309.6978554332066826294@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
nv_suspend() and nv_resume() walk the non-PCI configuration space with
for (i = 0; i <= np->register_size/sizeof(u32); i++)
which runs one iteration too many. saved_config_space is declared as
u32 saved_config_space[NV_PCI_REGSZ_MAX/4];
and NV_PCI_REGSZ_VER3 is equal to NV_PCI_REGSZ_MAX (0x604), so on a VER3
device register_size/sizeof(u32) is exactly the array length and the last
iteration addresses one element past the end.
The element it lands on is np->name_rx[0..3]: saved_config_space[] is
followed immediately by char name_rx[IFNAMSIZ + 3], and char needs no
padding. Nothing observable is corrupted by that, because nv_request_irq()
rewrites name_rx with sprintf() before it is ever passed to request_irq().
The bug is the out-of-bounds access itself, which UBSAN reports and which
CONFIG_UBSAN_TRAP=y turns into a trap that aborts the running kernel code,
plus an MMIO read and, on resume, an MMIO writel() to base + 0x604, one
dword past the range the driver mapped:
np->base = ioremap(addr, np->register_size);
VER1 and VER2 devices stay inside the array, but they too get the stray
read and the stray write one dword past their own window.
Caught by UBSAN on an Apple Macmini3,1 (MCP79) during a deep S3 cycle.
The splat below is trimmed: the build path in the file name, the CPU
and taint lines, the Workqueue line, the "?" hint frames, and the
frames below device_suspend are all cut. The kernel was tainted, with
an out-of-tree nouveau and CPU_OUT_OF_SPEC; forcedeth itself was the
stock module.
UBSAN: array-index-out-of-bounds in drivers/net/ethernet/nvidia/forcedeth.c:6225:25
index 385 is out of range for type 'u32 [385]'
Call Trace:
dump_stack_lvl+0x5d/0x80
ubsan_epilogue+0x5/0x2b
__ubsan_handle_out_of_bounds.cold+0x54/0x59
__this_module+0xe398c/0xe9010 [forcedeth]
pci_pm_suspend+0x80/0x170
dpm_run_callback+0x51/0x160
device_suspend+0x1a2/0x4a0
...
Both loops are hit. UBSAN reports each source location only once per module
load (__ubsan_handle_out_of_bounds() calls suppress_report(), which does
test_and_set_bit(REPORTED_BIT, ...) on the struct source_location), so the
two splats land in the first S3 cycle after the module is loaded and later
cycles are silent even though the access still runs off the end every time.
In that first cycle line 6225 is reported from pci_pm_suspend and line 6240
from pci_pm_resume.
The same off-by-one was fixed in nv_get_regs() by commit ba9aa13428
("forcedeth: fix buffer overflow") in 2012; these two loops were missed.
The suspend and resume side was reported on LKML in September 2013 by Marc
Weber, with the same analysis and the same one-character fix, but the patch
was attached rather than sent inline and the thread ended there.
Use < instead of <=, which saves and restores exactly register_size bytes.
Fixes: 1a1ca86158 ("[netdrvr] forcedeth: save/restore device configuration space")
Cc: stable@vger.kernel.org
Signed-off-by: Marek Czernohous <marek@czernohous.de>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Zhu Yanjun <yanjun.zhu@linux.dev>
Link: https://patch.msgid.link/178682367885.3748309.10595890901761762683@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Tariq Toukan says:
====================
net/mlx5: Preserve speed and state across vport modify commands
The firmware vport modify command bundles both admin state and max tx
speed in a single operation, which requires each side to preserve the
other field when it only intends to change one.
When modifying max tx speed, the driver already queries the current
admin state and passes it back to avoid overwriting it. However, this
query and the subsequent modify were not atomic, a state change
between the two could cause the modify to overwrite the new state with
a stale value. The fix holds esw->state_lock across the query-modify
sequence.
When support for setting max tx speed via the vport modify command was
introduced, the existing admin state modify path was not updated to
preserve the current speed. As a result, the firmware interprets the
zero speed field as an intentional reset. The fix adds a speed query
before the state modify and passes the result back in the command.
To support that, mlx5_query_vport_max_tx_speed() had to be fixed first:
it was returning zero whenever the vport was DOWN, which was correct
for the query_port_speed verb but would defeat the purpose of querying
before a state modify. The DOWN-to-zero logic is moved to the
verb-layer caller so the function returns the raw firmware value.
Patch #1 holds esw->state_lock across the state query and modify in
the speed modify path
Patch #2 moves the vport DOWN zero mapping to the verb-layer caller
so the query returns the raw firmware value
Patch #3 queries current max tx speed before modifying vport state to
preserve it
====================
Link: https://patch.msgid.link/20260816065015.3280733-1-tariqt@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When modifying vport state, the firmware interprets a zero in the max tx
speed field as an intentional reset, which can overwrite previously set
values. This patch attempts to fix this by querying the current max tx
speed from firmware before modifying the vport state and passing it back
in the modification command. If the query fails, fall back to the cached
agg_max_tx_speed value to avoid inadvertently resetting the speed.
Fixes: 50f1d188c5 ("net/mlx5: Propagate LAG effective max_tx_speed to vports")
Signed-off-by: Or Har-Toov <ohartoov@nvidia.com>
Reviewed-by: Mark Bloch <mbloch@nvidia.com>
Reviewed-by: Shay Drori <shayd@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260816065015.3280733-4-tariqt@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
mlx5_query_vport_max_tx_speed() was introduced to serve the
query_port_speed path, which uses max_tx_speed == 0 when port is down.
This is incorrect for callers that need the actual configured speed
regardless of vport state, such as modify-vport-state helpers
that must preserve the speed across state transitions.
Move this logic to the caller function in the verb flow and let
mlx5_query_vport_max_tx_speed() return the raw firmware value
unconditionally.
Fixes: aaecff5e13 ("RDMA/mlx5: Implement query_port_speed callback")
Signed-off-by: Or Har-Toov <ohartoov@nvidia.com>
Reviewed-by: Shay Drori <shayd@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260816065015.3280733-3-tariqt@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Protect vport admin state modifications and vport iteration with the
eswitch state_lock mutex to ensure proper serialization of concurrent
vport state changes.
Currently, calls to mlx5_modify_vport_admin_state() and loops iterating
over eswitch vports can race with each other, potentially leading to
inconsistent vport state. Fix this by acquiring esw->state_lock
Fixes: 7d0314b11c ("net/mlx5e: Modify uplink state on interface up/down")
Signed-off-by: Mark Bloch <mbloch@nvidia.com>
Reviewed-by: Shay Drori <shayd@nvidia.com>
Reviewed-by: Or Har-Toov <ohartoov@nvidia.com>
Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260816065015.3280733-2-tariqt@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
