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

[sashiko-bot@kernel.org]

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