Re: [PATCH net-next v3 10/13] net/rds: tcp: don't attach an accepted socket to a connection being destroyed
From: sashiko-bot@kernel.org
Date: 2026-09-15 03:37:39
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] The `rds_destroy_pending(conn)` check introduced by the patch is lockless with respect to connection teardown, leaving a Time-Of-Check to Time-Of-Use (TOCTOU) race condition that renders the patch's attempt to fix the Use-After-Free ineffective. -- commit 128ba6b33f50fd05797877892e71aaf3ccd2f612 Author: Allison Henderson [off-list ref] net/rds: tcp: don't attach an accepted socket to a connection being destroyed This commit attempts to prevent a use-after-free by refusing to attach an accepted socket to a connection whose teardown has already begun, doing so by checking rds_destroy_pending().
quoted hunk ↗ jump to hunk
diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c index dcac10a91a67f..e22ea9ca8c1ce 100644 --- a/net/rds/tcp_listen.c +++ b/net/rds/tcp_listen.c@@ -278,7 +278,15 @@ int rds_tcp_accept_one(struct rds_tcp_net *rtn) cp = rs_tcp->t_cpath; conn_state = rds_conn_path_state(cp); WARN_ON(conn_state == RDS_CONN_UP); - if (conn_state != RDS_CONN_CONNECTING && conn_state != RDS_CONN_ERROR) { + /* A connection whose destroy has begun has been quiesced and is + * only waiting for its last reference: its paths sit in + * RDS_CONN_DOWN, which rds_tcp_accept_one_path() happily claims. + * Installing a socket on it would leave sk_user_data pointing + * at a path that is about to be freed. + */ + if (rds_destroy_pending(conn) || + (conn_state != RDS_CONN_CONNECTING && + conn_state != RDS_CONN_ERROR)) {
[Severity: Critical] Does evaluating rds_destroy_pending() without an RCU read-side critical section leave a Time-Of-Check to Time-Of-Use race condition? Looking at rds_conn_destroy() in net/rds/connection.c, the teardown sequence sets the c_destroy_in_prog flag and calls synchronize_rcu(). Because rds_tcp_accept_one() isn't holding rcu_read_lock() here, synchronize_rcu() will not wait for it. If rds_tcp_accept_one() gets preempted immediately after this check, could rds_conn_destroy() run concurrently on another CPU, complete the teardown while the socket is still NULL, and free the connection? When rds_tcp_accept_one() resumes, it appears it would install the new socket onto the connection and drop its reference via rds_conn_put(). This would leave the socket's callbacks and sk_user_data pointing to freed memory, leading to the use-after-free this patch intends to prevent.
rds_conn_path_drop(cp, 0); goto rst_nsk; }
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260914033719.138057-1-achender@kernel.org?part=10