| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
rds_rdma_cm_event_handler_cmn() hands the return value of
rdma_resolve_route() straight back to the rdma_cm on
RDMA_CM_EVENT_ADDR_RESOLVED. A synchronous failure there - -ENOMEM
from the route work allocation, an SA query that cannot be set up, a
RoCE route with no usable device - makes addr_handler() destroy the
id the event was delivered on. That id is ic->i_cm_id, and nothing
clears the pointer: the connection sits in RDS_CONN_CONNECTING with a
freed id until its shutdown calls rdma_disconnect() and
rdma_destroy_id() on it.
The ROUTE_RESOLVED case already avoids this: rds_ib_cm_initiate_connect()
forces a zero return while ic->i_cm_id == cm_id, as the comment there
explains. Do the same here - drop the connection and return 0 - so
the id stays RDS's to destroy from the shutdown, and the reconnect
gets a fresh one.
Fixes: 55b7ed0b582f ("RDS: Common RDMA transport code")
Assisted-by: LLM
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20261003163408.250568-3-achender@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
rds_rdma_cm_event_handler_cmn() assigns trans only when the RDMA
device is an InfiniBand CA (RDMA_NODE_IB_CA). On any other device
type, e.g. an iWARP RNIC such as siw, trans stays uninitialized, but
the event switch dereferences it: unconditionally in the
RDMA_CM_EVENT_CONNECT_REQUEST case via trans->cm_handle_connect(),
and (with a connection context) in the ROUTE_RESOLVED and
ESTABLISHED cases.
An RDS listener on an iWARP device therefore crashes the kernel as
soon as a connect request arrives: with CONFIG_INIT_STACK_ALL_ZERO
the wild load becomes a NULL dereference at offset 0xa0
(&trans->cm_handle_connect) in the iw_cm_wq workqueue.
GCC masks the bug in default builds by folding the uninitialized
load into &rds_ib_transport; Clang-built kernels take the real
uninitialized path and oops.
The active side can land on a non-IB device too: the id that
rds_ib_conn_path_connect() creates is not restricted to a node type,
so rdma_resolve_addr() binds it to whichever device serves the local
address, and an iWARP RNIC on the same netdev qualifies. The crash
there is different: resolving a route on an iWARP id never fills in
cm_id->route.path_rec, and the ROUTE_RESOLVED case writes
path_rec[0].sl before it calls into the transport.
The iWARP transport was dropped long ago and IB is the only
transport left, so make that explicit: initialize trans to
&rds_ib_transport at declaration, drop the conditional assignment,
reject a connect request that arrives on a non-IB device, and drop a
connection whose address resolved to one instead of resolving a
route on it. The rejection is limited to RDMA_CM_EVENT_CONNECT_REQUEST
on purpose: a non-zero return from the handler makes rdma_cm destroy
the id the event was delivered on, which is right for the request's
freshly created id but would free a connection id that RDS still owns
for any other event. On the active side the id stays ic->i_cm_id and
the connection's shutdown destroys it, so that case returns 0.
Fixes: dcdede0406d3 ("RDS: Drop stale iWARP RDMA transport")
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Link: https://lore.kernel.org/netdev/20260824111701.2979194-1-ljp1205831794@gmail.com/
Link: https://lore.kernel.org/netdev/20260825021223.3483044-1-ljp1205831794@gmail.com/
Cc: stable@vger.kernel.org
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <henrymei@tencent.com>
[achender: reject only on RDMA_CM_EVENT_CONNECT_REQUEST instead of
bailing out for every event on a non-IB device, so that rdma_cm does
not destroy connection ids RDS still tracks; drop a connection whose
address resolved to a non-IB device before a route is resolved on it;
changelog adjusted]
Assisted-by: LLM
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20261003163408.250568-2-achender@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
__rds_conn_create() computes npaths from the caller's transport before
it decides whether a connection to one of the host's own addresses is
to be handled by the loopback transport instead. That substitution is
what an RDS/TCP socket sending to a local address gets, and after it
the path init loop still runs for the TCP transport's RDS_MPATH_WORKERS
paths and allocates an ordered workqueue for each, while
rds_loop_conn_alloc() only ever provides transport data for path 0.
rds_conn_destroy() sizes its teardown from c_trans, by then the
loopback transport, so it visits path 0 only - and
rds_conn_path_destroy() would skip the other paths anyway, since it
returns before destroy_workqueue() for a path without transport data.
kfree(c_path) then drops the last pointers to seven workqueues. That
repeats for every such connection, on every netns teardown or module
unload, and every distinct local destination address is a separate
connection.
Recompute npaths once the transport is final, so that creation and
destruction agree on the set of paths. The c_path array stays sized
for the caller's transport; the unused entries are freed with it.
Fixes: 4716af3897e9 ("net/rds: Give each connection path its own workqueue")
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260921215027.174657-1-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
rds_ib_map_frmr() stores the caller's scatterlist in the MR before DMA
mapping and registration can fail. On failure, __rds_rdma_map() unpins
the pages and frees the scatterlist, but rds_ib_free_frmr() can still
return the MR to the pool with the stale pointer set.
This leaves the pool with a dangling scatterlist and can lead to local
privilege escalation. KASAN detects the resulting use-after-free when the
MR is later torn down:
BUG: KASAN: slab-use-after-free in __rds_ib_teardown_mr
Read of size 8
Call Trace:
__rds_ib_teardown_mr
rds_ib_unreg_frmr
rds_ib_flush_mr_pool
rds_ib_flush_mrs
rds_free_mr
rds_setsockopt
Store the scatterlist in the MR only after DMA mapping succeeds. If DMA
mapping fails, return directly while the MR fields remain clear; the caller
keeps ownership of the scatterlist and its pinned pages. If a later
registration step fails, unmap the scatterlist and clear the MR fields
before returning.
Fixes: 1659185fb4d0 ("RDS: IB: Support Fastreg MR (FRMR) memory registration mode")
Cc: stable@vger.kernel.org
Signed-off-by: Dongliang Qin <cccccccccccc777777@gmail.com>
Reviewed-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260922031546.3874605-1-cccccccccccc777777@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
rds_ib_cm_connect_complete() runs from the RDMA-CM event handler with
conn->c_cm_lock held. When the peer negotiates a protocol version
older than RDS_PROTOCOL_COMPAT_VERSION, the handler calls
rds_conn_destroy(), which is only safe in the rmmod path: it
synchronously tears the connection down and flush_work()es the
shutdown work cp_down_w.
That shutdown work (rds_conn_shutdown()) needs cp_cm_lock, which is
the very lock the event handler still holds, so the flush never
completes: the two workers wait on each other and the RDS connection
workqueues stall for good.
All other RDMA-CM failure paths (REJECTED, CONNECT_ERROR,
DISCONNECTED) use rds_conn_drop(), which marks the connection
RDS_CONN_ERROR and schedules the shutdown work asynchronously. Use
it here as well.
Fixes: f147dd9ecabf ("RDS/IB: Disallow connections less than RDS 3.1")
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Cc: stable@vger.kernel.org
Reviewed-by: Allison Henderson <achender@kernel.org>
Signed-off-by: Aohan Mei <henrymei@tencent.com>
Link: https://patch.msgid.link/20260911073436.3542080-1-ljp1205831794@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
Pull networking fixes from Jakub Kicinski:
"Nothing too exciting, usual stream of fixes. Including fixes from
Netfilter, Bluetooth and WPAN.
Current release - new code bugs:
- Bluetooth: hci_sync: fix not setting CE length properly
- eth: enic: match mailbox replies to request numbers
Previous releases - regressions:
- tunnels: drop stale dst when building an ICMP error for PMTUD
- ipv6: null-check fib6_node before accessing in __ip6_del_rt_siblings()
(bug in the rtnl_lock -> RCU conversion)
- eth: bnxt_en:
- fix crashes on Thor2 due to OOB coalescing buffer accesses
- prevent queue stop with deferred completions
Previous releases - always broken:
- eth:
- ice: don't dereference pointers from TP_printk()
- fix OOB writes on ethtool flow rule dump in 3 drivers
- mlx5: fix FEC configuration with RS_544_514_INTERLEAVED_QUAD
- dsa: tag_brcm: legacy FCS: request needed tailroom
Misc:
- net: cap tx_queue_len at S16_MAX to prevent oversized ring alloc
- ipv6: flowlabel: cap duplicate leases per socket"
* tag 'net-7.3-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net: (164 commits)
selftests: tc-testing: test action batch failure cleanup
net/sched: act_api: release all action references on NEWACTION failure
openvswitch: fix wrong flag value in get_ipv6_ext_hdrs()
ipmr: account multicast table and route memory
net: phy: dp83td510: handle the active-high LED polarity mode
net: macb: initialize PTP state before registering clock
net: hsr: enable promiscuous mode on interlink port with fwd offload
ipv6: fix fib6 walker UAF on seq stop
net: stmmac: fix TX descriptor availability check for TSO traffic
net/rds: fix tcp stream corruption with large pages
net: mana: restore the XDP program pointer when pre-allocation fails
net: phy: dp83867: handle the active-high LED polarity mode
octeontx2-af: fix PF/CGX debugfs PCI bus lookup
net: net_failover: Fix the deadlock in net_failover_slave_name_change()
net: phy: mediatek-ge: disable EEE on the MT7530 PHY
tcp: reject non zerocopy devmem tx
net: ethernet: mtk_eth_soc: populate lpi_interfaces to fix EEE support
net: dsa: mt7530: populate lpi_interfaces to fix EEE support
net: hinic: fix mailbox segment buffer overflow
net: sun4i-emac: fix missing of_node_put() for phy_node
...
|
|
rds_message_map_pages() assigns PAGE_SIZE bytes to every
scatterlist entry, even when total_len ends in a partial page. The RDS
congestion map is defined as 8192 bytes, so on systems with PAGE_SIZE
greater than 8192 the scatterlist maps bytes beyond the end of the
congestion map. RDS-TCP transmits the SG contents according to those
lengths, so the extra bytes become part of the TCP RDS stream and are
interpreted as subsequent RDS message headers, corrupting the stream.
Limit the final scatterlist mapping to the number of bytes remaining.
This has no effect on systems with a 4K page size and allows RDS-TCP to
be used on systems with 16K and larger page sizes.
The RDS selftest, which previously hung on 16K pages, now passes.
Fixes: 7875e18e0996 ("RDS: Message parsing")
Signed-off-by: Greg Marsden <greg.marsden@oracle.com>
Reviewed-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/apxJjxvStibPI0AS@oracle.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
This is another run of the Coccinelle script for converting kmalloc()
family of allocations to kmalloc_obj() via the existing rules in
scripts/coccinelle/api/kmalloc_objs.cocci
This catches both the set of kmalloc() uses added since the first
kmalloc_obj() conversions in v7.0 and adds a large group missed in the
first pass due to Coccinelle not interacting well with the cleanup.h
scoped_...() family of macros[1]. I worked around this with spatch's
"--macro-file" argument to a file with all the scoped_...() macros mapped
to Coccinelle's YACFE_ITERATOR[2] as that was the closest viable control
flow indicator I could find.
Build tested allmodconfig on x86, arm64, arm, loongarch, mips, powerpc,
riscv, and s390 with no new warnings.
Link: https://lore.kernel.org/lkml/202609021314.8A9C0B8@keescook/ [1]
Link: https://github.com/coccinelle/coccinelle/blob/master/standard.h [2]
Signed-off-by: Kees Cook <kees+treewide@kernel.org>
|
|
rds_conn_shutdown() finishes by moving the path from
RDS_CONN_DISCONNECTING to RDS_CONN_DOWN, and also accepts
RDS_CONN_ERROR as the starting state of that final transition, so that
a FIN processed in softirq context during the teardown does not derail
the shutdown into a noisy error path.
But consuming that RDS_CONN_ERROR also consumes the shutdown pass that
came with it: rds_conn_path_drop() sets RDS_CONN_ERROR and then queues
cp_down_w, and a pass that starts on a path already in RDS_CONN_DOWN
is a no-op. For the FIN case that is harmless - the socket the FIN
arrived on is the very socket the teardown just released. It is not
harmless for a dropper that attached something to the path first.
rds_tcp_accept_one() is such a dropper. Its path claim in
rds_tcp_accept_one_path() transitions RDS_CONN_DOWN ->
RDS_CONN_CONNECTING, and a concurrent drop - a FIN on a previous
socket in softirq context, an administrative reset - can put the path
into RDS_CONN_ERROR between that claim and the state check that
follows, which accepts RDS_CONN_ERROR. The accept then installs the
freshly accepted socket with rds_tcp_set_callbacks() while the queued
teardown - which sampled tc->t_sock before this socket existed - is
still running. rds_connect_path_complete() fails its transition to
RDS_CONN_UP and drops the path again, queueing the pass that should
reap the socket it just installed. If the in-flight shutdown's final
transition consumes that drop's RDS_CONN_ERROR, the queued pass finds
the path in RDS_CONN_DOWN and does nothing. The installed socket is
never torn down: it sits established with its callbacks armed and its
rds_tcp_connection on rds_tcp_tc_list, the peer sees a connection that
nothing ever reads, and the path is wedged in RDS_CONN_DOWN until some
later event drops it again. Reproduced with widened race windows as
an ever-growing receive queue on a socket owned by a path stuck in
RDS_CONN_DOWN, with the peer's send path wedged behind it.
Make the final transition only DISCONNECTING -> DOWN. If it fails
because the path is in RDS_CONN_ERROR, a drop raced the teardown:
cancel the reconnect timer and clear RDS_RECONNECT_PENDING - the one
piece of the skipped tail that must not be left behind - and return,
letting the pass the drop queued finish the job: it tears down
whatever attached to the path in the meantime, completes the
transition to RDS_CONN_DOWN, and re-arms the reconnect from its own
tail.
The timer quiesce in that branch matters because the racing drop does
not always queue that pass: rds_conn_path_drop() returns without
queueing when a destroy is pending - exactly the situation during a
netns teardown or module unload, when a FIN on the dying socket is
processed while rds_conn_path_destroy() flushes cp_down_w. If the
flushed pass is the one that takes this return, no later pass exists,
and rds_conn_path_destroy() would find cp_conn_w still armed
(WARN_ON) and then free a path whose reconnect timer can still fire.
With the cancel in the branch, every exit of a shutdown pass leaves
the timer quiesced no matter which pass completes the transition.
The FIN case keeps making progress, one pass later and still without
noisy logging. Any other state keeps today's rds_conn_path_error()
handling; no current cp_state writer can leave a DISCONNECTING path
in anything but RDS_CONN_ERROR (every other writer is a cmpxchg from
a non-DISCONNECTING state), so that branch is defensive.
On kernels without the preceding patches the same hazard exists with
the sample-based quiesce; the fix applies there equally.
Fixes: e97656d03ca0 ("rds: tcp: allow progress of rds_conn_shutdown if the rds_connection is marked ERROR by an intervening FIN")
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260828223921.202913-8-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
rds_conn_shutdown() quiesces the transmit and receive-refill paths by
waiting for RDS_IN_XMIT and RDS_RECV_REFILL to be sampled clear, and
then runs the transport shutdown and rds_conn_path_reset(). Sampling
the bits clear is not the same as owning them: the moment after the
wait_event() returns, rds_send_xmit() can re-acquire RDS_IN_XMIT (or
rds_ib_recv_refill() can re-acquire RDS_RECV_REFILL) and run
concurrently with the teardown.
The sender does recheck the connection state after taking the lock,
but that recheck is a classic store-buffering pattern: teardown writes
the state and reads the bit while the sender writes the bit and reads
the state. acquire_in_xmit() is only an acquire operation, so on
weakly ordered architectures both sides can miss each other's write,
and the transmit path then runs while the transport zeroes its rings
(e.g. rds_ib_ring_init()) and rds_send_path_reset() rewrites the
transmit state under it.
Oracle UEK fixed the same class of crashes - a 14-year tail of
BUG_ON()s in rds_ib_sub_signaled(), unexpected op-codes and NULL
dereferences in rds_ib_send_cqe_handler() during failover testing -
by making the teardown path *acquire* the fastpath bit locks instead
of testing them ("rds: Make sure transmit path and connection
tear-down does not run concurrently"). Ownership of a single word is
decided by RMW atomicity, so no cross-variable ordering is needed.
Do the same here: take both locks before calling the transport
shutdown, hold them across rds_conn_path_reset(), and release them
explicitly with a wake-up afterwards. Both are released with
clear_bit_unlock(), so that the ring re-initialization done by the
transport shutdown and the transmit state rewritten by
rds_send_path_reset() are ordered before either bit is seen clear by
the next acquire_in_xmit() or acquire_refill().
The fastpath users of these bits - rds_send_xmit() and
rds_ib_recv_refill() - are trylock style and back off while teardown
owns the locks, so no new lock dependency is introduced for them.
rds_tcp_reset_callbacks() is different: since the previous patch it
acquires RDS_IN_XMIT as well, and it blocks doing so, so its wait now
spans the teardown instead of at most one send batch. That waiter
runs from rds_tcp_accept_one() on the single-threaded krdsd workqueue
and holds rds_tcp_accept_lock and t_conn_path_lock while it waits, so
a duelling SYN accepted while its path is being torn down parks
accept processing for the duration of the teardown - for TCP bounded
by the (up to 5 s) drain loop in rds_tcp_conn_path_shutdown(). An IB
path's drain in rds_ib_conn_path_shutdown() has no round cap, but no
blocking waiter either: rds_tcp_reset_callbacks() is the only blocking
acquirer of these bits and waits only on its own TCP path, and the
fastpaths are trylock-and-back-off on both transports, so a long IB
drain lengthens only that path's own quiesce. The
window is narrow: the accept-side state check has to pass before the
teardown moves the path to RDS_CONN_DISCONNECTING.
Because krdsd is a single global workqueue, everything else queued
there - accept processing for other connections and network
namespaces, and the flush_workqueue(rds_wq) in rds_tcp_listen_stop()
during namespace teardown - waits behind the parked accept worker for
that time. It cannot deadlock, although the waits do point at each
other: the teardown blocks until the bit's holder releases it, and
the holder may be that krdsd accept worker. The holder finishes
without needing anything the teardown owns: the sync cancels
rds_tcp_reset_callbacks() issues target cp_send_w and cp_recv_w on
the path's ordered cp_wq, whose only execution slot is occupied by
the blocked cp_down_w itself, so they are pending at most and cancel
without flushing - a reliance on cp_wq being ordered that is now
noted next to those cancels (on the allocation-failure fallback where
a path shares rds_wq, the work items simply serialize).
Nor is the blocking wait itself new: rds_tcp_reset_callbacks() has
waited on RDS_IN_XMIT from the krdsd work item since
commit 335b48d980f6 ("RDS: TCP: Add/use rds_tcp_reset_callbacks to
reset tcp socket safely"); this patch stretches its worst case from
a sender's batch to the teardown's drain. The alternative to parking
is the accept path racing the teardown, which is what these patches
close; making the teardown itself non-blocking is a separate item.
One observable side effect: the SENDING flag reported by rds-info has
always mirrored RDS_IN_XMIT, so it now also covers the window where
teardown owns the bit.
The comments that describe the old sample-based handshake or name
rds_send_xmit() as the only other holder of these bits - in
rds_send_xmit(), above rds_conn_path_reset(), in rds_ib_recv_refill()
and in rds_tcp_reset_callbacks() - are updated to match.
For anyone backporting this patch standalone: it depends on
"net/rds: clear cp_flags bits individually in rds_conn_path_reset()"
and "net/rds: acquire RDS_IN_XMIT in rds_tcp_reset_callbacks()"
earlier in this series. Without the former, the blanket cp_flags
clear in rds_conn_path_reset() would drop both held bits in the middle
of the teardown; without the latter, rds_tcp_reset_callbacks() would
still sample t_sock without owning RDS_IN_XMIT. "net/rds: use
clear_bit_unlock() in release_refill()" is needed for the refill
side's release to pair with the acquire added here, and the follow-up
"net/rds: don't let rds_conn_shutdown() consume a concurrent drop"
completes the teardown-state handling for the waiter this patch
parks; a backport should carry all four.
Fixes: 0f4b1c7e89e6 ("rds: fix rds_send_xmit() serialization")
Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
[achender: reimplement for net-next shutdown path: acquire the existing
RDS_IN_XMIT/RDS_RECV_REFILL bit locks in rds_conn_shutdown() and release
after teardown; update comments and commit message]
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260828223921.202913-7-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
rds_tcp_reset_callbacks() quiesces the transmit path by setting the
path state to RDS_CONN_RESETTING and then waiting for RDS_IN_XMIT to
be sampled clear before swapping the underlying socket and calling
rds_send_path_reset().
Sampling the bit clear is not the same as owning it: rds_send_xmit()
can re-acquire RDS_IN_XMIT right after the wait_event() returns. Its
state recheck after taking the lock is a store-buffering pattern (the
resetter writes the state and reads the bit, the sender writes the
bit and reads the state) and acquire_in_xmit() is only an acquire
operation, so on weakly ordered architectures both sides can miss
each other's write and the transmit path then runs concurrently with
rds_send_path_reset() rewriting cp_xmit_* state - which is exactly
what the comment above rds_send_path_reset() tells its callers to
prevent.
Take the lock instead, hold it across the socket swap and
rds_send_path_reset(), and release it with a wake-up at the end. The
lock-ordering constraint documented above the wait still holds: the
lock is acquired before lock_sock(), so a sender inside tcp_sendmsg()
can never be waited on while we hold the socket lock.
Two details of the old code go away with the same change:
- t_sock is now read only after the lock is acquired. The old code
cached it before waiting; the teardown in rds_conn_shutdown()
releases that socket and clears t_sock, so a pointer cached before
the wait can be stale by the time the accept path resumes. Reading
it under RDS_IN_XMIT is what makes the exclusion complete once the
teardown owns the same lock, which the next patch arranges; until
then the teardown still only samples the bit, and the two paths
remain as exposed to each other as they are today.
- The old !osock early path called rds_send_path_reset() with no
serialization at all. It now runs under the lock like the normal
path. The conditional RDS_CONN_RESETTING transition of the
previous patch happens before the socket check either way: a path
found without a socket is either still connecting (its reconnect
worker blocked on t_conn_path_lock) and legitimately goes
RESETTING -> UP on the new socket, or it has been torn down
meanwhile and is dropped.
The in-function comment describing the old wait-based quiesce is
rewritten to describe the lock-based one, and the stale block comment
above the function (which still described a return value and an
incomplete list of t_sock writers) is refreshed to name all four
writers - the connect, accept, teardown and swap paths - and what
serializes each of them.
Fixes: 335b48d980f6 ("RDS: TCP: Add/use rds_tcp_reset_callbacks to reset tcp socket safely")
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260828223921.202913-6-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
rds_tcp_reset_callbacks() resolves a duelling SYN by storing
RDS_CONN_RESETTING into cp_state unconditionally. Nothing serializes
that store against the shutdown path: rds_tcp_accept_one() checks
for RDS_CONN_CONNECTING or RDS_CONN_ERROR under t_conn_path_lock, but
neither rds_conn_path_drop(), which forces RDS_CONN_ERROR, nor
rds_conn_shutdown(), which moves the path to RDS_CONN_DISCONNECTING
under cp_cm_lock, takes that lock. The store can therefore land on
top of a shutdown that is already in progress, or that gets queued
right after the accept-side check.
When it does, the shutdown worker's final DISCONNECTING -> DOWN
transition fails and the path goes through rds_conn_path_error() and
a second drop/shutdown cycle instead of a clean reconnect, tearing
down the socket the accept path has just installed. Before commit
ad22d24be635 ("net/rds: No shortcut out of RDS_CONN_ERROR") a path
found in RDS_CONN_RESETTING even made rds_conn_shutdown() bail out
altogether.
Make the transition conditional: move CONNECTING -> RESETTING (or
stay in RESETTING from an earlier duel), and drop the path in any
other state. The drop has side effects of its own: it replaces the
shutdown's RDS_CONN_DISCONNECTING (or RDS_CONN_ERROR) with
RDS_CONN_ERROR and queues one more cp_down_w run. The difference is
that rds_conn_shutdown() accepts RDS_CONN_ERROR in its final
transition to RDS_CONN_DOWN, so the shutdown in flight completes
normally instead of through rds_conn_path_error(); the extra
down-work pass then finds the path already down and falls through to
the reconnect check, or catches a reconnect that has already started
and restarts it. The accept path still installs the new socket,
rds_connect_path_complete() then fails its RESETTING -> UP transition
and drops it: the raced socket ends up torn down as it does today.
The comment at that call site, which promised that
rds_connect_path_complete() marks the path RDS_CONN_UP, is updated to
name this outcome as well.
The state can change again between the failed transitions and the
drop. That is inherent to rds_conn_path_drop(), which the socket
state-change callbacks also call unconditionally, and costs at most
one extra drop/reconnect cycle.
Based on Oracle UEK commit "net/rds: Don't force state
RDS_CONN_RESETTING" by Gerd Rausch.
Fixes: 9c79440e2c5e ("RDS: TCP: fix race windows in send-path quiescence by rds_tcp_accept_one()")
Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
[achender: port to net-next: use the two-argument
rds_conn_path_transition()/rds_conn_path_drop() and rewrite the
changelog for the upstream shutdown path]
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260828223921.202913-5-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
rds_conn_path_reset() wipes the whole flag word with a plain
cp->cp_flags = 0 store. Every other accessor of that word uses
atomic bitops, and some of them can run concurrently with the reset:
RDS_LL_SEND_FULL is set from rds_send_xmit() and cleared from the
transport completion paths, neither of which holds anything that
excludes the shutdown worker. A plain store racing an atomic
read-modify-write on the same word is a data race, and whichever
side loses has its update silently discarded.
Clear the two bits the reset is actually responsible for instead.
RDS_IN_XMIT and RDS_RECV_REFILL need no store at all here: they
belong to the caller, rds_conn_shutdown(), which waits for both to be
clear before calling the transport shutdown and this reset.
This also gives every bit in cp_flags a single well-defined writer
discipline, which the following patches rely on when they turn
RDS_IN_XMIT and RDS_RECV_REFILL into bit locks held across the
teardown: a blanket store mid-teardown would destroy lock ownership
that an atomic clear preserves.
Oracle UEK carries the same conversion ("net/rds: Preserve essential
connection state flags"), motivated by its asynchronous shutdown
state machine, whose progress and destroy flags must survive the
reset. UEK's variant also clears RDS_IN_XMIT and RDS_RECV_REFILL
because there the reset runs as the final step of a teardown that
owns both bits, making those clears its unlock. Upstream that
release belongs in rds_conn_shutdown(): once a later patch in this
series turns the two bits into locks held across the teardown, ending
ownership needs release semantics and a wake-up that a plain clear
inside the reset would not provide.
Based on Oracle UEK commit "net/rds: Preserve essential connection
state flags" by Gerd Rausch.
Fixes: 00e0f34c6166 ("RDS: Connection handling")
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260828223921.202913-4-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
release_refill() drops the RDS_RECV_REFILL bit with a plain
clear_bit(). clear_bit() has no ordering semantics, and the
smp_mb__after_atomic() that follows it sits on the wrong side for a
lock release: it orders the clear against the waitqueue_active() load
below it, but does nothing to order the refill critical section's ring
and descriptor stores before the clear itself.
That matters once connection teardown owns RDS_RECV_REFILL as a lock
across the transport shutdown and path reset, rather than sampling it
clear, which "net/rds: acquire the fastpath locks in
rds_conn_shutdown()" later in this series arranges: on a weakly
ordered architecture the teardown can win the bit and start the
shutdown and reset while some of the refill's stores are not yet
visible to it. The same gap existed under the sample-based scheme - a
waiter that saw the bit clear had no guarantee it also observed the
refill's stores - but taking the bit as a lock makes the missing
release pairing load-bearing.
Switch to clear_bit_unlock(), which orders the critical section before
the release, and replace the open-coded barrier-plus-waitqueue_active()
with wq_has_sleeper(), whose internal full barrier keeps the
store-buffering guarantee between clearing the bit and checking for
sleepers. This mirrors what "net/rds: use wq_has_sleeper() in
release_in_xmit()" does for RDS_IN_XMIT.
The fast-path acquire side, acquire_refill(), uses test_and_set_bit(),
a full-barrier RMW that pairs with this release. The teardown at this
point in the series still samples the bit, so on its own this change
is release-side hardening; the shutdown-conversion patch named above
makes the teardown acquire the bit with the same RMW, completing the
pairing at the end of the series.
Fixes: 73ce4317bf98 ("RDS: make sure we post recv buffers")
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260828223921.202913-3-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
release_in_xmit() clears RDS_IN_XMIT with clear_bit_unlock() and then
checks waitqueue_active() to decide whether anyone needs waking.
clear_bit_unlock() is only a release operation: it orders the
critical section before the bit clear, but does not order the
subsequent plain load of the wait queue head after it. The waiter
side does the mirror image - it adds itself to the wait queue and
then tests the bit. That is the classic store-buffering pattern: the
releasing CPU can read the wait queue as empty while the waiting CPU
still reads the bit as set, so the sleeper is never woken.
The waiters are rds_conn_shutdown() and rds_tcp_reset_callbacks(),
both in uninterruptible wait_event() with no timeout. A lost wake-up
strands the shutdown worker on its single-threaded workqueue until
some other sender releases the bit again - and on a connection that
is being torn down precisely because it failed, there may never be
another sender.
The barrier used to be there: release_in_xmit() did clear_bit()
followed by smp_mb__after_atomic() until commit 1422f28826d2 ("rds:
introduce acquire/release ordering in acquire/release_in_xmit()")
folded both into clear_bit_unlock(), which strengthened the lock
hand-off but silently dropped the full barrier the wake-up check
depends on. The refill counterpart, release_refill() in
net/rds/ib_recv.c, still carries its smp_mb__after_atomic() for
exactly this reason.
Use wq_has_sleeper(), which is waitqueue_active() preceded by the
required full barrier.
Fixes: 1422f28826d2 ("rds: introduce acquire/release ordering in acquire/release_in_xmit()")
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260828223921.202913-2-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
Pull networking fixes from Jakub Kicinski:
"Including fixes from Bluetooth, IPSec and Netfilter.
Current release - fix to a fix:
- netfilter: ipset: remove need to allocate memory on delete operations
Current release - regressions:
- macb: drop CONFIG_OF #if block, fix build
Previous releases - always broken:
- stream of fixes for SCTP continues
- inet: frags: strip GSO state from fragments before reassembly
- virtio-net: ensure that TCP packets don't overflow gso_segs
- tcp-ao: fix use-after-free of current_key on reconnect to another
peer
- page_pool: remove zone/policy GFP flags when allocating XArray
entries
- Bluetooth: L2CAP: reject accept queue add unless BT_LISTEN
- tls: device: fix out-of-bounds write in tls_append_frag()
- eth: bnxt:
- ring the doorbell when SW USO exits early, avoid packets stuck
in Tx
- gate TPH enablement behind BNXT_SUPPORTS_QUEUE_API check, avoid
users of older NICs seeing non-actionable warning messages
- eth: qede: fix NULL pointer dereference in TPA fragment processing"
* tag 'net-7.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net: (216 commits)
inet: frags: strip GSO state from fragments before reassembly
net/sched: sch_htb: limit htb_classify inner-class filter hops
selftests/net: packetdrill: add tcp_urg_ptr_retransmit
tcp: fix corruption of urgent data on multi-segment retransmit
usb: atm: usbatm: fix invalid ci_range initialization
net: fec: only stop PTP if it was initialized
slip: remove slip_hangup() to fix use-after-free in slip_receive_buf()
net: bridge: mcast: fix use-after-free of a master VLAN's multicast context
net/sched: bound qdisc_pkt_len to prevent qdisc soft lockup
net: dsa: mxl862xx: enable assisted learning on CPU port
net: stmmac: restore NET_IP_ALIGN in the RX DMA offset
net: stmmac: drop gso_enabled_types and rely on netdev features
net: stmmac: selftests: Don't test flow control for small rx fifos
net: stmmac: selftests: Account for the UC filter list for filtering tests
net: stmmac: dwxgmac: Account for the primary MAC address for UC filtering
net: stmmac: dwmac4: Account for the primary MAC address for UC filtering
net: stmmac: dwmac1000: Account for the primary MAC address for UC filtering
net: stmmac: selftests: Check multiple MMC counters
selftests: net: Fix slow configurations in big_tcp_tunnels.sh
selftests: net: Lower threshold with csum offload off in big_tcp_tunnels.sh
...
|
|
rds_cong_map_updated() runs after a peer's congestion map has been
rewritten (by rds_tcp_cong_recv() and rds_ib_cong_recv(), or the
clear-all in the loopback and IB send-completion paths). It bumps
rds_cong_generation and then checks waitqueue_active() on
map->m_waitq and on rds_poll_waitq to decide whether anyone needs
waking. atomic_inc() carries no ordering and waitqueue_active() is a
plain load, so nothing orders the map and generation stores before
the wait queue reads. The waiters do the mirror image: rds_cong_wait()
adds itself to m_waitq and then tests the port bit, and rds_poll()
registers on rds_poll_waitq and then reads the generation. That is
the store-buffering pattern described above waitqueue_active() in
include/linux/wait.h - the updater can observe an empty wait queue
while the waiter still observes the port as congested, and no wake-up
is issued.
rds_cong_wait() is an interruptible sleep with no timeout, so a
sender blocked on a congested port stays blocked until the next
congestion update from that peer arrives or a signal is delivered.
A poll() waiter misses the map-updated notification the same way.
Use wq_has_sleeper(), which is waitqueue_active() preceded by the
required full barrier, as rds_tcp_state_change() already does for
the same pattern.
Fixes: 922cb17a5c81 ("RDS: Congestion-handling code")
Signed-off-by: Allison Henderson <achender@kernel.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260822052647.88318-1-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Pull RDMA updates from Jason Gunthorpe:
"About the normal size, still a lot of AI bug fixes and so on, but some
interesting new functionality too:
- Assorted locking, bounds-checking, cleanup, and error-path fixes
across UCMA/CMA, bng_re, bnxt_re, cxgb4, EFA, ERDMA, HFI1, HNS,
ionic, iRDMA, mlx4/mlx5, RXE, SIW, SRP/SRPT, and iSER target.
- netlink report for max # of supported resources
- get_zeroed_page()/etc removal
- Robust udata for ionic
- Allow unique RDMA device names per network namespace
- Completion counters and v2 admit queue support for EFA
- UC QP support for MANA
- Completion timestamps for ionic
- Harden uverbs data validation and resource lifetime handling,
fixing several core use-after-free conditions.
- bnxt_re toggle-page ownership and lifetime bug fixes
- dmabuf SRQ support for mlx5"
* tag 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma: (160 commits)
RDMA/ucma: Allow path records to exactly fit the output buffer
RDMA/uverbs: Guard legacy bundles without method_elm
RDMA/efa: Add support for 128B admin v2 SQ entry
RDMA/efa: Generalize the admin SQ
RDMA/efa: Decouple admin command payload from admin header
RDMA/rxe: Fix OOB in free_rd_atomic_resources()
RDMA/cma: Fix WARNING in res_to_rt
RDMA/cxgb4: Free debugfs on registration failure
RDMA/cxgb4: Cancel reg_work before freeing device on remove
RDMA/ucma: Lock the handler in ucma_set_ib_path()
RDMA/ucma: Lock the handler in ucma_write_cm_event()
RDMA/erdma: restrict the driver to little-endian systems
RDMA/ionic: Embed counter driver data in rdma_counter allocation
RDMA/ionic: Cap eq_count to the eth driver's interrupt vector budget
RDMA/siw: Fix use-after-free in siw_accept()
IB/isert: post the full-feature receive buffers after session registration
IB/isert: delay the final Login Response until the session is registered
RDMA/srp: fix heap information leak on a truncated SRP_CRED_REQ
RDMA/erdma: Hold QP references for AE and CM processing
RDMA/erdma: Hold CQ references when processing EQ events
...
|
|
The commit that introduced the receive-path latency trace added the
clearing of inc->i_rx_lat_trace[] to rds_inc_init() only;
rds_inc_path_init() never got it.
That asymmetry matters for the one caller that reuses memory:
rds_tcp_data_recv() carves its rds_tcp_incoming out of a kmem_cache
with no zeroing and no constructor, so after rds_inc_path_init() the
array still holds the timestamps of whatever message previously
occupied that slab object. No stale value is user-visible today -
every message that reaches the socket happens to overwrite all four
slots (RX_HDR at allocation, RX_START when the header completes,
RX_END at delivery, RX_CMSG at recvmsg time) before
RDS_CMSG_RXPATH_LATENCY reads them back as deltas - but that is a
property of the current writers, not of the init contract, and a
future trace point or an early-exit path would expose another
message's timestamps to userspace.
Clear the array in rds_inc_path_init() too, so both init helpers
leave the inc fully initialized. memset is the form the clearing
already takes on the rds_inc_init() side since commit 1635bb548f84
("net: rds: use memset to optimize the recv"). Hardening only; no
user-visible bug in the current code.
Assisted-by: Claude-Code:claude-fable-5
Signed-off-by: Allison Henderson <achender@kernel.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260810055631.299558-1-achender@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
rds_inc_init() initializes every field of the embedded rds_incoming
except i_conn_path, and incomings are not zero-allocated (IB carves
them out of a slab cache). The field therefore holds stale garbage
for incs created by rds_ib.
The loopback transport is different: rds_loop_xmit() re-runs
rds_inc_init() on the message's embedded inc after
rds_send_queue_rm() has already stored the connection path in it, so
there the field holds a live value rather than garbage, and a NULL
store would discard it. Switch rds_loop_xmit() to
rds_inc_path_init() with the connection's single path, which is
exactly the value readers of the field reconstruct for a
non-multipath transport.
With loopback preserving the field, initialize it to NULL in
rds_inc_init() so that any future reader trips over a clean NULL
pointer instead of a stale one, and so the two init helpers
(rds_inc_init/rds_inc_path_init) leave the structure in an
equivalent, fully-initialized state. Hardening only; no reader
dereferences i_conn_path for a non-multipath transport today.
This mirrors Oracle UEK commit "rds: rds_inc_init() should initialize
the inc->i_conn_path field".
Signed-off-by: William Kucharski <william.kucharski@oracle.com>
[achender: port to net-next; keep loopback's i_conn_path valid by
switching rds_loop_xmit() to rds_inc_path_init(); update commit
message]
Assisted-by: Claude-Code:claude-fable-5
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260809005103.82371-3-achender@kernel.org
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
The to_be_dropped list is declared once at the top of rds_send_xmit()
but the function can loop via "goto restart" after each batch. The
code currently relies on rds_send_remove_from_sock() having emptied
the list entry by entry (via list_del_init()) at the end of the
previous batch; nothing in rds_send_xmit() itself guarantees the list
head is empty when a new batch starts.
Re-initialize the list on every restart, and warn once if it is ever
found non-empty there: entries left on the list at that point would
keep their message reference, their RDS_MSG_ON_SOCK accounting and
their pending RDS_RDMA_DROPPED notification, so a silent re-init
would orphan them. This is hardening: no user-visible bug is known
in the current code.
This mirrors Oracle UEK commit "net/rds: rds_send_xmit should
INIT_LIST_HEAD(&to_be_dropped) on restart".
Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
Signed-off-by: Sharath Srinivasan <sharath.srinivasan@oracle.com>
[achender: port to net-next (keep the existing LIST_HEAD declaration and
add only the restart re-init); warn if the restart invariant is
violated; update commit message]
Assisted-by: Claude-Code:claude-fable-5
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260809005103.82371-2-achender@kernel.org
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
rds_info_getsockopt() reads a callback from rds_info_funcs and invokes it
without protecting the callback's lifetime. Transport modules register
functions stored in this array. For example, rds_tcp.ko registers
rds_tcp_tc_info() for RDS_INFO_TCP_SOCKETS.
This permits the following interleaving:
CPU0 CPU1
rds_info_getsockopt()
func = rds_tcp_tc_info
rmmod rds_tcp
rds_tcp_exit()
rds_info_deregister_func()
rds_info_funcs[offset] = NULL
free rds_tcp module text
func()
The reader can therefore branch to an address in unloaded module text.
Protect callback invocation with SRCU. Enter the SRCU read-side critical
section before loading the callback and leave it only after the callback
returns. Clear the callback with WRITE_ONCE() and call
synchronize_srcu() before deregistration returns, preventing module unload
from freeing its text while an old reader is still executing it. SRCU is
required because callbacks such as RDS_INFO_COUNTERS can sleep.
Keep the callback array unannotated and use READ_ONCE() and WRITE_ONCE()
for concurrent slot access so sparse does not have to apply __rcu through
the function-pointer typedef. Replace the two callback-slot BUG_ON()
checks with WARN_ON_ONCE() and return without changing the slot on
mismatch.
Link: https://lore.kernel.org/netdev/20260720184955.3008978-1-nicoyip.dev@gmail.com/
Suggested-by: Allison Henderson <achender@kernel.org>
Suggested-by: Kuniyuki Iwashima <kuniyu@google.com>
Reviewed-by: Allison Henderson <achender@kernel.org>
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
Link: https://patch.msgid.link/20260801054234.3535077-1-nicoyip.dev@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The pages backing an RDS memory region are pinned in __rds_rdma_map()
with rds_pin_pages(), which uses pin_user_pages_fast(): each page's
refcount is biased by GUP_PIN_COUNTING_BIAS to account the pin. The
scatterlist is then handed to the IB transport, and the transport
releases the pages in __rds_ib_teardown_mr() with
set_page_dirty(page);
put_page(page);
put_page() drops a single reference instead of removing the pin bias,
so every MR teardown permanently strands the remaining references and
the pages are never freed - a userspace-triggerable memory leak of up
to RDS_MAX_MSG_SIZE per RDS_GET_MR/RDS_GET_MR_FOR_DEST call.
The conversion to the pin API updated the unpin sites in rdma.c but
missed this one on the transport side. Release the pages with
unpin_user_pages_dirty_lock(), which removes the pin bias and also
dirties the page under the folio lock, closing the truncation race
that a bare set_page_dirty() leaves open.
Dirtying under the folio lock can sleep, which is safe in every path
that reaches __rds_ib_teardown_mr(): the registration-reuse path
(rds_ib_map_frmr()) runs in syscall context, and the pool flush
(rds_ib_unreg_frmr()) runs under pool->flush_lock, a mutex, and
already sleeps in rds_ib_post_inv(). The WARN_ON that guarded the
old irq-context set_page_dirty() case is dropped along with it.
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260730041629.3512480-5-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When rds_send_xmit() picks the next message off cp_send_queue it takes
its own reference with rds_message_addref(). If the message then hits
the never-retransmit check (RDS_MSG_FLUSH, or an RDMA op that was
already retransmitted), it is moved to the local to_be_dropped list and
that reference is dropped after the batch.
However, if RDS_MSG_ON_CONN has already been cleared, the message is
not added to to_be_dropped and the reference taken above is never
dropped: cp_xmit_rm has not been set at this point, so the loop simply
abandons rm and the rds_message (and everything it pins: pages, MRs,
notifiers) leaks after an RDMA error.
The only other places that clear RDS_MSG_ON_CONN are
rds_send_path_drop_acked() and rds_send_drop_to(), and both can run
while rds_send_xmit() has dropped cp_lock between moving the message
to cp_retrans and re-taking the lock in the never-retransmit check:
rds_send_path_drop_acked() can ack away a message that already sat on
cp_retrans - the RDS_MSG_RETRANSMITTED case above - and
rds_send_drop_to() runs on socket close. Both unlink the message
under cp_lock and put their own reference, leaving the xmit-path
reference stranded.
Drop the reference directly in that case.
This mirrors Oracle UEK commit "net/rds: fix rds_message memleak in
rds_send_xmit".
Signed-off-by: Gerd Rausch <gerd.rausch@oracle.com>
Signed-off-by: Sharath Srinivasan <sharath.srinivasan@oracle.com>
[achender: port to net-next; update commit message, checkpatch nits]
Signed-off-by: Allison Henderson <achender@kernel.org>
Link: https://patch.msgid.link/20260730041629.3512480-4-achender@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Each rds_mr stores a bare back pointer to the socket that created it
(mr->r_sock) but takes no reference on it. When the mr is dest |
