Service connections use peer-selected CIDs and do not own client IDR entries, but the common destructor removes those IDs. This can delete a live client connection or race allocation and corrupt the IDR and its RCU callbacks. Client destruction can also repeat an earlier removal and delete a reused ID. Restrict client cleanup to clients. Restore a per-local writer lock and an ownership flag so each ID is released once. Release IDs at idle retirement even if the bundle slot was replaced; otherwise deferred final puts can leave the IDR nonempty at endpoint teardown and trigger BUG(). Keeping IDs until retirement preserves active calls and terminal retransmissions. Client-only cleanup also prevents nr_client_conns underflow and restores the intended expiry selection without changing the timeout values. The issue was found by an LLM. Fixes: 9d35d880e0e4 ("rxrpc: Move client call connection to the I/O thread") Cc: stable@vger.kernel.org Signed-off-by: Kyle Zeng --- v2: - Release a retired client connection's ID even if its bundle slot has been replaced. Keep the ID until retirement so active calls and terminal retransmissions remain reachable. - Describe the nr_client_conns accounting correction and its effect on normal-versus-fast expiry selection. - Repeat the RxRPC module and targeted x86 allmodconfig/allyesconfig W=1 builds. The reproducer was not rerun for v2. v1: https://lore.kernel.org/r/20261030042129.73690-1-kylebot@openai.com Review: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261030042129.73690-1-kylebot%40openai.com I have a reproducer for this issue and can share via DM net/rxrpc/ar-internal.h | 2 ++ net/rxrpc/conn_client.c | 21 +++++++++++++++++---- net/rxrpc/conn_object.c | 3 ++- net/rxrpc/local_object.c | 1 + 4 files changed, 22 insertions(+), 5 deletions(-) diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h index 865f05fe37ab..5e49fe0f4a6f 100644 --- a/net/rxrpc/ar-internal.h +++ b/net/rxrpc/ar-internal.h @@ -377,6 +377,7 @@ struct rxrpc_local { bool dead; bool service_closed; /* Service socket closed */ struct idr conn_ids; /* List of connection IDs */ + spinlock_t conn_ids_lock; /* Lock for conn_ids */ struct list_head new_client_calls; /* Newly created client calls need connection */ spinlock_t client_call_lock; /* Lock for ->new_client_calls */ struct sockaddr_rxrpc srx; /* local address */ @@ -472,6 +473,7 @@ enum rxrpc_call_completion { * Bits in the connection flags. */ enum rxrpc_conn_flag { + RXRPC_CONN_HAS_IDR, /* Has a client conn ID assigned */ RXRPC_CONN_IN_SERVICE_CONNS, /* Conn is in peer->service_conns */ RXRPC_CONN_DONT_REUSE, /* Don't reuse this connection */ RXRPC_CONN_PROBING_FOR_UPGRADE, /* Probing for service upgrade */ diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c index 48519f0de185..201a50caeaca 100644 --- a/net/rxrpc/conn_client.c +++ b/net/rxrpc/conn_client.c @@ -40,12 +40,16 @@ static void rxrpc_activate_bundle(struct rxrpc_bundle *bundle) } /* - * Release a connection ID for a client connection. + * Release a client connection ID exactly once. The final reference may be + * dropped outside the local endpoint's I/O thread. */ static void rxrpc_put_client_connection_id(struct rxrpc_local *local, struct rxrpc_connection *conn) { - idr_remove(&local->conn_ids, conn->proto.cid >> RXRPC_CIDSHIFT); + spin_lock(&local->conn_ids_lock); + if (test_and_clear_bit(RXRPC_CONN_HAS_IDR, &conn->flags)) + idr_remove(&local->conn_ids, conn->proto.cid >> RXRPC_CIDSHIFT); + spin_unlock(&local->conn_ids_lock); } /* @@ -56,6 +60,7 @@ static void rxrpc_destroy_client_conn_ids(struct rxrpc_local *local) struct rxrpc_connection *conn; int id; + spin_lock(&local->conn_ids_lock); if (!idr_is_empty(&local->conn_ids)) { idr_for_each_entry(&local->conn_ids, conn, id) { pr_err("AF_RXRPC: Leaked client conn %p {%d}\n", @@ -65,6 +70,7 @@ static void rxrpc_destroy_client_conn_ids(struct rxrpc_local *local) } idr_destroy(&local->conn_ids); + spin_unlock(&local->conn_ids_lock); } /* @@ -162,15 +168,20 @@ rxrpc_alloc_client_connection(struct rxrpc_bundle *bundle) if (!conn) return ERR_PTR(-ENOMEM); + spin_lock(&local->conn_ids_lock); id = idr_alloc_cyclic(&local->conn_ids, conn, 1, 0x40000000, GFP_ATOMIC | __GFP_NOWARN); + if (id >= 0) { + conn->proto.cid = id << RXRPC_CIDSHIFT; + set_bit(RXRPC_CONN_HAS_IDR, &conn->flags); + } + spin_unlock(&local->conn_ids_lock); if (id < 0) { kfree(conn); return ERR_PTR(id); } refcount_set(&conn->ref, 1); - conn->proto.cid = id << RXRPC_CIDSHIFT; conn->proto.epoch = local->rxnet->epoch; conn->out_clientflag = RXRPC_CLIENT_INITIATED; conn->bundle = rxrpc_get_bundle(bundle, rxrpc_bundle_get_client_conn); @@ -682,6 +693,9 @@ static void rxrpc_unbundle_conn(struct rxrpc_connection *conn) if (conn->flags & RXRPC_CONN_FINAL_ACK_MASK) rxrpc_process_delayed_final_acks(conn, true); + /* A replaced connection is no longer in the bundle. */ + rxrpc_put_client_connection_id(bundle->local, conn); + bindex = conn->bundle_shift / RXRPC_MAXCALLS; if (bundle->conns[bindex] == conn) { _debug("clear slot %u", bindex); @@ -689,7 +703,6 @@ static void rxrpc_unbundle_conn(struct rxrpc_connection *conn) bundle->conn_ids[bindex] = 0; for (i = 0; i < RXRPC_MAXCALLS; i++) clear_bit(conn->bundle_shift + i, &bundle->avail_chans); - rxrpc_put_client_connection_id(bundle->local, conn); rxrpc_deactivate_bundle(bundle); rxrpc_put_connection(conn, rxrpc_conn_put_unbundle); } diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c index 0ece717db0f8..bae95cadd72b 100644 --- a/net/rxrpc/conn_object.c +++ b/net/rxrpc/conn_object.c @@ -332,7 +332,8 @@ static void rxrpc_clean_up_connection(struct work_struct *work) rxrpc_purge_queue(&conn->rx_queue); rxrpc_free_skb(conn->tx_response, rxrpc_skb_put_response); - rxrpc_kill_client_conn(conn); + if (rxrpc_conn_is_client(conn)) + rxrpc_kill_client_conn(conn); conn->security->clear(conn); key_put(conn->key); diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c index 169f9dfdaa77..5a7acd595e11 100644 --- a/net/rxrpc/local_object.c +++ b/net/rxrpc/local_object.c @@ -140,6 +140,7 @@ static struct rxrpc_local *rxrpc_alloc_local(struct net *net, memcpy(&local->srx, srx, sizeof(*srx)); local->srx.srx_service = 0; idr_init(&local->conn_ids); + spin_lock_init(&local->conn_ids_lock); get_random_bytes(&tmp, sizeof(tmp)); tmp &= 0x3fffffff; if (tmp == 0) base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9 -- 2.55.0.openai.911.g06873a30d2f3