Re: [PATCH net-next v7 05/12] net/rds: split connection destroy into quiesce and kref-governed free

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: rds: permanent connection leak via orphaned probe message on concurrent teardown
--

commit 85ee160094b05c4fe02ec81d1c727379b584a32f
Author: Sharath Srinivasan <sharath.srinivasan@oracle.com>

net/rds: split connection destroy into quiesce and kref-governed free

This commit introduces a refcounting framework for RDS connections, splitting
the teardown process into synchronous quiescing and deferred freeing triggered
when the last reference is dropped.

> diff --git a/net/rds/connection.c b/net/rds/connection.c
> index cbc49426ba085..638e9f3140e28 100644
> --- a/net/rds/connection.c
> +++ b/net/rds/connection.c
[ ... ]
> @@ -567,16 +581,55 @@ static void rds_conn_path_destroy(struct rds_conn_path *cp)
>  	cp->cp_conn->c_trans->conn_free(cp->cp_transport_data);
>  }
>  
> +/* Free a connection.  This runs from rds_conn_put() when the last
> + * reference is dropped, after rds_conn_destroy() has quiesced the
> + * connection and dropped the initial reference.
> + */
> +static void rds_conn_destroy_fini(struct kref *kref)
> +{
> +	struct rds_connection *conn = container_of(kref, struct rds_connection,
> +						   c_refcount);
> +	int npaths = (conn->c_trans->t_mp_capable ? RDS_MPATH_WORKERS : 1);
> +	unsigned long flags;
> +	int i;
> +
> +	for (i = 0; i < npaths; i++)
> +		rds_conn_path_free(&conn->c_path[i]);
> +
> +	kfree(conn->c_path);

[Severity: High]
Does this code leak the connection and its associated memory if a probe message
is queued concurrently during teardown?

If a concurrent sendmsg() is preempted after checking rds_destroy_pending(),
and rds_conn_destroy() completes rds_conn_path_quiesce() which purges
cp_send_queue, the resumed sendmsg() can call rds_send_ping() ->
rds_send_probe().

In rds_send_probe(), the message is added to cp_send_queue and a connection
reference is taken:

net/rds/send.c:rds_send_probe() {
    ...
    spin_lock_irqsave(&cp->cp_lock, flags);
    list_add_tail(&rm->m_conn_item, &cp->cp_send_queue);
    ...
    rds_conn_get(cp->cp_conn);
    ...
    if (!rds_destroy_pending(cp->cp_conn))
        queue_delayed_work(cp->cp_wq, &cp->cp_send_w, 1);
    ...
}

Because rds_destroy_pending() is now true, the worker isn't queued. Since the
probe message lacks an associated socket (rm->m_rs == NULL), closing the
socket won't purge the message via rds_send_drop_to().

Because rds_conn_destroy_fini() does not traverse or drain cp_send_queue, it
seems the connection reference taken by rds_send_probe() will never be dropped.
This could lead to a permanent leak of the connection and stall module unloads
in rds_conn_wait_conns_freed().

> +	kmem_cache_free(rds_conn_slab, conn);
> +
> +	spin_lock_irqsave(&rds_conn_lock, flags);
> +	rds_conn_count--;
> +	spin_unlock_irqrestore(&rds_conn_lock, flags);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927061448.167862-1-achender@kernel.org?part=5