| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
fib_nhc_update_mtu() walks the nexthop exception table under RTNL, but
RTNL does not serialize this walk with PMTU exception updates. The walk
uses rcu_dereference_protected() with a constant true condition without
holding fnhe_lock.
The following interleaving can therefore occur:
CPU 0 CPU 1
fib_nhc_update_mtu() update_or_create_fnhe()
load fnhe spin_lock_bh(&fnhe_lock)
fnhe_remove_oldest()
unlink fnhe
kfree_rcu(fnhe, rcu)
<quiescent state>
access fnhe after grace period
KASAN reported:
BUG: KASAN: slab-use-after-free in fib_nhc_update_mtu+0x3df/0x410
Read of size 8 at addr ffff888107d49000 by task poc/90
Call Trace:
fib_nhc_update_mtu+0x3df/0x410
fib_sync_mtu+0x7a/0xd0
fib_netdev_event+0x229/0x3f0
netif_set_mtu_ext+0x33a/0x570
dev_set_mtu+0x88/0x120
The same walk updates fnhe_pmtu and fnhe_mtu_locked. These fields form a
pair and other writers serialize them with fnhe_lock. RCU alone prevents
reclamation, but would still allow concurrent writers to leave a mixed
pair.
Walk the table under RCU and acquire fnhe_lock only while updating each
exception. RCU keeps the current entry alive while the short critical
section serializes its paired PMTU fields. This avoids holding the global
lock while scanning all 2048 buckets for every nexthop.
Fixes: af7d6cce5369 ("net: ipv4: update fnhe_pmtu when first hop's MTU changes")
Cc: stable@vger.kernel.org
Suggested-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Link: https://patch.msgid.link/20260807181710.1178747-1-nicoyip.dev@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
sctp_auth_calculate_hmac() can fail when building the association secret
under memory pressure, but its void return silently leaves the HMAC digest
zeroed. On the receive path, sctp_sf_authenticate() compares this zeroed
digest against the peer-supplied one using crypto_memneq(), potentially
accepting an all-zero HMAC from the peer if the allocation failed. On the
send path, sctp_packet_pack() transmits a packet with a zeroed HMAC that
the peer would reject.
Improve error handling by making sctp_auth_calculate_hmac() return int:
- sctp_sf_authenticate() returns SCTP_IERROR_NOMEM instead of accepting
a zero HMAC.
- sctp_packet_pack() drops the packet on failure instead of transmitting
a zeroed HMAC.
Update the declaration in auth.h accordingly.
Assisted-by: LLM
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
Acked-by: Xin Long <lucien.xin@gmail.com>
Link: https://patch.msgid.link/20260807064314.500742-1-l1138897701@163.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Make sure we check if nla_put_u32(id) was successful after creating
objects. This is theoretical today, the skbs are large enough to
always fit the ID.
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Nikolay Aleksandrov <razor@blackwall.org>
Reviewed-by: Joe Damato <joe@dama.to>
Link: https://patch.msgid.link/20260809000609.327659-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
tcf_ct_handle_fragments() runs its header sanity checks before handing
anything to the defragmentation engine:
if (family == NFPROTO_IPV4)
err = tcf_ct_ipv4_is_fragment(skb, &frag);
else
err = tcf_ct_ipv6_is_fragment(skb, &frag);
if (err || !frag)
return err;
tcf_ct_ipv4_is_fragment() returns -EINVAL or -ENOMEM;
tcf_ct_ipv6_is_fragment() adds -EPROTO when ipv6_find_hdr() fails. None of
them frees or queues the skb, so on that path the caller still owns it.
tcf_ct_act() however funnels every non-zero return into the
ownership-transfer exit:
err = tcf_ct_handle_fragments(net, skb, family, p->zone, &defrag);
if (err)
goto out_frag;
...
out_frag:
if (err != -EINPROGRESS)
tcf_action_inc_drop_qstats(&c->common);
return TC_ACT_CONSUMED;
TC_ACT_CONSUMED means the action took ownership of the skb, so no caller
frees it - sch_handle_ingress(), sch_handle_egress() and
tcf_qevent_handle() all deliberately skip the free for that verdict. The
skb is therefore orphaned: one sk_buff plus its data buffer is leaked per
malformed packet, unbounded. Note the drop counter is already incremented
for these errors, so the statistics claim a drop that never happens.
Three different ownership states reach out_frag: today - the skb may be
queued by the defrag engine (-EINPROGRESS), already freed by
nf_ct_handle_fragments(), or still owned by us. Tell the caller which of
those it is, and free the packet ourselves in the last case, which
restores the TC_ACT_SHOT behaviour that predated the Fixes: commit.
Reproduced on v7.2-rc6 with a 54-byte frame carrying a 40-byte IPv6
header with nexthdr = 0 (hop-by-hop) and nothing after it, on a
clsact ingress chain with "action ct". kmemleak reports one leaked
232-byte skbuff_head_cache object plus its 704-byte data buffer per
packet; with this patch it reports none.
Fixes: 3f14b377d01d ("net/sched: act_ct: fix skb leak and crash on ooo frags")
Cc: stable@vger.kernel.org # v6.8+
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Link: https://patch.msgid.link/20260806101235.809370-1-hj351016@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
upayload->data includes an extra NUL terminator even though it is never
used as a C string. In-tree users access only the first
upayload->datalen bytes.
Remove the redundant NUL terminator and allocate one byte less for
upayload->data in dns_resolver_preparse().
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260806200454.245444-3-thorsten.blum@linux.dev
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
On cross-region connections we observed delayed ACKs suddenly turning
into immediate ACKs plus a TCP_MAX_QUICKACKS burst, as if the
connection had just received its first data segment.
Commit 95b9a87c6a6b ("tcp: record last received ipv6 flowlabel")
squeezed icsk_ack.ato into 8 bits, sized for TCP_DELACK_MAX. But both
writers still bound ato by icsk_rto, which can be well above 255
jiffies, so the bitfield assignment silently wraps mod 256: repeated
delack timer misses double ato up to icsk_rto, storing 320 as 64 and
256 as 0, and ato == 0 is the "first data packet" sentinel in
tcp_event_data_recv().
Clamp both writers to TCP_DELACK_MAX, which the static_assert already
guarantees to fit and tcp_send_delayed_ack() effectively caps ato at
anyway.
Fixes: 95b9a87c6a6b ("tcp: record last received ipv6 flowlabel")
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Reviewed-by: Neal Cardwell <ncardwell@google.com>
Link: https://patch.msgid.link/20260807014437.36687-1-jiayuan.chen@linux.dev
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
tcf_action_check_ctrlact() range checks the primary control action:
if (!opcode)
ret = action > TC_ACT_VALUE_MAX ? -EINVAL : 0;
TC_ACT_VALUE_MAX is TC_ACT_TRAP, so kernel-internal verdicts above it
cannot be set that way. But act_gact and act_police each carry a second,
independent control action supplied by user space that never reaches that
helper - TCA_GACT_PROB.paction and TCA_POLICE_RESULT. Both only reject
TC_ACT_GOTO_CHAIN, so any other value is stored verbatim and returned
verbatim from the action.
In particular user space can store TC_ACT_CONSUMED, which is
TC_ACT_VALUE_MAX + 1 and is deliberately not part of the UAPI value
range. That verdict tells every caller the action took ownership of the
skb, so nobody frees it: sch_handle_ingress(), sch_handle_egress() and
tcf_qevent_handle() all deliberately skip the free for it. The result is
one leaked sk_buff plus its data buffer per packet traversing the filter,
unbounded, for all traffic on the chain including kernel-generated
packets.
Both are trivially deterministic. act_gact clamps tcfg_pval to >= 1, so
with pval = 1 gact_determ() returns the fallback for every packet.
act_police has no mandatory rate, so rate = 0 leaves tcfp_mtu = ~0 and
tcf_police_mtu_check() always passes.
TC_ACT_CONSUMED was added by commit 720f22fed81b ("net: sched: refactor
reinsert action"), after both goto-chain guards were written:
commit 9469f375ab09 ("net/sched: act_gact: disallow 'goto chain' on
fallback control action") and
commit c08f5ed5d625 ("net/sched: act_police: disallow 'goto chain' on
fallback control action"). Neither guard was widened when the new
verdict appeared.
Factor the existing range test out of tcf_action_check_ctrlact() as
tcf_action_valid() and apply it to both fallbacks. The helper cannot call
tcf_action_check_ctrlact() directly because that also allocates a
goto_chain, which is exactly what these two sites must not do.
Reproduced on v7.2-rc6: kmemleak reports one leaked 232-byte
skbuff_head_cache object plus its 704-byte data buffer per packet. With
this patch both configurations are rejected with -EINVAL and kmemleak
reports none.
Fixes: 720f22fed81b ("net: sched: refactor reinsert action")
Cc: stable@vger.kernel.org # v5.3+
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Tested-by: Victor Nogueira <victor@mojatatu.com>
Link: https://patch.msgid.link/20260806101252.809593-1-hj351016@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
netdev_queue_get_dma_dev() uses "compat" locking assert which wants
either the rtnl_lock or netdev instance lock. This is not right,
the callers are taking the instance lock unconditionally. All entry
points for queue config are purely instance locked.
In other words the callers use netdev_get_by_index_lock(), not
netdev_get_by_index_lock_ops_compat(). All the state we will
access is effectively instance lock protected (it's const for
devices which are not ops-locked).
Update the assert to avoid false positive warnings.
Cc: stable@vger.kernel.org
Fixes: b6c5f9454ef34 ("io_uring/zcrx: call netdev_queue_get_dma_dev() under instance lock")
Reported-by: syzbot+a78926bdac2adb52dc0e@syzkaller.appspotmail.com
Reviewed-by: Simon Horman <horms@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://patch.msgid.link/20260806225627.3998672-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
If the NF_CT_EXPECT_DEAD expectation flag is set on, bail out on
insertion. Moreover, add also DEBUG_NET_WARN_ON_ONCE() since this should
not ever happen. This is hardening commit b8b09dc2bf35 ("netfilter:
nf_conntrack_expect: use conntrack GC to reap expectations").
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
The existing check might extend the timeout if the ESTABLISHED timeout
has been tuned to be lower than UNACK via sysctl. Reported by sashiko.
Fixes: bf80e6802273 ("netfilter: conntrack: tcp: use UNACK timeout for non-closing RST packets")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
For bitmap this change makes no difference, because destructors are
called synchronously.
List type however calls them via call_rcu() so accounting decrement can
happen after list_set_flush() set ext_size to 0.
'set->elements = 0' can be removed for the same reason in the list type
case, it calls 'set->elements--' for each element.
Fixes: 9e41f26a505c ("netfilter: ipset: Count non-static extension memory for userspace")
Suggested-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
If list_set_uadd() calls list_set_replace() to swap an expired entry,
the element count remains the same, therefore the increment must be elided.
Fixes: 702b71e7c666 ("netfilter: ipset: Add element count to all set types header")
Link: https://sashiko.dev/#/patchset/20260806101947.2802-1-fw%40strlen.de
Signed-off-by: Florian Westphal <fw@strlen.de>
Acked-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
nf_flow_table_iterate() only treats original-direction tuple nodes as
owning entries. Publishing the original node first lets GC observe and
free a flow while flow_offload_add() is still inserting the reply node.
Publish the reply node first and the original node last so GC never
sees a partially installed flow.
KASAN can trigger slab-use-after-free read and write reports in the
flowtable/rhashtable path (rht_deferred_worker, jhash, flow_offload_del,
flow_offload_lookup, etc.).
Fixes: ac2a66665e23 ("netfilter: add generic flow table infrastructure")
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
Assisted-by: Codex:gpt-5
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
In nft_flow_rule_offload_abort(), WARN_ON_ONCE(err) is triggered on every
error during rollback, including -ENOMEM. Memory allocation failures are
expected under low-memory conditions and do not indicate a kernel bug.
Trace for example:
nft_flow_offload_chain() // FLOW_BLOCK_BIND
nft_flow_block_chain()
nft_chain_offload_cmd()
nft_block_offload_cmd()
->ndo_setup_tc()
nsim_setup_tc()
flow_block_cb_setup_simple()
flow_block_cb_alloc() // fails to -ENOMEM
The warning was reproduced on the 5.10 stable kernel under memory pressure
via fault injection, but the underlying bug exists in mainline as well,
as demonstrated by the ENOMEM trace above. The following splat was
triggered during nf_tables transaction processing:
WARNING: CPU: 0 PID: 8567 at net/netfilter/nf_tables_offload.c:532 nft_flow_rule_offload_abort net/netfilter/nf_tables_offload.c:532 [inline]
WARNING: CPU: 0 PID: 8567 at net/netfilter/nf_tables_offload.c:532 nft_flow_rule_offload_commit+0x971/0xcd0 net/netfilter/nf_tables_offload.c:591
Modules linked in:
CPU: 0 PID: 8567 Comm: syz-executor.0 Not tainted 5.10.260-syzkaller #0
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
RIP: 0010:nft_flow_rule_offload_abort net/netfilter/nf_tables_offload.c:532 [inline]
RIP: 0010:nft_flow_rule_offload_commit+0x971/0xcd0 net/netfilter/nf_tables_offload.c:591
Call Trace:
nf_tables_commit+0x3bd/0x4bd0 net/netfilter/nf_tables_api.c:8604
nfnetlink_rcv_batch+0xb1e/0x1f20 net/netfilter/nfnetlink.c:509
nfnetlink_rcv_skb_batch net/netfilter/nfnetlink.c:579 [inline]
nfnetlink_rcv+0x3b3/0x420 net/netfilter/nfnetlink.c:597
netlink_unicast_kernel net/netlink/af_netlink.c:1314 [inline]
netlink_unicast+0x6cd/0xa00 net/netfilter/af_netlink.c:1340
netlink_sendmsg+0x906/0xe10 net/netfilter/af_netlink.c:1919
sock_sendmsg_nosec net/socket.c:651 [inline]
__sock_sendmsg+0x155/0x190 net/socket.c:663
____sys_sendmsg+0x705/0x870 net/socket.c:2379
___sys_sendmsg+0x100/0x170 net/socket.c:2433
__sys_sendmsg+0xe9/0x1c0 net/socket.c:2462
do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46
entry_SYSCALL_64_after_hwframe+0x67/0xd1
Change the condition to WARN_ON_ONCE(err && err != -ENOMEM) so that
warnings are only emitted for unexpected errors. This aligns with the
common kernel practice of not warning on -ENOMEM.
Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
Fixes: 63b48c73ff56 ("netfilter: nf_tables_offload: undo updates if transaction fails")
Signed-off-by: Alexey Velichayshiy <a.velichayshiy@ispras.ru>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
While the outer IP header is already pulled into the skb head,
we must be careful and revalidate the embedded headers after
reading them from the skb frags to prevent out-of-bounds
access.
One such place reported by Sashiko is ip_vs_nat_icmp() where
local process can change the ihl field and after
skb_ensure_writable() we can see larger value which is a
problem for the ip_send_check(cih) calls.
Add check to drop the packet if the ihl field is changed.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Link: https://sashiko.dev/#/patchset/20260730183506.87473-1-ja%40ssi.bg
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
ip_vs_in_icmp() rebases an skb from the outer ICMP packet to the
quoted original request before passing it to icmp_send(). However,
IPCB(skb)->opt still describes the outer IPv4 header.
A timestamp option in the outer header can therefore leave an offset
that points into the quoted transport header after the rebase.
__ip_options_echo() treats a byte at that stale location as the option
length and copies it into the fixed-size option storage on the
__icmp_send() stack, causing a stack out-of-bounds write.
Clear the stale option metadata after resetting the network header.
Keep the remaining control block fields, including the ingress
interface used by the ICMP response path.
Fixes: f2edb9f7706d ("ipvs: implement passive PMTUD for IPIP packets")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6-sol Codex:gpt-5.5-cyber
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: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
sashiko reports: "nfnl_log_net_exit() calls nf_log_unset(), which
clears the logger pointer without an RCU grace period. Immediately after,
ops_free_list() frees the per-net state while concurrent packets might
still be executing nf_log_packet() under rcu_read_lock()."
Clear the pointer via .pre_exit to make sure rcu readers have completed
before pernet storage is free'd. The change in nf_log_syslog.c is only
done for consistency: it doesn't use pernet data.
Link: https://sashiko.dev/#/patchset/20260731151806.849724-1-pablo%40netfilter.org
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
TCP and SCTP conntrack paths can emit invalid-packet logs while ct->lock
is still held.
When invalid logging is routed to nfnetlink_log and conntrack export is
enabled, the log path can re-enter conntrack netlink glue and dump the
same conntrack again. Protocol attribute dumping may take ct->lock, so
logging while holding that lock can deadlock.
Defer the TCP invalid logs by storing only the minimal log context while
ct->lock is held and emitting the log after unlocking. Also make the TCP
timeout-lowering invalid path return whether a log is needed, then emit
that log after unlocking.
Do the same for the SCTP invalid state-transition log that can be reached
while ct->lock is held.
Add a lockdep assertion to nf_ct_l4proto_log_invalid() so future callers
that log invalid conntracks while holding ct->lock are caught outside TCP
and SCTP as well.
Fixes: 628d694344a0 ("netfilter: conntrack: reduce timeout when receiving out-of-window fin or rst")
Fixes: d9a6f0d0df18 ("netfilter: conntrack: prepare tcp_in_window for ternary return value")
Fixes: f71cb8f45d09 ("netfilter: conntrack: sctp: use nf log infrastructure for invalid packets")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
Reviewed-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
IPVS configuration paths update destination availability while connection
accounting updates destination overload state. The two independent states
share dest->flags, so their read-modify-write updates can race and lose one
another.
Keep OVERLOAD in flags, where the preceding patch serializes its updates
with dst_lock, and move AVAILABLE to cflags. This keeps configuration-
controlled availability out of the scheduler hot cacheline until a
scheduler needs to check it. It also prevents availability updates from
clobbering overload state.
The destination status bits are not exposed through the IPVS sockopt or
netlink interfaces, so keep their definitions in the internal IPVS header.
Readers can still observe stale destination state; this does not provide a
cross-field snapshot.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
Reported-by: Ao Wang <wangao@seu.edu.cn>
Reported-by: Xuewei Feng <fengxw06@126.com>
Reported-by: Qi Li <qli01@tsinghua.edu.cn>
Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
Link: https://lore.kernel.org/all/8913381c-1e02-35c7-0ec4-61de5a12fd35@ssi.bg/
Assisted-by: Claude-Code:GLM-5.2
Suggested-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Acked-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
The upper/lower connection thresholds for dest can be changed,
so use ip_vs_dest_update_overload() to properly update the
dest overload flag.
The thresholds were not limited, fit them in the 0 .. INT_MAX
range as already done in ipvsadm.
As the thresholds are also read when connections are created
and expired, use WRITE_ONCE/READ_ONCE to access them.
As the lower threshold is optional, use (u - (u >> 2)) to
calculate the 75% default value based on the upper threshold
by preserving the integer rounding, as suggested by Yizhou Zhao.
Trigger flag update when totalconns reaches one of the
thresholds and use dst_lock to serialize the updating.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
Replace the inactconns dest counter with totalconns, now
inactconns can be obtained from totalconns - activeconns.
This reduces the atomic inc/dec ops for TCP/SCTP from
6 to 4 if the connection is established and then closed.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
A bridge nftables ct zone set rule can attach a conntrack template to
an skb before nf_ct_bridge_pre() sees it. For non-IPv4 and non-IPv6
EtherTypes, nf_ct_bridge_pre() currently overwrites skb->_nfct with
IP_CT_UNTRACKED without releasing the existing template reference.
That makes the per-cpu template, and any temporary templates allocated
for concurrent use, unreachable and leaks memory until the host runs out
of slab.
Reset the skb conntrack state before marking the frame untracked so the
existing template reference is dropped on the non-IP path.
Fixes: 3c171f496ef5 ("netfilter: bridge: add connection tracking system")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
__ip_set_put_byindex() resolved the index to a set pointer under RCU,
then took ip_set_ref_lock in __ip_set_put() to decrement set->ref.
ip_set_swap() holds that same lock while swapping both the ip_set_list
slots and the two sets' ref counters, so it can interleave between the
dereference and the lock acquisition, leaving the caller to decrement a
set whose reference already moved to the other index and hit
BUG_ON(set->ref == 0). list_set_gc() reaches this from timer softirq,
which the nfnl mutex does not serialize against swap: an expiring
list:set member calls list_set_del() -> ip_set_put_byindex() while
IPSET_CMD_SWAP runs on the referenced sets.
Resolve the index and decrement under ip_set_ref_lock, as ip_set_swap()
already does, keeping the refcount tied to the index rather than to a
stale set pointer.
kernel BUG at net/netfilter/ipset/ip_set_core.c:685!
Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
RIP: 0010:ip_set_put_byindex (net/netfilter/ipset/ip_set_core.c:870)
Call Trace:
<IRQ>
list_set_del (net/netfilter/ipset/ip_set_list_set.c:159)
set_cleanup_entries (net/netfilter/ipset/ip_set_list_set.c:181)
list_set_gc (net/netfilter/ipset/ip_set_list_set.c:578)
call_timer_fn (kernel/time/timer.c:1748)
__run_timers (kernel/time/timer.c:1799 kernel/time/timer.c:2374)
run_timer_softirq (kernel/time/timer.c:2405)
</IRQ>
Kernel panic - not syncing: Fatal exception in interrupt
Fixes: 9076aea76538 ("netfilter: ipset: Increase the number of maximal sets automatically")
Reported-by: AutonomousCodeSecurity@microsoft.com
Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Acked-by: Jozsef Kadlecsik <kadlec@netfilter.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
xprtsock replaces and restores sk->sk_data_ready and
sk->sk_write_space on live sockets with plain stores, and
xs_udp_do_set_buffer_size() invokes sk->sk_write_space via a plain
load. These callback pointers are shared with generic socket and
protocol paths that may read or invoke them concurrently, so xprtsock
needs the same READ_ONCE()/WRITE_ONCE() callback visibility contract
that the validated 4022 family applied elsewhere.
When SUNRPC takes over an AF_LOCAL, UDP, or TCP socket and later
restores the lower-socket callbacks during teardown, another CPU may
still hold an earlier callback snapshot. The plain replace/restore
pattern leaves the same visibility hole as the validated 4022 family,
so a stale snapshot can still invoke xs_data_ready() or
xs_udp_write_space() after the live callback fields have already been
restored to the lower-socket handlers.
Use WRITE_ONCE() for the shared sk_data_ready and sk_write_space
stores in xs_local_finish_connecting(), xs_udp_finish_connecting(),
xs_tcp_finish_connecting(), and xs_restore_old_callbacks(). Use
READ_ONCE() for the direct sk_write_space invocation in
xs_udp_do_set_buffer_size(). This matches the required callback
visibility contract while leaving adjacent sk_state_change and
sk_error_report handling unchanged.
Fixes: a246b0105bbd ("[PATCH] RPC: introduce client-side transport switch")
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
|
|
rpcb_register_inet4() and rpcb_register_inet6() store the result of
rpc_sockaddr2uaddr() into map->r_addr without checking it for NULL.
rpc_sockaddr2uaddr() returns NULL when its final kstrdup() fails, and
the unchecked NULL is then carried into the synchronous RPCBPROC_SET
encode path: rpcb_register_call() -> rpc_call_sync() ->
rpcb_enc_getaddr() -> encode_rpcb_string(), whose first statement is
strlen(string), dereferencing NULL and oopsing the kernel.
The crash reproduces under failslab on v6.12; with KASAN the NULL
dereference surfaces as a fault on the shadow of address zero:
Oops: general protection fault, probably for non-canonical address
0xdffffc0000000000 [#1] PREEMPT SMP KASAN
RIP: 0010:strlen (lib/string.c:409)
Call Trace:
encode_rpcb_string (net/sunrpc/rpcb_clnt.c:890)
rpcb_enc_getaddr (net/sunrpc/rpcb_clnt.c:910)
rpcauth_wrap_req_encode (net/sunrpc/auth.c:745)
call_encode (net/sunrpc/clnt.c:1966)
__rpc_execute (net/sunrpc/sched.c:952)
rpc_run_task (net/sunrpc/clnt.c:1243)
rpc_call_sync (net/sunrpc/clnt.c:1272)
rpcb_v4_register (net/sunrpc/rpcb_clnt.c:500)
svc_generic_rpcbind_set
nfsd_rpcbind_set
svc_register
svc_setup_socket
svc_addsock
write_ports
nfsctl_transaction_write
vfs_write
The crash is reachable when an in-kernel RPC service (nfsd, lockd,
nfs-callback) registers with the local rpcbind under enough memory
pressure for the small GFP_KERNEL kstrdup() in rpc_sockaddr2uaddr() to
fail. The asynchronous getport path already handles this exact failure
mode by returning -ENOMEM; only the two register helpers omit the check.
Mirror that handling: bail out with -ENOMEM when rpc_sockaddr2uaddr()
returns NULL, before the address is fed into the encoder.
Fixes: d77385f23830 ("SUNRPC: Fix rpc_sockaddr2uaddr")
Reported-by: Xiang Mei <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
|
|
Now that svc_seq_show() and the nfsd netlink stats handler both use
the per-netns svc_stat vs_count arrays, the global per-version
vs_count percpu counters are no longer read by anything. Remove the
vs_count field from struct svc_version and all the associated
DEFINE_PER_CPU_ALIGNED arrays and initializers across nfsd, lockd,
and the NFS client callback service.
Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260717-exportd-netlink-v7-4-b7ce17b83b60@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Update svc_seq_show() to read from the per-netns
statp->vs_count[] arrays instead of the global
svc_version->vs_count[].
The only caller is nfsd, which always allocates vs_count via
svc_stat_alloc_counts() in nfsd_net_init(), so the per-netns
arrays are always available.
This makes /proc/net/rpc/nfsd report per-network-namespace
procedure call counts.
Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260717-exportd-netlink-v7-2-b7ce17b83b60@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
The existing per-procedure call counts live in global
svc_version->vs_count[] arrays which are not network-namespace-aware.
Add per-netns equivalents in struct svc_stat so the upcoming netlink
stats interface can return namespace-scoped statistics.
Add a vs_count pointer array to struct svc_stat, along with
svc_stat_alloc_counts() and svc_stat_free_counts() helpers to manage
per-version percpu call count arrays.
Increment the per-net counter alongside the global one in
svc_generic_init_request(). Call the alloc/free helpers from
nfsd_net_init() and nfsd_net_exit().
Assisted-by: LLM
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260717-exportd-netlink-v7-1-b7ce17b83b60@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Now that the pool mode is always pernode, svc_serv.sv_nrpools is
redundant with sv_is_pooled: an unpooled service always has a single
pool, and a pooled service has svc_pool_map.npools pools (which is one on
a single-node host). sv_nrpools cannot distinguish an unpooled service
from a pooled service that happens to have one pool, so it is sv_nrpools,
not sv_is_pooled, that carries no unique information.
Replace the cached field with a svc_serv_nrpools() helper that derives
the count from sv_is_pooled and the pool map, and convert all readers to
it. svc_pool_map is file-local to svc.c, so export the helper for the
svc_xprt.c and nfsd callers.
Reading svc_pool_map.npools without svc_pool_map_mutex is safe: the
mutex protects only svc_pool_map.count, and npools is already read
locklessly in svc_pool_for_cpu().
A pooled service holds a map reference for its whole lifetime, so npools
is stable while any reader could observe it. The hot path
(svc_pool_for_cpu()) already dereferences svc_pool_map for to_pool, and
npools shares that cacheline, so there is no new locking or coherence
cost.
__svc_create() keeps using its local npools argument for the sv_pools[]
allocation, since sv_is_pooled is not set until svc_create_pooled() has
returned from it.
Doing this also removes a modulus operation from svc_pool_for_cpu(),
which should make for more efficient RPC queueing.
Assisted-by: Claude:claude-opus-4-8
Suggested-by: NeilBrown <neilb@ownmail.net>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260706-sunrpc-pool-mode-v5-5-6c4ee7cd89aa@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
svc_destroy() drops the service's reference to the global svc_pool_map
before iterating serv->sv_pools[] to destroy each pool's percpu counters.
That ordering happens to be fine today because the loop is bounded by the
per-service sv_nrpools field.
A following patch removes sv_nrpools and derives the pool count from the
pool map instead. svc_pool_map_put() zeroes svc_pool_map.npools when the
last reference is dropped, so a derived loop bound would read as zero for
the last pooled service and skip svc_pool_destroy_counters() entirely,
leaking the percpu counters (which remain linked on the global
percpu_counters list while the svc_serv is freed).
Reorder svc_destroy() to destroy the pool counters while the map is still
referenced, then drop the reference. No functional change.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260706-sunrpc-pool-mode-v5-4-6c4ee7cd89aa@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
svc_set_num_threads() spreads the requested thread count evenly across
the service's pools. In pernode mode each pool maps to a NUMA node, and
svc_pool_for_cpu() steers an incoming transport to the pool for the node
it arrived on. When fewer threads than pools are requested, even
distribution leaves some pools empty, and a transport steered to an
empty pool has no thread to service it.
Floor each pool at one thread when auto-distributing a non-zero count,
so no pool is left empty. Every pool maps to a node that had CPUs when
the pool map was built (svc_pool_map_init_pernode() only creates pools
for nodes returned by for_each_node_with_cpus()), so there is no pool
that should be left threadless. The resulting total may exceed the
requested count. This only affects the auto-distribute path (a
single-value array, i.e. svc_set_num_threads()); callers that set
per-pool counts explicitly via svc_set_pool_threads() are unchanged and
may still set a pool to zero.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: NeilBrown <neil@brown.name>
Link: https://patch.msgid.link/20260706-sunrpc-pool-mode-v5-3-6c4ee7cd89aa@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
The SVC_POOL_AUTO/GLOBAL/PERCPU/PERNODE pool mode selection machinery
was added when NUMA was new and the right default was unclear. The
default has always been "global" (a single pool for the whole service);
the other modes were only used when an admin explicitly set the
pool_mode parameter or asked for "auto", which then picked a mode from
the host topology. Today, pernode is the right choice everywhere:
- On multi-NUMA hosts, it gives one pool per node with proper thread
affinity and NUMA-local memory allocation.
- On single-node hosts, pernode degenerates to exactly one pool,
identical to the old "global" mode -- svc_pool_for_cpu() short-
circuits when sv_nrpools <= 1, no CPU affinity is set, and memory
is allocated from the single node.
The percpu mode (one pool per CPU) created excessive pools relative to
the number of threads most deployments run, and was only auto-selected
in a narrow case (single node, >2 CPUs).
Note that this changes the default behaviour on multi-NUMA hosts: a
service that previously ran with a single global pool now gets one pool
per NUMA node by default. This in turn means a host running fewer
threads than it has NUMA nodes can end up with pools that have no
threads. svc_pool_for_cpu() already falls back to a populated pool in
that case, so transports are still serviced.
Remove the SVC_POOL_* enum, mode selection heuristic,
svc_pool_map_init_percpu(), and all mode-based switch statements.
Simplify pool map functions to always use the pernode path. If pool
map allocation fails, svc_pool_map_get() now returns 0 and service
creation fails, rather than silently falling back to a single global
pool.
With the mode check gone, svc_pool_map_get_node() would dereference the
shared pool_to[] for every service that starts a thread. Only services
created via svc_create_pooled() hold a map reference that keeps that
array allocated, so gate the lookup in svc_new_thread() on sv_is_pooled:
unpooled services (e.g. lockd, the NFS callback) use NUMA_NO_NODE and
never consult the map. The kmalloc_node() callers in
svc_prepare_thread() already accept NUMA_NO_NODE, but __folio_alloc_node()
requires a valid node id, so resolve NUMA_NO_NODE to numa_mem_id() for
the scratch folio allocation.
The module parameter and netlink interfaces are preserved for backward
compatibility:
- Writing any of the four documented mode names still succeeds silently
- Reading always returns "pernode"
- Writing to the module parameter emits a deprecation notice
Update Documentation/admin-guide/kernel-parameters.txt to mark the
pool_mode parameter deprecated and describe the new behaviour.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260706-sunrpc-pool-mode-v5-2-6c4ee7cd89aa@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
svc_set_num_threads() spreads the requested threads evenly across the
service's pools (base = nrservs / sv_nrpools). When a service runs
fewer threads than it has pools -- e.g. an nfsd configured with fewer
threads than the host has NUMA nodes while running in "pernode" or
"percpu" mode -- the trailing pools are left with no threads at all.
svc_xprt_enqueue() selects a pool from the CPU servicing the transport,
queues the transport on that pool's sp_xprts, and only wakes a thread
from the same pool. Each thread services exclusively its own pool, so a
transport that lands on a threadless pool is enqueued on sp_xprts and
never picked up: the connection hangs indefinitely.
Have svc_pool_for_cpu() skip pools that currently have no threads,
falling back to the next populated pool. This trades NUMA locality for
a guarantee that the work is actually serviced. sp_nrthreads is only
updated under the service mutex; the lockless read here is a best-effort
routing hint, so annotate it with data_race().
Fixes: bfd241600a3b ("[PATCH] knfsd: make rpc threads pools numa aware")
Cc: stable@vger.kernel.org
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260706-sunrpc-pool-mode-v5-1-6c4ee7cd89aa@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Commit d57e43b72bf2 ("SUNRPC: Update svcxdr_init_decode() to call
xdr_set_scratch_folio()") changed svc_pool_map_get_node() to return
numa_mem_id() instead of NUMA_NO_NODE, because __folio_alloc_node()
cannot accept NUMA_NO_NODE. That return value is not equivalent: it
is evaluated in the context of the task creating the nfsd threads,
once per thread created, and it is passed to kthread_create_on_node()
and to the per-thread allocations in svc_prepare_thread().
Since commit d1a89197589c ("kthread: Default affine kthread to its
preferred NUMA node"), the node argument of kthread_create_on_node()
no longer only places the task structure and stack: a kthread created
with a real node id normally affines itself to that node's CPUs when
it is first woken to run its thread function. All nfsd threads are
typically started together, by one task writing to
/proc/fs/nfsd/threads, so under the default pool_mode=global each
nfsd thread is now affined to the local-memory node of the CPU its
creating iteration happened to run on - typically the same node for
every thread. The CPUs of the other nodes are then unable to run
nfsd at all, and the threads' allocations - svc_rqst structures,
page pointer arrays, newly allocated task stacks, and the per-RPC
pages allocated at run time - all prefer that one node.
Restore the NUMA_NO_NODE behaviour that global mode has had since
commit 11fd165c68b7 ("sunrpc: use better NUMA affinities"), and
handle NUMA_NO_NODE at the one call site that cannot take it by
resolving it to numa_mem_id() there, exactly as alloc_pages_node()
did for the scratch page before the conversion. The mapped percpu
and pernode branches are unchanged. Unpooled services such as lockd
and the NFS client callback service also take this fallback when no
percpu or pernode map is active, restoring their thread placement in
that case.
A bisect of a 2x NFS READ throughput regression between v6.17 and
v6.18 converged on d57e43b72bf2. On the affected 4-node server every
nfsd thread comes up with its CPU affinity restricted to the CPUs of
a single node; with this change the threads are runnable on all CPUs
again and the observed regression is resolved.
Fixes: d57e43b72bf2 ("SUNRPC: Update svcxdr_init_decode() to call xdr_set_scratch_folio()")
Cc: stable@vger.kernel.org
Signed-off-by: Ameer Hamza <ameer.hamza@truenas.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260722182012.2063936-1-ameer.hamza@truenas.com
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
An RPC-over-RDMA client can request a reply, such as an NFS READ
payload, without providing a Write list or a Reply chunk to carry
it. When such a reply needs more scatter/gather entries than the
device's Send Queue supports, svc_rdma_pull_up_needed() selects
pull-up and svc_rdma_pull_up_reply_msg() linearizes the whole
reply into sctxt->sc_xprt_buf. That buffer is only sc_max_req_size
bytes, while the reply on this path is bounded only by the client's
request, so svc_rdma_xb_linearize() copies past the end of the
buffer and corrupts adjacent slab memory. The oversized length is
then stored in sc_sges[0].length and posted, so the device also
reads beyond the mapped region.
The SGE-exhaustion branch is the only pull-up path that can exceed
the buffer: the threshold branch pulls up only replies smaller
than RPCRDMA_PULLUP_THRESH, and replies that fit the device's SGE
budget are sent directly without linearization. Make
svc_rdma_pull_up_needed() report -E2BIG when the reply it would
pull up cannot fit sc_max_req_size, and fail the request with
ERR_CHUNK as RFC 8166 Section 4.5.3 directs rather than dropping
the connection.
The helper no longer answers a simple yes/no question: it now
reports pull-up, no pull-up, or -E2BIG for a reply too large to
linearize. Rename svc_rdma_pull_up_needed() to
svc_rdma_check_pull_up() so its name no longer implies a boolean
predicate.
Fixes: e248aa7be86e ("svcrdma: Remove max_sge check at connect time")
Cc: stable@vger.kernel.org
Reported-by: Chris Mason <clm@meta.com>
Assisted-by: kres:claude-opus-4-7
Link: https://patch.msgid.link/20260623014728.826032-1-cel@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
svc_rqst_free() frees rqstp->rq_argp and rqstp->rq_resp synchronously
via kfree(), but defers the rqstp struct free via kfree_rcu(). After
svc_exit_thread() calls list_del_rcu() and svc_rqst_free(), there is
a window where RCU readers that started before list_del_rcu() can still
traverse the thread list and find the rqstp. These readers (e.g.
nfsd_nl_rpc_status_get_dumpit()) dereference rqstp->rq_argp, which has
already been freed — a use-after-free.
Fix this by moving the kfree of rq_argp and rq_resp into an explicit
call_rcu() callback alongside the struct free. Resources not accessed
by RCU readers (bvec, buffer pages, scratch folio, auth_data) remain
synchronously freed.
Fixes: 812443865c5f ("sunrpc: add a rcu_head to svc_rqst and use kfree_rcu to free it")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260611-nfsd-testing-v2-4-5b90e276f2d9@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
A pooled RPC service sizes its threads dynamically, growing and
shrinking each pool between its minimum and maximum bounds as load
varies. The count of running threads therefore reflects recent
demand, not the service's capacity. A consumer that sizes a data
structure against the concurrency the service can sustain -- NFSD's
NFSv4 session slot tables, for one -- needs that stable ceiling, and
computing it means summing sp_nrthrmax across every pool.
Add svc_serv_maxthreads() so the summation, and its dependence
on the layout of struct svc_serv and struct svc_pool, stays within
sunrpc. The read is lock-free: pool maxima change only when a service
is reconfigured, a path callers already serialize against startup and
shutdown, so a racing reader observes at worst a transient value. This
is acceptable for the sizing heuristics that will consume it.
nfsd_nrthreads() already sums sp_nrthrmax across pools by hand; convert
it to svc_serv_maxthreads(), giving the new export an in-tree consumer
and removing a copy of the dependence on svc_serv internals.
Reviewed-by: NeilBrown <neil@brown.name>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Reviewed-by: Benjamin Coddington <bcodding@hammerspace.com>
Link: https://patch.msgid.link/20260610-nfsd-slot-growth-clamp-v1-1-7b966700df0b@kernel.org
Signed-off-by: Chuck Lever <cel@kernel.org>
|
|
Replacing strcpy() with strscpy() ensures that overflow of the target
buffer cannot happen.
Signed-off-by: David Laight <david.laight.linux@gmail.com>
Link: https://patch.msgid.link/20260608095523.2606-16-david.laight.linux@gmail.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
rpcrdma_rn_register() inserts @rn into rd_xa with xa_alloc() before
storing the caller's callback in rn->rn_done. The xarray makes @rn
reachable to rpcrdma_remove_one(), which walks rd_xa and invokes
rn->rn_done(rn) for every registered notification. A device removal
that races a fresh registration can therefore observe @rn with
rn_done still NULL, because the notification objects are zero
allocated by their owners, and call through a NULL function pointer.
Store rn->rn_done before xa_alloc() publishes @rn. The xarray's
store-side and load-side ordering then guarantees that any CPU which
finds @rn in rd_xa also observes the armed callback.
rpcrdma_rn_unregister() treats a non-NULL rn_done as the sentinel
for a completed registration, so the early store must not survive a
failed registration. Clear rn_done again when xa_alloc() fails.
Were it left set, the failed-accept cleanup path would call
rpcrdma_rn_unregister() on an @rn that was never inserted, erasing
an unrelated rd_xa slot and underflowing rd_kref.
Fixes: 7e86845a0346 ("rpcrdma: Implement generic device removal")
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20260601201703.46078-1-cel@kernel.org
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
__svc_create() initializes three per-pool percpu_counter stats and
ignores every return value. On SMP, percpu_counter_init() fails when
__alloc_percpu_gfp() cannot satisfy the allocation, leaving the failed
counter with fbc->counters == NULL and its embedded raw_spinlock_t,
list_head, and count never initialized. __svc_create() returns the
half-constructed svc_serv to nfsd, lockd, or the NFS callback service
anyway.
Once that service is live, the hot-path increments in
svc_xprt_enqueue(), svc_handle_xprt(), and
svc_pool_wake_idle_thread() reach a counter whose backing pointer is
NULL. The pointer is a per-cpu offset, so the access does not fault:
it resolves to offset zero of the current CPU's per-cpu area and
silently corrupts whatever variable lives there. A
/proc/fs/nfsd/pool_stats read walks the same NULL per-cpu storage and
returns garbage, and on CONFIG_DEBUG_SPINLOCK or lockdep it splats on
the never-initialized lock.
Creating the broken service requires a percpu allocation failure during
RPC server startup, so it is reachable only by a local administrator
under memory pressure or fault injection; a remote peer cannot induce
the bad state on its own.
Check each percpu_counter_init() return value in __svc_create() and
fail when an allocation fails, unwinding the counters already set up
in the current pool and in every pool initialized before it. A
discrete percpu_counter_destroy() per counter at teardown frees each
per-cpu allocation exactly once.
Fixes: ccf08bed6e7a ("SUNRPC: Replace pool stats with per-CPU variables")
Cc: stable@vger.kernel.org
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260530-tier2-local-v2-2-5a0fd532db57@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
create_use_gss_proxy_proc_entry() publishes /proc/net/rpc/use-gss-proxy
via proc_create_data() before init_gssp_clnt() runs mutex_init() on
sn->gssp_lock. Once the dentry is linked under proc_subdir_lock it is
immediately reachable from userspace, so a write that lands in the
window drives set_gssp_clnt() into mutex_lock() on a zero-initialized
struct mutex.
create_use_gss_proxy_proc_entry(net)
proc_create_data("use-gss-proxy", ...) /* dentry live */
init_gssp_clnt(sn)
mutex_init(&sn->gssp_lock) /* too late */
write_gssp()
set_gssp_clnt(net)
mutex_lock(&sn->gssp_lock) /* uninitialized */
gssp_rpc_create(...)
sn->gssp_clnt = clnt
mutex_unlock(&sn->gssp_lock)
The window spans only the two statements between proc_create_data()
returning and init_gssp_clnt(), so a writer reaches it only if the
registering thread is preempted there while another task is already
opening the freshly published file. register_pernet_subsys() runs in
preemptible context under pernet_ops_rwsem, so that preemption is
possible, and the window widens on auth_rpcgss module load, when the
proc entry is created for every live net namespace whose tasks are
already running. A writer that wins the race locks a zero-filled
struct mutex. On CONFIG_DEBUG_MUTEXES the missing magic value trips a
"lock used without init" splat; on a production kernel the fast path
acquires the lock via CMPXCHG(owner, 0, current). In the latter case
a second writer that arrives before init_gssp_clnt() re-zeroes owner
can enter set_gssp_clnt() concurrently, shut down the first writer's
clnt while it is still in use, and leak the loser's clnt.
Fix by initializing sn->gssp_lock in sunrpc_init_net() so its lifetime
matches the sunrpc_net it lives in. sn->gssp_clnt is already NULL from
the kzalloc that backs net_generic storage, so the lazy helper is no
longer needed; drop init_gssp_clnt(), its prototype, and the call from
create_use_gss_proxy_proc_entry(). sunrpc.ko is a build-time
dependency of auth_rpcgss.ko, so sunrpc_init_net() has always run on
every netns before any auth_gss pernet init can publish the proc
entry.
Fixes: 030d794bf498 ("SUNRPC: Use gssproxy upcall for server RPCGSS authentication.")
Cc: stable@vger.kernel.org
Assisted-by: kres:claude-opus-4-7
Signed-off-by: Chris Mason <clm@meta.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260530-tier2-local-v2-1-5a0fd532db57@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
A backchannel receive can complete a request while the NFS callback
service is being torn down. xprt_complete_bc_request() removes the
request from bc_pa_list, drops bc_alloc_count, marks the request in use,
and then asks xprt_enqueue_bc_request() to hand it to the callback
service.
If teardown has already cleared xprt->bc_serv, xprt_enqueue_bc_request()
currently returns without enqueueing or freeing the committed request.
The xprt_get() taken on entry is leaked as well. If the producer wins
the race before bc_serv is cleared, it can also enqueue onto sv_cb_list
after nfs_callback_down() has stopped the callback threads, leaving the
request linked to a svc_serv that is about to be freed.
Close the producer side before callback threads are stopped. Add
xprt_svc_shutdown_bc() to clear xprt->bc_serv under bc_pa_lock, and call
it on callback shutdown and callback-start failure before stopping the
service threads. Requests that lose the NULL transition in
xprt_enqueue_bc_request() are released through the normal backchannel
free path after balancing bc_slot_count. Finally, drain any remaining
sv_cb_list requests after the callback threads have stopped and before
svc_destroy() frees the service.
Fixes: 441244d4273a ("SUNRPC: cleanup common code in backchannel request")
Fixes: 9e9fdd0ad0fb ("NFSv4.1: protect destroying and nullifying bc_serv structure")
Cc: stable@vger.kernel.org
Signed-off-by: Chris Mason <clm@meta.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260528-tier2-v1-6-d026a1415e0b@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
svcauth_gss_decode_credbody() writes the caller's
rpc_gss_wire_cred field by field and assigns gc_ctx.len only on
the success tail. The caller storage is svcdata->clcred, which
lives in the per-svc_rqst gss_svc_data and is reused across
requests. Early decode failures leave partially decoded state
mixed with residue from the prior request.
The trailing body_len tightness check is the sharpest case:
xdr_stream_decode_opaque_inline() has already written gc_ctx.data
with a borrowed inline pointer into the current request's XDR
pages, but gc_ctx.len retains its prior value. Once the request
pages are released the pooled clcred carries a dangling pointer
paired with a stale length.
Zero the caller's rpc_gss_wire_cred at function entry so that
every early-return path leaves a deterministic all-zero cred.
On the trailing tightness-check path, gc_ctx.len is now zero
instead of stale, which neuters length-driven consumers such as
gss_svc_searchbyctx() that would otherwise walk the dangling
data pointer.
Fixes: b0bc53470d1a ("SUNRPC: Convert the svcauth_gss_accept() pre-amble to use xdr_stream")
Cc: stable@vger.kernel.org
Signed-off-by: Chris Mason <clm@meta.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260528-tier2-v1-5-d026a1415e0b@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
svcauth_gss_release() reads gc_proc and switches on gc_svc before
consulting rq_auth_stat. On the SVC_DENIED path after a failed
svcauth_gss_accept(), those fields may hold stale values from a
prior request or uninitialized slab residue: svcauth_gss_accept()
allocates gss_svc_data with non-zeroing kmalloc and clears only
gsd_databody_offset and rsci per request, not clcred.
Because RPC_GSS_PROC_DATA is zero, a zeroed or stale-zero gc_proc
passes the existing guard and falls through into the gc_svc switch,
which can dispatch to svcauth_gss_wrap_integ() or
svcauth_gss_wrap_priv(). Both wrap helpers call
svcauth_gss_prepare_to_wrap() before any rsci->mechctx dereference,
and that helper already returns early when rq_auth_stat is not
rpc_auth_ok, so the downstream NULL dereference is blocked. The
dispatch itself remains structurally wrong: it reads scalars that
the caller has no contract to have initialized after a failed
authentication.
Mirror the existing rq_auth_stat gate in
svcauth_gss_prepare_to_wrap() one frame up, so
svcauth_gss_release() skips the clcred dispatch entirely when
authentication has not succeeded. The cleanup tail that releases
rq_client, rq_gssclient, cr_group_info, and rsci still runs.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Chris Mason <clm@meta.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260528-tier2-v1-4-d026a1415e0b@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
gssx_dec_option_array() walks the wire-supplied option array and, for
every entry whose name matches CREDS_VALUE, calls
gssx_dec_linux_creds() on the same struct svc_cred. That helper
unconditionally installs a fresh groups_alloc() result into
creds->cr_group_info without releasing whatever pointer was already
there:
for (i = 0; i < count; i++) {
... decode name ...
if (length == sizeof(CREDS_VALUE) &&
memcmp(p, CREDS_VALUE, sizeof(CREDS_VALUE)) == 0) {
err = gssx_dec_linux_creds(xdr, creds);
...
}
}
A reply that carries two CREDS_VALUE entries therefore overwrites
cr_group_info on the second iteration and orphans the group_info
allocated by the first call. The earlier free_creds path only
releases the last cr_group_info via free_svc_cred(), so the first
allocation's refcount stays at one and its kvmalloc-backed storage
is leaked. No in-tree caller of gssp_accept_sec_context_upcall()
expects more than one CREDS_VALUE per reply.
Fix by tracking whether a CREDS_VALUE option has already been
decoded and returning -EINVAL on any subsequent match, so the
free_creds path releases the single group_info that was installed.
Fixes: 1d658336b05f ("SUNRPC: Add RPC based upcall mechanism for RPCGSS auth")
Cc: stable@vger.kernel.org
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260528-tier2-v1-3-d026a1415e0b@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
Four coupled defects in the gssx XDR option-array decoder make the
error paths unsafe: a NULL deref in the caller, a refcount leak on
the decoded group_info, and a latent use-after-free that the leak
fix would otherwise expose.
gssx_dec_option_array() sets oa->count = 1 before allocating
oa->data. If that allocation fails, -ENOMEM is returned with
oa->count == 1 and oa->data == NULL. All other error paths jump
to free_oa: which frees oa->data and NULLs it but also leaves
oa->count == 1. The caller trusts the count:
gssp_accept_sec_context_upcall()
gssx_dec_accept_sec_context()
gssx_dec_option_array() /* fails, count=1 data=NULL */
data = res.options.data[0].value /* NULL deref */
Independently, free_creds: releases the partially decoded svc_cred
with a bare kfree(creds). gssx_dec_linux_creds() installs a
groups_alloc() result into creds->cr_group_info; that object is
kvmalloc-backed and refcounted, and only put_group_info() reaches
kvfree(). A plain kfree(creds) drops the wrapper and leaks the
group_info allocation.
The natural fix for the leak is to call free_svc_cred(creds) before
kfree(creds), but free_svc_cred() invokes put_group_info() on
creds->cr_group_info unconditionally when non-NULL. The existing
out_free_groups: path in gssx_dec_linux_creds() already called
groups_free() on that pointer without clearing it, so once
free_svc_cred() is wired in, the subsequent put_group_info() would
touch freed memory.
Fix all four together:
- Move the oa->count = 1 assignment below the oa->data allocation
so it is never set when oa->data is NULL.
- Reset oa->count to 0 at free_oa: so count and data stay
coherent and the caller sees an empty option array.
- Call free_svc_cred(creds) before kfree(creds) at free_creds:
so the refcounted cr_group_info is released. free_svc_cred()
either NULL-guards each field explicitly (cr_group_info has
an if() check) or delegates to a helper that is NULL-safe
itself (kfree for the string fields, gss_mech_put() which
guards with if(gm) at gss_mech_switch.c:342), so it is safe
to call on a partially decoded svc_cred where only
cr_uid/cr_gid/cr_group_info have been written and everything
else is zero from kzalloc.
- In gssx_dec_linux_creds()'s out_free_groups: path, release
cr_group_info with put_group_info() rather than groups_free()
so the teardown matches free_svc_cred()'s refcount-aware path,
and clear the pointer so a later free_svc_cred() on the same
creds does not release it a second time.
Fixes: 3cfcfc102a5e ("SUNRPC: fix some memleaks in gssx_dec_option_array")
Cc: stable@vger.kernel.org
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260528-tier2-v1-2-d026a1415e0b@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
gss_krb5_unwrap_v2() sets buf->len to a logical
length, which can be much smaller than head[0].iov_len
(the allocated receive-page capacity). It then calls
xdr_buf_trim() with a trim length derived from the 16-bit
"extra count" (ec) field in the Kerberos v2 token header.
The ec field is authenticated by the post-decrypt memcmp()
against the encrypted header copy, so a randomly-mutated
value is rejected. However, any peer holding a valid GSS
context can legitimately encrypt a token whose ec exceeds
the plaintext length. Per RFC 4121, such a token is
structurally malformed.
Although xdr_buf_trim() now clamps the buf->len subtraction
to avoid unsigned underflow, the buffer is still left in a
semantically invalid state (zero length, inconsistent iov
lengths) when ec is oversized.
Reject these tokens before calling xdr_buf_trim(), giving
callers a well-defined GSS_S_DEFECTIVE_TOKEN error and
keeping the xdr_buf internally consistent. The wrapped blob
begins at a nonzero offset -- both callers pass len as
offset + opaque_len -- so buf->len still counts the offset
bytes that precede the blob. Compare the trim length
against the remaining wrapped segment, buf->len - offset,
rather than the whole buffer; comparing against buf->len
alone leaves an offset-wide window in which an oversized ec
passes the test and xdr_buf_trim() cuts into the bytes ahead
of the blob.
Fixes: cf4c024b9083 ("sunrpc: trim off EC bytes in GSSAPI v2 unwrap")
Cc: stable@vger.kernel.org
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260528-tier2-v1-1-d026a1415e0b@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
When svc_rdma_listen_handler() handles RDMA_CM_EVENT_ADDR_CHANGE,
it creates a replacement listener cm_id and returns 1, telling
the CM core to destroy the old one. If the replacement allocation
fails, sc_cm_id still points at the old cm_id that the CM core is
about to destroy. Any subsequent dereference of sc_cm_id --
such as svc_rdma_detach()'s rdma_disconnect() call -- is a
use-after-free.
NULL sc_cm_id on the failure path and guard svc_rdma_detach()'s
rdma_disconnect() call against NULL so that the listener can
be torn down safely when the server shuts down.
Fixes: d1b586e75ec6 ("svcrdma: Handle ADDR_CHANGE CM event properly")
Cc: stable@vger.kernel.org
Acked-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260527-rdma-follow-on-v1-5-1b09bd87b6cd@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
handle_connect_req() returns without action when
svc_rdma_create_xprt() fails to allocate the new transport.
The CM core returns 0 for CONNECT_REQUEST events, so it does
not destroy the new rdma_cm_id. Each allocation failure under
memory pressure leaks one rdma_cm_id, and a remote peer driving
connection attempts can amplify this.
Reject the connection by returning a non-zero status from the
CM event handler, which tells the CM core to destroy the
orphaned cm_id.
Fixes: 377f9b2f4529 ("rdma: SVCRDMA Core Transport Services")
Cc: stable@vger.kernel.org
Acked-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260527-rdma-follow-on-v1-4-1b09bd87b6cd@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
|
svc_rdma_create() calls kfree(cma_xprt) when
svc_rdma_create_listen_id() fails. svc_xprt_init() has already
acquired a net namespace reference via get_net_track(); kfree
bypasses svc_xprt_free() which releases it.
Replace the kfree() with svc_xprt_put() so the kref_init birth
reference drops to zero and svc_xprt_free() dispatches
svc_rdma_free() to clean up properly. sc_cm_id is still NULL
at that point; the preceding patch added the necessary NULL
guard in svc_rdma_free().
svc_xprt_free() also drops the module reference via
module_put(), but the caller _svc_xprt_create() does the same
on xpo_create failure, double-putting the single
try_module_get() it acquired. Take a compensating
__module_get() before the svc_xprt_put() to keep the count
balanced, matching the convention in svc_rdma_accept()'s error
path.
Fixes: 4fb8518bdac8 ("sunrpc: Tag svc_xprt with net")
Cc: stable@vger.kernel.org
Acked-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260527-rdma-follow-on-v1-3-1b09bd87b6cd@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
|
