[PATCH net v3] net: rds: fix uninitialized trans dereference in CM event handler

[PATCH net v3] net: rds: fix uninitialized trans dereference in CM event handler

[Allison Henderson]

From: Aohan Mei <henrymei@tencent.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 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,
and reject a connect request that arrives on a non-IB device.  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.

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; changelog adjusted]
Assisted-by: Claude-Code:claude-fable-5
Signed-off-by: Allison Henderson <achender@kernel.org>
---
Carrying Aohan's fix forward as v3, since the v2 thread has been quiet
since 2026-08-28 and the crash is still there for stable kernels.

v3 (achender): reject only on RDMA_CM_EVENT_CONNECT_REQUEST rather than
    bail out for every event on a non-IB device - a non-zero return has
    rdma_cm destroy the id the event was delivered on, which is right
    for a request's fresh id but would free a connection id RDS still
    owns for any other event.  Rebased onto current net.
v2: https://lore.kernel.org/netdev/20260825021223.3483044-1-ljp1205831794@gmail.com/
v1: https://lore.kernel.org/netdev/20260824111701.2979194-1-ljp1205831794@gmail.com/

 net/rds/rdma_transport.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/rds/rdma_transport.c b/net/rds/rdma_transport.c
index b15cf316b23a..164bd7cbe208 100644
--- a/net/rds/rdma_transport.c
+++ b/net/rds/rdma_transport.c
@@ -52,7 +52,7 @@ static int rds_rdma_cm_event_handler_cmn(struct rdma_cm_id *cm_id,
 {
 	/* this can be null in the listening path */
 	struct rds_connection *conn = cm_id->context;
-	struct rds_transport *trans;
+	struct rds_transport *trans = &rds_ib_transport;
 	int ret = 0;
 	int *err;
 	u8 len;
@@ -60,9 +60,6 @@ static int rds_rdma_cm_event_handler_cmn(struct rdma_cm_id *cm_id,
 	rdsdebug("conn %p id %p handling event %u (%s)\n", conn, cm_id,
 		 event->event, rdma_event_msg(event->event));
 
-	if (cm_id->device->node_type == RDMA_NODE_IB_CA)
-		trans = &rds_ib_transport;
-
 	/* Prevent shutdown from tearing down the connection
 	 * while we're executing. */
 	if (conn) {
@@ -82,6 +79,15 @@ static int rds_rdma_cm_event_handler_cmn(struct rdma_cm_id *cm_id,
 
 	switch (event->event) {
 	case RDMA_CM_EVENT_CONNECT_REQUEST:
+		/* Only the IB transport is supported, but RDS listens on
+		 * every RDMA device: reject a request that arrived on any
+		 * other kind.  A non-zero return has rdma_cm destroy the
+		 * request's id, which is what we want here and only here.
+		 */
+		if (cm_id->device->node_type != RDMA_NODE_IB_CA) {
+			ret = 1;
+			break;
+		}
 		ret = trans->cm_handle_connect(cm_id, event, isv6);
 		break;
 
-- 
2.25.1

Re: [PATCH net v3] net: rds: fix uninitialized trans dereference in CM event handler

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · 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).

- [High] The fix only checks the device type on the passive side, and
  only for RDMA_CM_EVENT_CONNECT_REQUEST.

--

net: rds: fix uninitialized trans dereference in CM event handler

This commit initializes trans to &rds_ib_transport in
rds_rdma_cm_event_handler_cmn() and drops the conditional assignment on
the node type. It also rejects RDMA_CM_EVENT_CONNECT_REQUEST when the
request arrives on a non-IB device, so an RDS listener no longer crashes
when an iWARP device gets a connect request.

Link: https://lore.kernel.org/netdev/20260824111701.2979194-1-ljp1205831794@gmail.com/
Link: https://lore.kernel.org/netdev/20260825021223.3483044-1-ljp1205831794@gmail.com/

> diff --git a/net/rds/rdma_transport.c b/net/rds/rdma_transport.c
> index b15cf316b23a2..164bd7cbe2083 100644
> --- a/net/rds/rdma_transport.c
> +++ b/net/rds/rdma_transport.c

[ ... ]

> @@ -82,6 +79,15 @@ static int rds_rdma_cm_event_handler_cmn(struct rdma_cm_id *cm_id,
>  
>  	switch (event->event) {
>  	case RDMA_CM_EVENT_CONNECT_REQUEST:
> +		/* Only the IB transport is supported, but RDS listens on
> +		 * every RDMA device: reject a request that arrived on any
> +		 * other kind.  A non-zero return has rdma_cm destroy the
> +		 * request's id, which is what we want here and only here.
> +		 */
> +		if (cm_id->device->node_type != RDMA_NODE_IB_CA) {
> +			ret = 1;
> +			break;
> +		}

[Severity: High]
This check only covers the passive side, and only for
RDMA_CM_EVENT_CONNECT_REQUEST. Can an outgoing RDS/IB connection still
end up on an iWARP RNIC? If so, would it oops in the
RDMA_CM_EVENT_ROUTE_RESOLVED case of this same handler?

rds_ib_conn_path_connect() creates the id without restricting its node
type:

net/rds/ib_cm.c:rds_ib_conn_path_connect() {
    ...
	ic->i_cm_id = rdma_create_id(&init_net, handler, conn,
				     RDMA_PS_TCP, IB_QPT_RC);
    ...
	ret = rdma_resolve_addr(ic->i_cm_id, (struct sockaddr *)&src,
				(struct sockaddr *)&dest,
				RDS_RDMA_RESOLVE_TIMEOUT_MS);
    ...
}

restricted_node_type stays RDMA_NODE_UNSPECIFIED. That means
cma_acquire_dev_by_src_ip() can pick any device that matches
conn->c_laddr, including an RNIC:

drivers/infiniband/core/cma.c:cma_acquire_dev_by_src_ip() {
    ...
		if (id_priv->restricted_node_type != RDMA_NODE_UNSPECIFIED &&
		    id_priv->restricted_node_type != cma_dev->device->node_type)
			continue;
    ...
}

After ADDR_RESOLVED, rdma_resolve_route() on an iWARP port goes to
cma_resolve_iw_route(). That function queues ROUTE_RESOLVED but never
allocates cm_id->route.path_rec. The handler then runs:

	case RDMA_CM_EVENT_ROUTE_RESOLVED:
		...
			if (ibic && ibic->i_cm_id == cm_id) {
				cm_id->route.path_rec[0].sl =
					TOS_TO_SL(conn->c_tos);
				ret = trans->cm_initiate_connect(cm_id, isv6);

Isn't that a write through a NULL path_rec in the cma_wq worker? It
would happen with id_priv->handler_mutex and conn->c_cm_lock held. The
write comes before trans is used, so initializing trans doesn't prevent
it.

There seem to be two ways to reach this.

The first is remote. On a host with both an IB_CA device and an iWARP
device, a peer sends an RDS REQ over the IB device, so the new check
passes. In the private data, dp_daddr is the local iWARP interface
address and dp_saddr is a neighbour on that subnet.

rds_ib_cm_handle_connect() hands these addresses to rds_conn_create()
without checking that laddr belongs to the device the request arrived
on. The connection is hashed.

After a disconnect or setup failure, the reconnect then uses the
addresses the peer chose:

rds_conn_shutdown()
  rds_queue_reconnect()
    rds_connect_worker()
      rds_ib_conn_path_connect()

The lowest-address rule in rds_queue_reconnect() only applies to TCP,
so it doesn't stop this.

The second is local. A socket bound to an address on an IB device
passes rds_ib_laddr_check_cm() at bind time. If that address later
moves to an iWARP netdev, rds_sendmsg() reuses rs->rs_transport and
doesn't check the device again.

The same crash was possible before this patch. However, the commit
message lists ROUTE_RESOLVED among the cases that crash on non-IB
devices, and says:

  "IB is the only transport left, so make that explicit"

while only incoming requests are rejected.

Would it work to call rdma_restrict_node_type(id, RDMA_NODE_IB_CA) on
the ids RDS creates, meaning the active id in
rds_ib_conn_path_connect() and the listen ids? smbdirect already uses
this helper. cma_acquire_dev_by_src_ip() and cma_listen_on_dev() both
honour it, and it could replace the per-event check here.

Another option is to check the device type on ADDR_RESOLVED or
ROUTE_RESOLVED, then call rds_conn_drop() and return 0, so RDS keeps
ownership of the id.

>  		ret = trans->cm_handle_connect(cm_id, event, isv6);
>  		break;
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260928044507.335883-1-achender%40kernel.org