mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
net: rds: fix uninitialized trans dereference in CM event handler
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: dcdede0406 ("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>
This commit is contained in:
parent
4f6ac74fa5
commit
492ba6ac1b
|
|
@ -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,11 +79,32 @@ 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;
|
||||
|
||||
case RDMA_CM_EVENT_ADDR_RESOLVED:
|
||||
if (conn) {
|
||||
/* The address resolved to a device RDS has no
|
||||
* transport for. Do not go on to resolve a route:
|
||||
* an iWARP route leaves cm_id->route.path_rec
|
||||
* unset, which the ROUTE_RESOLVED case below
|
||||
* dereferences. Drop the connection instead; the
|
||||
* id is still ic->i_cm_id, so return 0 and let the
|
||||
* shutdown destroy it.
|
||||
*/
|
||||
if (cm_id->device->node_type != RDMA_NODE_IB_CA) {
|
||||
rds_conn_drop(conn);
|
||||
break;
|
||||
}
|
||||
rdma_set_service_type(cm_id, conn->c_tos);
|
||||
rdma_set_min_rnr_timer(cm_id, IB_RNR_TIMER_000_32);
|
||||
/* XXX do we need to clean up if this fails? */
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user