Re: [PATCH net-next v3 13/13] net/rds: hold a connection reference from struct rds_incoming

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition in `rds_send_probe()` can leak `rds_message` and connection reference during connection teardown.
--

commit 9f78a5cbb0d8c28990a439e07491dbc084983be8
Author: Sharath Srinivasan <sharath.srinivasan@oracle.com>

net/rds: hold a connection reference from struct rds_incoming

This commit ensures that every holder of rds_incoming->i_conn takes a
reference to the connection. This prevents a use-after-free when the
connection is destroyed by device removal or module unload while messages
are still queued on the socket.

Closes: https://lore.kernel.org/netdev/20260720184955.3008978-1-nicoyip.dev@gmail.com/

> diff --git a/net/rds/send.c b/net/rds/send.c
> index 94d6ac174dde8..f59a487b079d5 100644
> --- a/net/rds/send.c
> +++ b/net/rds/send.c
[ ... ]
> @@ -1577,6 +1580,7 @@ rds_send_probe(struct rds_conn_path *cp, __be16 sport,
>  	list_add_tail(&rm->m_conn_item, &cp->cp_send_queue);
>  	set_bit(RDS_MSG_ON_CONN, &rm->m_flags);
>  	rds_message_addref(rm);
> +	rds_conn_get(cp->cp_conn);	/* put in rds_message_put() */

[Severity: High]
Does this code allow a message and connection reference to be permanently
leaked during connection teardown?

In rds_send_queue_rm(), rds_destroy_pending() is checked while holding
cp->cp_lock before queueing a message. However, here in rds_send_probe(), the
message is added to the send queue and the connection reference is taken
without first checking rds_destroy_pending() under the lock.

If a concurrent network namespace teardown or module unload initiates
rds_conn_destroy(), it calls rds_conn_path_quiesce() which purges the
cp_send_queue under cp_lock. This probe message can then be inserted into
the queue right after the purge.

After releasing the lock, rds_send_probe() checks rds_destroy_pending()
outside the lock to decide whether to queue the worker:

	rcu_read_lock();
	if (!rds_destroy_pending(cp->cp_conn))
		queue_delayed_work(cp->cp_wq, &cp->cp_send_w, 1);
	rcu_read_unlock();

If the teardown race described above occurs, rds_destroy_pending() will
evaluate to true here, and the worker will not be queued.

Since the message was already added to the purged cp_send_queue and the
worker is skipped, is the message left stranded indefinitely? This would
cause the connection reference to never be dropped, leading to an indefinite
hang during transport module unload, as rds_conn_wait_conns_freed() waits
forever for the connection count to reach zero.

>  	rm->m_inc.i_conn = cp->cp_conn;
>  	rm->m_inc.i_conn_path = cp;
>

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