[PATCH] rds: fix out-of-bounds conn_path access via handshake NPATHS

[PATCH] rds: fix out-of-bounds conn_path access via handshake NPATHS

[Henry Martin]

Connections on the IB transport (t_mp_capable unset) own exactly one
rds_conn_path slot, but the peer can still claim eight lanes through
the NPATHS extension header of a handshake packet, and every
subsequent fan-out path then treats all eight entries as real: the
mp-start helper iterates conn->c_path[0..7], dereferencing cp->cp_conn,
taking cp->cp_lock and queueing delayed work on cp->cp_wq at 504-byte
strides into whatever comes after the single allocated slot (KASAN:
"Read of size 8 in rds_conn_path_connect_if_down").

The negotiated lane count is now clamped to the same value the
transport was allocated with (trans->t_mp_capable ?
RDS_MPATH_WORKERS : 1), so a peer can never ask us to walk lanes that
do not exist.

This vulnerability was discovered by Tencent CodeBuddy Security.

Cc: stable@vger.kernel.org
Fixes: ba3d1f480c7a ("net/rds: size a connection's path set by the transport it ends up with")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
 net/rds/recv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--- a/net/rds/recv.c
+++ b/net/rds/recv.c
@@ -224,7 +224,8 @@
 		/* Process extension header here */
 		switch (type) {
 		case RDS_EXTHDR_NPATHS:
-			new_npaths = min_t(int, RDS_MPATH_WORKERS,
+			new_npaths = min_t(int, conn->c_trans->t_mp_capable ?
+					   RDS_MPATH_WORKERS : 1,
 					   be16_to_cpu(buffer.rds_npaths));
 			break;
 		case RDS_EXTHDR_GEN_NUM:
--
2.43.7

Re: [PATCH] rds: fix out-of-bounds conn_path access via handshake NPATHS

[sashiko-bot]

> Connections on the IB transport (t_mp_capable unset) own exactly one
> rds_conn_path slot, but the peer can still claim eight lanes through
> the NPATHS extension header of a handshake packet, and every
> subsequent fan-out path then treats all eight entries as real: the
> mp-start helper iterates conn->c_path[0..7], dereferencing cp->cp_conn,
> taking cp->cp_lock and queueing delayed work on cp->cp_wq at 504-byte
> strides into whatever comes after the single allocated slot (KASAN:
> "Read of size 8 in rds_conn_path_connect_if_down").
> 
> The negotiated lane count is now clamped to the same value the
> transport was allocated with (trans->t_mp_capable ?
> RDS_MPATH_WORKERS : 1), so a peer can never ask us to walk lanes that
> do not exist.
> 
> This vulnerability was discovered by Tencent CodeBuddy Security.
> 
> Cc: stable@vger.kernel.org
> Fixes: ba3d1f480c7a ("net/rds: size a connection's path set by the transport it ends up with")
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008163843.957325-1-bsdhenrymartin@gmail.com?part=1

Re: [PATCH] rds: fix out-of-bounds conn_path access via handshake NPATHS

[Allison Henderson]

On Fri, 2026-10-09 at 00:38 +0800, Henry Martin wrote:
> Connections on the IB transport (t_mp_capable unset) own exactly one
> rds_conn_path slot, but the peer can still claim eight lanes through
> the NPATHS extension header of a handshake packet, and every
> subsequent fan-out path then treats all eight entries as real: the
> mp-start helper iterates conn->c_path[0..7], dereferencing cp->cp_conn,
> taking cp->cp_lock and queueing delayed work on cp->cp_wq at 504-byte
> strides into whatever comes after the single allocated slot (KASAN:
> "Read of size 8 in rds_conn_path_connect_if_down").
> 
> The negotiated lane count is now clamped to the same value the
> transport was allocated with (trans->t_mp_capable ?
> RDS_MPATH_WORKERS : 1), so a peer can never ask us to walk lanes that
> do not exist.
> 
> This vulnerability was discovered by Tencent CodeBuddy Security.

Hi Henry,

Thanks for the patches.  This patch looked a little familiar, and it turns out this fix and the
companion patch ("fix out-of-bounds conn_path walk in peer gen update") are the same fix Paula
posted in August:

https://lore.kernel.org/netdev/20260825120132.51636-1-pamoutafpro@gmail.com/

The AI review of that version came back with some complaints:

https://lore.kernel.org/netdev/20260828230506.385788-1-kuba@kernel.org/

The good news is, I think the fix is a very simple port of a uek patch:
https://github.com/oracle/linux-uek/commit/2fdeb0513f18

If your willing to step in where this series left off, you'll want to start with the v2 thread,
and pick up the porting task described there:
https://lore.kernel.org/netdev/83915f73efb509328a3d4963b093e2cffcf9af99.camel@kernel.org/

Please send a single v2 that is just that port, against net.  The fix tag ba3d1f480c7a doesnt
look like it changes the allocations. The tags you want should be:
Fixes: 840df162b3eb ("rds: reduce memory footprint for RDS when transport is RDMA")
Fixes: 905dd4184e07 ("RDS: TCP: Track peer's connection generation number")

A link to Paula's v2 would also be a nice way to credit her analysis as well.

Thanks!
Allison

> 
> Cc: stable@vger.kernel.org
> Fixes: ba3d1f480c7a ("net/rds: size a connection's path set by the transport it ends up with")
> Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
> ---
>  net/rds/recv.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> --- a/net/rds/recv.c
> +++ b/net/rds/recv.c
> @@ -224,7 +224,8 @@
>  		/* Process extension header here */
>  		switch (type) {
>  		case RDS_EXTHDR_NPATHS:
> -			new_npaths = min_t(int, RDS_MPATH_WORKERS,
> +			new_npaths = min_t(int, conn->c_trans->t_mp_capable ?
> +					   RDS_MPATH_WORKERS : 1,
>  					   be16_to_cpu(buffer.rds_npaths));
>  			break;
>  		case RDS_EXTHDR_GEN_NUM:
> --
> 2.43.7

Re: [PATCH] rds: fix out-of-bounds conn_path access via handshake NPATHS

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 1 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Critical] This fix leaves the same out-of-bounds walk reachable
  through another lane loop.
- [Medium] The Fixes: tag names the wrong commit.

--

rds: fix out-of-bounds conn_path access via handshake NPATHS

This patch clamps the RDS_EXTHDR_NPATHS value the peer sends in
rds_recv_hs_exthdrs() to the number of rds_conn_path slots the transport
was allocated with. As a result, IB connections (t_mp_capable unset)
negotiate at most one lane. The aim is to stop fan-out paths such as
rds_start_mprds() from walking past the single allocated c_path entry.

> Cc: stable@vger.kernel.org
> Fixes: ba3d1f480c7a ("net/rds: size a connection's path set by the transport it ends up with")

[Severity: Medium]
Is this the right commit for the Fixes: tag?

All ba3d1f480c7a does is recompute npaths a second time after the
loopback swap in __rds_conn_create():

    npaths = (trans->t_mp_capable ? RDS_MPATH_WORKERS : 1);

Its own message says "The c_path array stays sized for the caller's
transport". So it doesn't change how many slots an IB connection gets.

IB connections have had a one-element c_path since 840df162b3eb ("rds:
reduce memory footprint for RDS when transport is RDMA"). That commit
replaced the inline c_path[RDS_MPATH_WORKERS] array with an allocation
sized by:

    int npaths = (trans->t_mp_capable ? RDS_MPATH_WORKERS : 1);

The NPATHS clamp to RDS_MPATH_WORKERS and the rds_start_mprds() loop over
c_npaths are even older (5916e2c1554f and 69b92b5b7419).

Stable tooling chooses target trees from the Fixes: tag. Would LTS trees
that carry 840df162b3eb but not ba3d1f480c7a miss this fix? Should the
tag be:

Fixes: 840df162b3eb ("rds: reduce memory footprint for RDS when transport is RDMA")

> diff --git a/net/rds/recv.c b/net/rds/recv.c
> index 6204e577a90ae..c9528812edef1 100644
> --- a/net/rds/recv.c
> +++ b/net/rds/recv.c
> @@ -224,7 +224,8 @@ 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, conn->c_trans->t_mp_capable ?
> +					   RDS_MPATH_WORKERS : 1,
>  					   be16_to_cpu(buffer.rds_npaths));
>  			break;
>  		case RDS_EXTHDR_GEN_NUM:

[Severity: Critical]
This clamps NPATHS. Can the RDS_EXTHDR_GEN_NUM case just below it still
cause the same out-of-bounds walk on IB connections?

rds_recv_hs_exthdrs() always calls rds_conn_peer_gen_update() at the end.
The call does not depend on c_npaths, fan_out or t_mp_capable:

    conn->c_ping_triggered = 0;
    rds_conn_peer_gen_update(conn, new_peer_gen_num);

rds_conn_peer_gen_update() then loops over a fixed RDS_MPATH_WORKERS
lanes:

net/rds/recv.c:rds_conn_peer_gen_update() {
    ...
	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++) {
				struct rds_conn_path *cp;

				cp = &conn->c_path[i];
				spin_lock_irqsave(&cp->cp_lock, flags);
				cp->cp_next_tx_seq = 1;
				cp->cp_next_rx_seq = 0;
				list_for_each_entry_safe(rm, tmp,
							 &cp->cp_retrans,
							 m_conn_item) {
					set_bit(RDS_MSG_FLUSH, &rm->m_flags);
				}
    ...
}

The WARN_ON() only prints a warning. It doesn't return.

On IB, __rds_conn_create() allocates a single c_path slot. A peer can
send two handshake probes carrying different nonzero GEN_NUM values. A
pong (h_sport == 0, h_dport == RDS_FLAG_PROBE_PORT) has no sysctl or
transport gate. A ping also works if rds_sysctl_ping_enable is set. The
call chain is:

rds_ib_process_recv()
  rds_recv_incoming()
    rds_recv_hs_exthdrs()
      rds_conn_peer_gen_update()

The first probe stores c_peer_gen_num, and the second enters the loop.
For i=1..7, would the loop take a spinlock and write cp_next_tx_seq and
cp_next_rx_seq in heap memory past the allocated slot? It would also walk
a cp_retrans list head read from that memory and call set_bit() through
whatever pointers it finds there.

If so, the commit message's claim "so a peer can never ask us to walk
lanes that do not exist" doesn't hold yet.

Could this loop also be bounded by:

    conn->c_trans->t_mp_capable ? RDS_MPATH_WORKERS : 1

which is the count __rds_conn_create() already uses? Another option is to
skip handshake extension processing entirely on transports that are not
mp-capable.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261008163843.957325-1-bsdhenrymartin%40gmail.com