* [PATCH v2 net] net/rds: fix out-of-bounds write in rds_conn_peer_gen_update()
@ 2026-08-25 12:01 pamoutaf
2026-08-25 23:16 ` Allison Henderson
2026-08-28 23:05 ` Jakub Kicinski
0 siblings, 2 replies; 3+ messages in thread
From: pamoutaf @ 2026-08-25 12:01 UTC (permalink / raw)
To: achender
Cc: davem, edumazet, kuba, pabeni, horms, netdev, linux-rdma,
rds-devel, linux-kernel
From: Paula Moutafian <paula@bynar.io>
rds_conn_peer_gen_update(), rds_start_mprds() and rds_check_all_paths()
all iterate conn->c_path[] up to RDS_MPATH_WORKERS (8) or conn->c_npaths,
but c_path is allocated with only
npaths = (trans->t_mp_capable ? RDS_MPATH_WORKERS : 1)
entries in __rds_conn_create() (net/rds/connection.c). Only the TCP
transport sets t_mp_capable, so for the IB/RDMA transport (and the loop
transport) exactly one rds_conn_path is allocated.
c_npaths is derived from the peer via the RDS_EXTHDR_NPATHS handshake
extension header and is only clamped to RDS_MPATH_WORKERS, never to the
transport's actual allocation. A remote peer on an RDS/IB (RoCE) fabric
can therefore complete the unauthenticated handshake and, by sending a
second RDS_EXTHDR_GEN_NUM with a changed generation number, drive
rds_conn_peer_gen_update() to walk c_path[1..7] past the end of a
one-element allocation -- taking cp_lock, writing cp_next_tx_seq /
cp_next_rx_seq and walking cp_retrans on neighbouring slab objects.
rds_start_mprds() and rds_check_all_paths() are reachable the same way
via a peer-supplied c_npaths > 1; note rds_check_all_paths() is a
do/while and dereferences c_path[0] before testing the bound.
Reproduced against an unmodified 7.2.0 KASAN build, triggered from a
hand-rolled RDS/IB peer completing the handshake and sending two probes
with differing RDS_EXTHDR_GEN_NUM:
BUG: KASAN: slab-out-of-bounds in do_raw_spin_lock+0x55/0x9a
Write of size 4 at addr ff1100000425a624 by task ksoftirqd/0/13
Call Trace:
do_raw_spin_lock+0x55/0x9a
_raw_spin_lock_irqsave+0x12/0x18
rds_recv_hs_exthdrs+0x34a/0x52a
rds_recv_incoming+0x5a8/0xb33
rds_ib_recv_cqe_handler+0xda1/0x12b6
poll_rcq+0x8e/0xb1
rds_ib_tasklet_fn_recv+0x1c4/0x348
Allocated by task 10:
__rds_conn_create+0x5b3/0x168f
rds_conn_create+0x18/0x1b
rds_ib_cm_handle_connect+0x486/0xa35
The buggy address is located 44 bytes to the right of
allocated 504-byte region, cache kmalloc-512
The corrupted neighbour's cp_retrans.next is then dereferenced on the
next line, producing a fatal GPF (RIP: rds_recv_hs_exthdrs+0x3ba/0x52a,
KASAN: null-ptr-deref) -- a crash, not merely a detected access.
Fix conn->c_npaths at the source instead of guarding every reader: in
rds_recv_hs_exthdrs() (net/rds/recv.c), clamp the peer-supplied
RDS_EXTHDR_NPATHS value to the transport's actual per-connection
allocation rather than to the fixed RDS_MPATH_WORKERS ceiling. c_npaths
can then never exceed the number of rds_conn_path entries
__rds_conn_create() allocated, which takes care of rds_start_mprds()
and rds_check_all_paths() -- both already bound their loops by
conn->c_npaths -- with no change at either call site.
rds_conn_peer_gen_update() is different: its loop is bound by the fixed
RDS_MPATH_WORKERS constant, not by conn->c_npaths, so the c_npaths fix
above does not reach it. It still needs its own bound, computed the same
way as the allocation site and mirroring the sibling pattern already used
in rds_conn_destroy() (net/rds/connection.c).
rds_conn_peer_gen_update() and rds_start_mprds() (net/rds/recv.c) were
both correct when written, with c_path still a fixed
RDS_MPATH_WORKERS-element array:
commit 905dd4184e07 ("RDS: TCP: Track peer's connection generation number")
commit 5916e2c1554f ("RDS: TCP: Enable multipath RDS for TCP")
They became wrong when the allocation was made conditional on
t_mp_capable without updating either loop or the RDS_EXTHDR_NPATHS clamp:
commit 840df162b3eb ("rds: reduce memory footprint for RDS when transport is RDMA")
rds_check_all_paths() (net/rds/connection.c) is unrelated to that
regression: it was added new, three years later, and was unbounded
from the moment it was written, but is fixed by the same c_npaths clamp:
commit 9ef845f894c9 ("rds: If one path needs re-connection, check all and re-connect")
Fixes: 840df162b3eb ("rds: reduce memory footprint for RDS when transport is RDMA")
Fixes: 9ef845f894c9 ("rds: If one path needs re-connection, check all and re-connect")
Cc: stable@vger.kernel.org
Assisted-by: Bynario AI
Signed-off-by: Paula Moutafian <paula@bynar.io>
---
v2:
- Per Allison Henderson's review, clamp conn->c_npaths itself at the
point it is set from the peer's RDS_EXTHDR_NPATHS in
rds_recv_hs_exthdrs(), instead of adding a second bound check at
each of its readers. This covers rds_start_mprds() and
rds_check_all_paths() with no further change at either site;
net/rds/connection.c is unchanged from v1.
- rds_conn_peer_gen_update()'s loop is bound by RDS_MPATH_WORKERS
directly, not by conn->c_npaths, so it is unaffected by the above
and keeps its own bound from v1.
[v1] https://lore.kernel.org/netdev/213829b7380f1fe12aed2f2ae9ba33c2870addd5.camel@kernel.org/T/#t
net/rds/recv.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/net/rds/recv.c b/net/rds/recv.c
index cf3884d87931..c7f575bad91c 100644
--- a/net/rds/recv.c
+++ b/net/rds/recv.c
@@ -133,15 +133,16 @@ static void rds_recv_rcvbuf_delta(struct rds_sock *rs, struct sock *sk,
static void rds_conn_peer_gen_update(struct rds_connection *conn,
u32 peer_gen_num)
{
- int i;
+ int npaths = (conn->c_trans->t_mp_capable ? RDS_MPATH_WORKERS : 1);
struct rds_message *rm, *tmp;
unsigned long flags;
+ int i;
WARN_ON(conn->c_trans->t_type != RDS_TRANS_TCP);
if (peer_gen_num != 0) {
if (conn->c_peer_gen_num != 0 &&
peer_gen_num != conn->c_peer_gen_num) {
- for (i = 0; i < RDS_MPATH_WORKERS; i++) {
+ for (i = 0; i < npaths; i++) {
struct rds_conn_path *cp;
cp = &conn->c_path[i];
@@ -210,6 +211,7 @@ static void rds_recv_hs_exthdrs(struct rds_header *hdr,
u32 new_peer_gen_num = 0;
int new_npaths;
bool fan_out;
+ int npaths = (conn->c_trans->t_mp_capable ? RDS_MPATH_WORKERS : 1);
new_npaths = conn->c_npaths;
@@ -221,7 +223,7 @@ static void rds_recv_hs_exthdrs(struct rds_header *hdr,
/* Process extension header here */
switch (type) {
case RDS_EXTHDR_NPATHS:
- new_npaths = min_t(int, RDS_MPATH_WORKERS,
+ new_npaths = min_t(int, npaths,
be16_to_cpu(buffer.rds_npaths));
break;
case RDS_EXTHDR_GEN_NUM:
base-commit: 564973a259ec76f2dad0853420e7034cc43994c4
--
2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2 net] net/rds: fix out-of-bounds write in rds_conn_peer_gen_update()
2026-08-25 12:01 [PATCH v2 net] net/rds: fix out-of-bounds write in rds_conn_peer_gen_update() pamoutaf
@ 2026-08-25 23:16 ` Allison Henderson
2026-08-28 23:05 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: Allison Henderson @ 2026-08-25 23:16 UTC (permalink / raw)
To: pamoutaf
Cc: davem, edumazet, kuba, pabeni, horms, netdev, linux-rdma,
rds-devel, linux-kernel
On Tue, 2026-08-25 at 13:01 +0100, pamoutaf wrote:
> From: Paula Moutafian <paula@bynar.io>
>
> rds_conn_peer_gen_update(), rds_start_mprds() and rds_check_all_paths()
> all iterate conn->c_path[] up to RDS_MPATH_WORKERS (8) or conn->c_npaths,
> but c_path is allocated with only
>
> npaths = (trans->t_mp_capable ? RDS_MPATH_WORKERS : 1)
>
> entries in __rds_conn_create() (net/rds/connection.c). Only the TCP
> transport sets t_mp_capable, so for the IB/RDMA transport (and the loop
> transport) exactly one rds_conn_path is allocated.
>
> c_npaths is derived from the peer via the RDS_EXTHDR_NPATHS handshake
> extension header and is only clamped to RDS_MPATH_WORKERS, never to the
> transport's actual allocation. A remote peer on an RDS/IB (RoCE) fabric
> can therefore complete the unauthenticated handshake and, by sending a
> second RDS_EXTHDR_GEN_NUM with a changed generation number, drive
> rds_conn_peer_gen_update() to walk c_path[1..7] past the end of a
> one-element allocation -- taking cp_lock, writing cp_next_tx_seq /
> cp_next_rx_seq and walking cp_retrans on neighbouring slab objects.
> rds_start_mprds() and rds_check_all_paths() are reachable the same way
> via a peer-supplied c_npaths > 1; note rds_check_all_paths() is a
> do/while and dereferences c_path[0] before testing the bound.
>
> Reproduced against an unmodified 7.2.0 KASAN build, triggered from a
> hand-rolled RDS/IB peer completing the handshake and sending two probes
> with differing RDS_EXTHDR_GEN_NUM:
>
> BUG: KASAN: slab-out-of-bounds in do_raw_spin_lock+0x55/0x9a
> Write of size 4 at addr ff1100000425a624 by task ksoftirqd/0/13
> Call Trace:
> do_raw_spin_lock+0x55/0x9a
> _raw_spin_lock_irqsave+0x12/0x18
> rds_recv_hs_exthdrs+0x34a/0x52a
> rds_recv_incoming+0x5a8/0xb33
> rds_ib_recv_cqe_handler+0xda1/0x12b6
> poll_rcq+0x8e/0xb1
> rds_ib_tasklet_fn_recv+0x1c4/0x348
> Allocated by task 10:
> __rds_conn_create+0x5b3/0x168f
> rds_conn_create+0x18/0x1b
> rds_ib_cm_handle_connect+0x486/0xa35
> The buggy address is located 44 bytes to the right of
> allocated 504-byte region, cache kmalloc-512
>
> The corrupted neighbour's cp_retrans.next is then dereferenced on the
> next line, producing a fatal GPF (RIP: rds_recv_hs_exthdrs+0x3ba/0x52a,
> KASAN: null-ptr-deref) -- a crash, not merely a detected access.
>
> Fix conn->c_npaths at the source instead of guarding every reader: in
> rds_recv_hs_exthdrs() (net/rds/recv.c), clamp the peer-supplied
> RDS_EXTHDR_NPATHS value to the transport's actual per-connection
> allocation rather than to the fixed RDS_MPATH_WORKERS ceiling. c_npaths
> can then never exceed the number of rds_conn_path entries
> __rds_conn_create() allocated, which takes care of rds_start_mprds()
> and rds_check_all_paths() -- both already bound their loops by
> conn->c_npaths -- with no change at either call site.
>
> rds_conn_peer_gen_update() is different: its loop is bound by the fixed
> RDS_MPATH_WORKERS constant, not by conn->c_npaths, so the c_npaths fix
> above does not reach it. It still needs its own bound, computed the same
> way as the allocation site and mirroring the sibling pattern already used
> in rds_conn_destroy() (net/rds/connection.c).
>
> rds_conn_peer_gen_update() and rds_start_mprds() (net/rds/recv.c) were
> both correct when written, with c_path still a fixed
> RDS_MPATH_WORKERS-element array:
>
> commit 905dd4184e07 ("RDS: TCP: Track peer's connection generation number")
> commit 5916e2c1554f ("RDS: TCP: Enable multipath RDS for TCP")
>
> They became wrong when the allocation was made conditional on
> t_mp_capable without updating either loop or the RDS_EXTHDR_NPATHS clamp:
>
> commit 840df162b3eb ("rds: reduce memory footprint for RDS when transport is RDMA")
>
> rds_check_all_paths() (net/rds/connection.c) is unrelated to that
> regression: it was added new, three years later, and was unbounded
> from the moment it was written, but is fixed by the same c_npaths clamp:
>
> commit 9ef845f894c9 ("rds: If one path needs re-connection, check all and re-connect")
>
> Fixes: 840df162b3eb ("rds: reduce memory footprint for RDS when transport is RDMA")
> Fixes: 9ef845f894c9 ("rds: If one path needs re-connection, check all and re-connect")
> Cc: stable@vger.kernel.org
> Assisted-by: Bynario AI
> Signed-off-by: Paula Moutafian <paula@bynar.io>
Ok, this looks good to me. Thanks Paula!
Reviewed-by: Allison Henderson <achender@kernel.org>
> ---
> v2:
> - Per Allison Henderson's review, clamp conn->c_npaths itself at the
> point it is set from the peer's RDS_EXTHDR_NPATHS in
> rds_recv_hs_exthdrs(), instead of adding a second bound check at
> each of its readers. This covers rds_start_mprds() and
> rds_check_all_paths() with no further change at either site;
> net/rds/connection.c is unchanged from v1.
> - rds_conn_peer_gen_update()'s loop is bound by RDS_MPATH_WORKERS
> directly, not by conn->c_npaths, so it is unaffected by the above
> and keeps its own bound from v1.
>
> [v1] https://lore.kernel.org/netdev/213829b7380f1fe12aed2f2ae9ba33c2870addd5.camel@kernel.org/T/#t
>
> net/rds/recv.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/net/rds/recv.c b/net/rds/recv.c
> index cf3884d87931..c7f575bad91c 100644
> --- a/net/rds/recv.c
> +++ b/net/rds/recv.c
> @@ -133,15 +133,16 @@ static void rds_recv_rcvbuf_delta(struct rds_sock *rs, struct sock *sk,
> static void rds_conn_peer_gen_update(struct rds_connection *conn,
> u32 peer_gen_num)
> {
> - int i;
> + int npaths = (conn->c_trans->t_mp_capable ? RDS_MPATH_WORKERS : 1);
> struct rds_message *rm, *tmp;
> unsigned long flags;
> + int i;
>
> WARN_ON(conn->c_trans->t_type != RDS_TRANS_TCP);
> if (peer_gen_num != 0) {
> if (conn->c_peer_gen_num != 0 &&
> peer_gen_num != conn->c_peer_gen_num) {
> - for (i = 0; i < RDS_MPATH_WORKERS; i++) {
> + for (i = 0; i < npaths; i++) {
> struct rds_conn_path *cp;
>
> cp = &conn->c_path[i];
> @@ -210,6 +211,7 @@ static void rds_recv_hs_exthdrs(struct rds_header *hdr,
> u32 new_peer_gen_num = 0;
> int new_npaths;
> bool fan_out;
> + int npaths = (conn->c_trans->t_mp_capable ? RDS_MPATH_WORKERS : 1);
>
> new_npaths = conn->c_npaths;
>
> @@ -221,7 +223,7 @@ static void rds_recv_hs_exthdrs(struct rds_header *hdr,
> /* Process extension header here */
> switch (type) {
> case RDS_EXTHDR_NPATHS:
> - new_npaths = min_t(int, RDS_MPATH_WORKERS,
> + new_npaths = min_t(int, npaths,
> be16_to_cpu(buffer.rds_npaths));
> break;
> case RDS_EXTHDR_GEN_NUM:
>
> base-commit: 564973a259ec76f2dad0853420e7034cc43994c4
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2 net] net/rds: fix out-of-bounds write in rds_conn_peer_gen_update()
2026-08-25 12:01 [PATCH v2 net] net/rds: fix out-of-bounds write in rds_conn_peer_gen_update() pamoutaf
2026-08-25 23:16 ` Allison Henderson
@ 2026-08-28 23:05 ` Jakub Kicinski
1 sibling, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-08-28 23:05 UTC (permalink / raw)
To: paula
Cc: Jakub Kicinski, achender, davem, edumazet, pabeni, horms, netdev,
linux-rdma, rds-devel, linux-kernel
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net/rds: fix out-of-bounds write in rds_conn_peer_gen_update()
This clamps the peer-supplied RDS_EXTHDR_NPATHS value in
rds_recv_hs_exthdrs() to the number of rds_conn_path entries the transport
actually allocated, and gives the loop in rds_conn_peer_gen_update() the
same bound instead of the fixed RDS_MPATH_WORKERS constant.
Reference to the earlier posting:
[v1] https://lore.kernel.org/netdev/213829b7380f1fe12aed2f2ae9ba33c2870addd5.camel@kernel.org/T/#t
> diff --git a/net/rds/recv.c b/net/rds/recv.c
> index cf3884d879319..c7f575bad91c8 100644
> --- a/net/rds/recv.c
> +++ b/net/rds/recv.c
> @@ -133,15 +133,16 @@ static void rds_recv_rcvbuf_delta(struct rds_sock *rs, struct sock *sk,
> static void rds_conn_peer_gen_update(struct rds_connection *conn,
> u32 peer_gen_num)
> {
> - int i;
> + int npaths = (conn->c_trans->t_mp_capable ? RDS_MPATH_WORKERS : 1);
> struct rds_message *rm, *tmp;
> unsigned long flags;
> + int i;
[Severity: High]
This isn't a bug introduced by this patch, but is
conn->c_trans->t_mp_capable really equivalent to the count used at the
allocation site?
__rds_conn_create() derives npaths from the incoming trans, allocates
c_path plus one ordered workqueue per path, and only afterwards may replace
the transport:
net/rds/connection.c:__rds_conn_create() {
int npaths = (trans->t_mp_capable ? RDS_MPATH_WORKERS : 1);
...
trans = &rds_loop_transport;
...
conn->c_trans = trans;
for (i = 0; i < npaths; i++) {
...
cp->cp_wq = alloc_ordered_workqueue(...);
For an outgoing TCP connection to a local non-loopback address
(rds_tcp_transport has t_prefer_loopback set) the conn therefore ends up
with 8 paths and 8 workqueues while conn->c_trans is rds_loop_transport,
which has t_mp_capable clear.
rds_conn_destroy() recomputes the count exactly the way this hunk does:
net/rds/connection.c:rds_conn_destroy() {
int npaths = (conn->c_trans->t_mp_capable ? RDS_MPATH_WORKERS : 1);
...
for (i = 0; i < npaths; i++) {
cp = &conn->c_path[i];
rds_conn_path_destroy(cp);
...
kfree(conn->c_path);
Does that leak the cp_wq workqueues belonging to paths 1..7 in this case?
Note rds_conn_path_destroy() also returns early before destroy_workqueue():
net/rds/connection.c:rds_conn_path_destroy() {
if (!cp->cp_transport_data)
return;
...
if (cp->cp_wq != rds_wq) {
destroy_workqueue(cp->cp_wq);
so simply widening the destroy loop would not reach the workqueues either.
The failure path in __rds_conn_create() uses the pre-swap npaths to destroy
all the workqueues, which suggests npaths, not the post-swap transport, is
the real allocation count.
The derivation is harmless in this patch (1 is always <= the allocation),
but would recording the allocated path count in struct rds_connection and
using that here avoid duplicating the derivation in a third place?
>
> WARN_ON(conn->c_trans->t_type != RDS_TRANS_TCP);
> if (peer_gen_num != 0) {
> if (conn->c_peer_gen_num != 0 &&
> peer_gen_num != conn->c_peer_gen_num) {
> - for (i = 0; i < RDS_MPATH_WORKERS; i++) {
> + for (i = 0; i < npaths; i++) {
> struct rds_conn_path *cp;
>
> cp = &conn->c_path[i];
[Severity: High]
The new bound stops the walk past the end of c_path[], but does the rest of
this function stay reachable from the same non-TCP peer described in the
changelog?
rds_recv_incoming() dispatches to the handshake parser purely on
peer-supplied header fields, with no transport check:
net/rds/recv.c:rds_recv_incoming() {
if (be16_to_cpu(inc->i_hdr.h_dport) == RDS_FLAG_PROBE_PORT &&
inc->i_hdr.h_sport == 0) {
rds_recv_hs_exthdrs(&inc->i_hdr, cp->cp_conn);
and rds_recv_hs_exthdrs() calls rds_conn_peer_gen_update() unconditionally.
rds_ib_process_recv() hands the wire header straight to rds_recv_incoming(),
and rds_ib_transport sets .t_type = RDS_TRANS_IB, so the WARN_ON above is
true for every crafted probe on an IB connection.
Can that flood dmesg from a remote peer, or panic the box on
panic_on_warn=1 kernels? Would ratelimiting it, or refusing handshake
extension headers on non mp-capable transports, be preferable?
With npaths == 1 the i == 0 iteration is still executed. Two probes with
differing non-zero RDS_EXTHDR_GEN_NUM let the same peer reset
c_path[0].cp_next_tx_seq to 1 and cp_next_rx_seq to 0 and set RDS_MSG_FLUSH
on every message queued on c_path[0].cp_retrans of a live IB connection.
rds_send_probe() only ever emits these headers for t_mp_capable transports:
net/rds/send.c:rds_send_probe() {
if (RDS_HS_PROBE(be16_to_cpu(sport), be16_to_cpu(dport)) &&
cp->cp_conn->c_trans->t_mp_capable) {
so on IB both NPATHS and GEN_NUM are always forged. Would an early return in
rds_recv_hs_exthdrs() for transports without t_mp_capable remove both the
WARN and the c_path[0] state mutation, and make the new bound here
unnecessary?
[Severity: Medium]
This is a pre-existing issue, but the compare-then-store of
conn->c_peer_gen_num in this function happens outside any lock — cp_lock
only covers the per-path queue walk:
if (peer_gen_num != 0) {
if (conn->c_peer_gen_num != 0 &&
peer_gen_num != conn->c_peer_gen_num) {
...
conn->c_peer_gen_num = peer_gen_num;
For a t_mp_capable conn each path has its own cp_wq and receive worker, so
two lanes can run rds_recv_hs_exthdrs(cp->cp_conn) for the same conn at the
same time. Can two probes carrying different generation numbers then either
double-flush or skip the RDS_MSG_FLUSH marking entirely, leaving stale
cp_retrans messages after a peer restart?
> @@ -210,6 +211,7 @@ static void rds_recv_hs_exthdrs(struct rds_header *hdr,
> u32 new_peer_gen_num = 0;
> int new_npaths;
> bool fan_out;
> + int npaths = (conn->c_trans->t_mp_capable ? RDS_MPATH_WORKERS : 1);
[Severity: Low]
This isn't a bug, but the new declaration is the longest one in the block
and is placed last, which does not follow the reverse xmas tree ordering
netdev asks for. The first hunk of this patch moves int i; below
unsigned long flags; for exactly that reason, so the two hunks disagree.
>
> new_npaths = conn->c_npaths;
>
> @@ -221,7 +223,7 @@ static void rds_recv_hs_exthdrs(struct rds_header *hdr,
> /* Process extension header here */
> switch (type) {
> case RDS_EXTHDR_NPATHS:
> - new_npaths = min_t(int, RDS_MPATH_WORKERS,
> + new_npaths = min_t(int, npaths,
> be16_to_cpu(buffer.rds_npaths));
> break;
> case RDS_EXTHDR_GEN_NUM:
[Severity: Medium]
This is a pre-existing issue, but conn->c_npaths — now the single
authoritative bound for rds_start_mprds() and rds_check_all_paths() — is
read, tested and published here with no lock, no atomics and no
READ_ONCE()/WRITE_ONCE():
net/rds/recv.c:rds_recv_hs_exthdrs() {
new_npaths = conn->c_npaths;
...
if (new_npaths > 1 && new_npaths != conn->c_npaths) {
...
conn->c_npaths = max_t(int, new_npaths, 1);
Since each path of an mp-capable conn has its own receive worker, two lanes
can be in this function for the same conn concurrently. Can one CPU publish
c_npaths = 8 and take the fan-out branch while a second CPU, processing a
probe with no NPATHS extension, writes c_npaths = 1 and silently reverts it
while lanes are still coming up?
The lane mapping in rds_sendmsg() reads the same field with no
synchronisation:
net/rds/send.c:rds_sendmsg() {
cpath = &conn->c_path[RDS_MPATH_HASH(rs, conn->c_npaths ? : 1)];
so the local and peer lane hashing could disagree. Every stored value still
passes through max_t(int, new_npaths, 1) with new_npaths already clamped, so
this looks like a consistency problem rather than an indexing one — is that
the intended reading?
The comment above rds_recv_hs_exthdrs() only promises serialisation against
other receives on "this specific conn", which for mprds is per-path. Would
WRITE_ONCE()/READ_ONCE() plus per-conn serialisation of handshake
processing be the right follow-up?
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-28 23:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-25 12:01 [PATCH v2 net] net/rds: fix out-of-bounds write in rds_conn_peer_gen_update() pamoutaf
2026-08-25 23:16 ` Allison Henderson
2026-08-28 23:05 ` Jakub Kicinski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®