[PATCH v2] RDMA/siw: Fix early peer close handling on listeners

[PATCH v2] RDMA/siw: Fix early peer close handling on listeners

[Jaeyeong Lee]

A TCP child socket inherits the listener callbacks and sk_user_data before
kernel_accept() associates it with a new connection endpoint. If the peer
closes during this window, siw_cm_llp_state_change() queues a PEER_CLOSE
work item against the listening endpoint.

The worker releases that endpoint without removing it from the listener
list. A later listener teardown then walks the dangling entry, causing a
use-after-free in siw_destroy_listen().

Ignoring close notifications while the endpoint is LISTENING is not
sufficient. The endpoint remains IDLE for a short interval after listen()
starts accepting connections, and a close delivered before the accepted
socket is handed to its new endpoint would otherwise be lost.

Ignore close notifications from unaccepted child sockets during the IDLE
interval as well. During accept, serialize the endpoint handoff with TCP
state changes and reject a child that has already reached TCP_CLOSE or
TCP_CLOSE_WAIT. Live accepted sockets continue through the normal close
path.

The issue was found by static analysis. It was reproduced with SIW over
loopback in an x86_64 QEMU guest. KASAN reported the listener endpoint
use-after-free on the unpatched kernel. Tests covering RST before accept,
an MPA request followed by FIN, and an extended post-listen IDLE window
completed without KASAN or lockdep reports after this change.

Fixes: 6c52fdc244b5 ("rdma/siw: connection management")
Cc: stable@vger.kernel.org
Signed-off-by: Jaeyeong Lee <lee@jaeyeong.cc>
---
Changes in v2:
- Cover the IDLE interval between listen() and the LISTENING transition.
- Serialize accepted-socket handoff and reject an already closed child.

 drivers/infiniband/sw/siw/siw_cm.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
index ed49818793dde..92fcbcabb5984 100644
--- a/drivers/infiniband/sw/siw/siw_cm.c
+++ b/drivers/infiniband/sw/siw/siw_cm.c
@@ -973,6 +973,7 @@ static void siw_accept_newconn(struct siw_cep *cep)
 	struct socket *s = cep->sock;
 	struct socket *new_s = NULL;
 	struct siw_cep *new_cep = NULL;
+	bool peer_closed;
 	int rv = 0; /* debug only. should disappear */
 
 	if (cep->state != SIW_EPSTATE_LISTENING)
@@ -1009,11 +1010,23 @@ static void siw_accept_newconn(struct siw_cep *cep)
 	}
 	new_cep->sock = new_s;
 	siw_cep_get(new_cep);
-	new_s->sk->sk_user_data = new_cep;
 
 	if (siw_tcp_nagle == false)
 		tcp_sock_set_nodelay(new_s->sk);
+
+	lock_sock(new_s->sk);
+	write_lock_bh(&new_s->sk->sk_callback_lock);
 	new_cep->state = SIW_EPSTATE_AWAIT_MPAREQ;
+	new_s->sk->sk_user_data = new_cep;
+	peer_closed = new_s->sk->sk_state == TCP_CLOSE ||
+		      new_s->sk->sk_state == TCP_CLOSE_WAIT;
+	write_unlock_bh(&new_s->sk->sk_callback_lock);
+	release_sock(new_s->sk);
+
+	if (peer_closed) {
+		rv = -ECONNRESET;
+		goto error;
+	}
 
 	rv = siw_cm_queue_work(new_cep, SIW_CM_WORK_MPATIMEOUT);
 	if (rv)
@@ -1361,6 +1374,10 @@ static void siw_cm_llp_state_change(struct sock *sk)
 
 	case TCP_CLOSE:
 	case TCP_CLOSE_WAIT:
+		if (cep->state == SIW_EPSTATE_LISTENING ||
+		    (cep->state == SIW_EPSTATE_IDLE &&
+		     !READ_ONCE(sk->sk_socket)))
+			break;
 		if (cep->qp)
 			cep->qp->tx_ctx.tx_suspend = 1;
 		siw_cm_queue_work(cep, SIW_CM_WORK_PEER_CLOSE);
-- 
2.43.0

Re: [PATCH v2] RDMA/siw: Fix early peer close handling on listeners

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] RDMA/siw: siw_accept_newconn() drops connections in post-listen IDLE window
--

commit fd783e525f31231ba5b3c0e855c9ecc089f5379f
Author: Jaeyeong Lee <lee@jaeyeong.cc>

RDMA/siw: Fix early peer close handling on listeners

This commit prevents a use-after-free in siw_destroy_listen() when a TCP
child socket closes before kernel_accept() associates it with a new
connection endpoint. It ignores close notifications on listening and
unaccepted post-listen IDLE endpoints, and serializes the accepted-socket
handoff in siw_accept_newconn() while rejecting child sockets that have
already reached TCP_CLOSE or TCP_CLOSE_WAIT.

> diff --git a/drivers/infiniband/sw/siw/siw_cm.c b/drivers/infiniband/sw/siw/siw_cm.c
> index ed49818793dde..92fcbcabb5984 100644
> --- a/drivers/infiniband/sw/siw/siw_cm.c
> +++ b/drivers/infiniband/sw/siw/siw_cm.c
> @@ -973,6 +973,7 @@ static void siw_accept_newconn(struct siw_cep *cep)
>  	struct socket *s = cep->sock;
>  	struct socket *new_s = NULL;
>  	struct siw_cep *new_cep = NULL;
> +	bool peer_closed;
>  	int rv = 0; /* debug only. should disappear */
>  
>  	if (cep->state != SIW_EPSTATE_LISTENING)
> @@ -1009,11 +1010,23 @@ static void siw_accept_newconn(struct siw_cep *cep)
>  	}
>  	new_cep->sock = new_s;
>  	siw_cep_get(new_cep);
> -	new_s->sk->sk_user_data = new_cep;
>  
>  	if (siw_tcp_nagle == false)
>  		tcp_sock_set_nodelay(new_s->sk);
> +
> +	lock_sock(new_s->sk);
> +	write_lock_bh(&new_s->sk->sk_callback_lock);
>  	new_cep->state = SIW_EPSTATE_AWAIT_MPAREQ;
> +	new_s->sk->sk_user_data = new_cep;
> +	peer_closed = new_s->sk->sk_state == TCP_CLOSE ||
> +		      new_s->sk->sk_state == TCP_CLOSE_WAIT;
> +	write_unlock_bh(&new_s->sk->sk_callback_lock);
> +	release_sock(new_s->sk);
> +
> +	if (peer_closed) {
> +		rv = -ECONNRESET;
> +		goto error;
> +	}
>  
>  	rv = siw_cm_queue_work(new_cep, SIW_CM_WORK_MPATIMEOUT);
>  	if (rv)
> @@ -1361,6 +1374,10 @@ static void siw_cm_llp_state_change(struct sock *sk)
>  
>  	case TCP_CLOSE:
>  	case TCP_CLOSE_WAIT:
> +		if (cep->state == SIW_EPSTATE_LISTENING ||
> +		    (cep->state == SIW_EPSTATE_IDLE &&
> +		     !READ_ONCE(sk->sk_socket)))
> +			break;

[Severity: High]
When a connection arrives during the post-listen SIW_EPSTATE_IDLE window
in siw_create_listen(), can the SIW_CM_WORK_ACCEPT work item run before
cep->state transitions to SIW_EPSTATE_LISTENING and strand the child
socket in the TCP accept queue?

In siw_create_listen(), s->ops->listen() is called while cep->state is
still SIW_EPSTATE_IDLE and cep->in_use is not held:

drivers/infiniband/sw/siw/siw_cm.c:siw_create_listen() {
	...
	rv = s->ops->listen(s, backlog);
	if (rv) {
		siw_dbg(id->device, "listen error %d\n", rv);
		goto error;
	}
	cep->cm_id = id;
	id->add_ref(id);
	...
	if (!id->provider_data) {
		id->provider_data =
			kmalloc_obj(struct list_head);
		...
	}
	list_add_tail(&cep->listenq, (struct list_head *)id->provider_data);
	cep->state = SIW_EPSTATE_LISTENING;
}

If a peer completes a TCP handshake during this window,
siw_cm_llp_state_change() queues SIW_CM_WORK_ACCEPT with delay 0, and if
the peer also closes, the new check ignores TCP_CLOSE / TCP_CLOSE_WAIT
because cep->state is SIW_EPSTATE_IDLE and sk->sk_socket is NULL.

However, if siw_cm_work_handler() runs siw_accept_newconn(cep) before
siw_create_listen() sets cep->state = SIW_EPSTATE_LISTENING,
siw_accept_newconn() bails out without calling kernel_accept():

drivers/infiniband/sw/siw/siw_cm.c:siw_accept_newconn() {
	...
	if (cep->state != SIW_EPSTATE_LISTENING)
		goto error;
}

Because the SIW_CM_WORK_ACCEPT work item is consumed and never re-queued
when cep->state becomes SIW_EPSTATE_LISTENING, the unaccepted child socket
is left at the head of icsk_accept_queue. A live connection will hang
unaccepted, or if the peer closed it during the IDLE window, the dead
child socket will be dequeued by the next connection's SIW_CM_WORK_ACCEPT
and trigger the new peer_closed abort, stranding the subsequent valid
connection.

Would holding siw_cep_set_inuse(cep) or setting cep->state =
SIW_EPSTATE_LISTENING before calling s->ops->listen() in
siw_create_listen() avoid this post-listen IDLE window altogether?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104100953.125140-1-lee@jaeyeong.cc?part=1