[PATCH net] net/smc: fix TOCTOU race between smc_listen_out() and listener close

[PATCH net] net/smc: fix TOCTOU race between smc_listen_out() and listener close

[Sidraya Jayagond]

smc_listen_out() reads lsmc->sk.sk_state without the listener lock,
then acquires lock_sock_nested() only after the check passes. This
opens a window where smc_close_active() can transition the listener
to SMC_CLOSED, call smc_close_cleanup_listen() to drain the accept
queue, and release the lock, all between the lockless read and the
delayed lock acquisition:

  smc_listen_work (smc_hs_wq)          smc_close_active()
  -------------------------------      -------------------------
  release_sock(child)
  if (sk_state == SMC_LISTEN) TRUE
                                        lock_sock(listener)
                                        sk_state = SMC_CLOSED
                                        smc_close_cleanup_listen()
                                        release_sock(listener)
                                        flush_work(tcp_listen_work)
  lock_sock_nested(listener)
  smc_accept_enqueue(listener, child) /* child enqueued on dead listener */

smc_close_active() flushes only tcp_listen_work. Work items already
dispatched onto smc_hs_wq for the CLC handshake continue running
unguarded. smc_accept_enqueue() takes a sock_hold() on the child that
is never released, so the child smc_sock, its clcsock, and the
reference all leak. A remote peer that opens TCP connections while the
server calls close() can exhaust kernel memory.

Move lock_sock_nested() to before the sk_state check so that the test
and the enqueue are atomic under the listener lock.

Fixes: fd57770dd198 ("net/smc: wait for pending work before clcsock release_sock")
Reviewed-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
Signed-off-by: Sidraya Jayagond <sidraya@linux.ibm.com>
---
 net/smc/af_smc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index b5db69073e20..00403175b740 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1931,11 +1931,12 @@ static void smc_listen_out(struct smc_sock *new_smc)
 		atomic_dec(&lsmc->queued_smc_hs);
 
 	release_sock(newsmcsk); /* lock in smc_listen_work() */
+	lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
 	if (lsmc->sk.sk_state == SMC_LISTEN) {
-		lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
 		smc_accept_enqueue(&lsmc->sk, newsmcsk);
 		release_sock(&lsmc->sk);
 	} else { /* no longer listening */
+		release_sock(&lsmc->sk);
 		smc_close_non_accepted(newsmcsk);
 	}
 
-- 
2.53.0

Re: [PATCH net] net/smc: fix TOCTOU race between smc_listen_out() and listener close

[Breno Leitao]

> @@ -1931,11 +1931,12 @@ static void smc_listen_out(struct smc_sock *new_smc)
>  		atomic_dec(&lsmc->queued_smc_hs);
>  
>  	release_sock(newsmcsk); /* lock in smc_listen_work() */
> +	lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
>  	if (lsmc->sk.sk_state == SMC_LISTEN) {
> -		lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
>  		smc_accept_enqueue(&lsmc->sk, newsmcsk);
>  		release_sock(&lsmc->sk);
>  	} else { /* no longer listening */
> +		release_sock(&lsmc->sk);
>  		smc_close_non_accepted(newsmcsk);
>  	}

Do you need to call smc_close_non_accepted() without the lock? otherwise
you can have the lock around the whole if/else clause.

Re: [PATCH net] net/smc: fix TOCTOU race between smc_listen_out() and listener close

[Sidraya Jayagond]

On 03/08/26 6:24 pm, Breno Leitao wrote:
>> @@ -1931,11 +1931,12 @@ static void smc_listen_out(struct smc_sock *new_smc)
>>  		atomic_dec(&lsmc->queued_smc_hs);
>>  
>>  	release_sock(newsmcsk); /* lock in smc_listen_work() */
>> +	lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
>>  	if (lsmc->sk.sk_state == SMC_LISTEN) {
>> -		lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
>>  		smc_accept_enqueue(&lsmc->sk, newsmcsk);
>>  		release_sock(&lsmc->sk);
>>  	} else { /* no longer listening */
>> +		release_sock(&lsmc->sk);
>>  		smc_close_non_accepted(newsmcsk);
>>  	}
> 
> Do you need to call smc_close_non_accepted() without the lock? otherwise
> you can have the lock around the whole if/else clause.

Yes, smc_close_non_accepted() calls __smc_release() in turn calls
smc_close_active(), which in the SMC_ACTIVE case hits
smc_close_stream_wait() a blocking wait that can sleep up to
SMC_MAX_STREAM_WAIT_TIMEOUT (2 minutes). Holding the listener lock
across that would stall any concurrent smc_accept(), smc_listen_out(),
or smc_release() on the listener for the same duration.

The release_sock() is intentionally placed before
smc_close_non_accepted() to keep the critical section minimal:
lock
  check state
    enqueue or not
      unlock
        then do the slow close work without the listener lock held.

Re: [PATCH net] net/smc: fix TOCTOU race between smc_listen_out() and listener close

[Breno Leitao]

On Tue, Aug 04, 2026 at 12:52:12PM +0530, Sidraya Jayagond wrote:
> 
> 
> On 03/08/26 6:24 pm, Breno Leitao wrote:
> >> @@ -1931,11 +1931,12 @@ static void smc_listen_out(struct smc_sock *new_smc)
> >>  		atomic_dec(&lsmc->queued_smc_hs);
> >>  
> >>  	release_sock(newsmcsk); /* lock in smc_listen_work() */
> >> +	lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
> >>  	if (lsmc->sk.sk_state == SMC_LISTEN) {
> >> -		lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
> >>  		smc_accept_enqueue(&lsmc->sk, newsmcsk);
> >>  		release_sock(&lsmc->sk);
> >>  	} else { /* no longer listening */
> >> +		release_sock(&lsmc->sk);
> >>  		smc_close_non_accepted(newsmcsk);
> >>  	}
> > 
> > Do you need to call smc_close_non_accepted() without the lock? otherwise
> > you can have the lock around the whole if/else clause.
> 
> Yes, smc_close_non_accepted() calls __smc_release() in turn calls
> smc_close_active(), which in the SMC_ACTIVE case hits
> smc_close_stream_wait() a blocking wait that can sleep up to
> SMC_MAX_STREAM_WAIT_TIMEOUT (2 minutes). Holding the listener lock
> across that would stall any concurrent smc_accept(), smc_listen_out(),
> or smc_release() on the listener for the same duration.
> 
> The release_sock() is intentionally placed before
> smc_close_non_accepted() to keep the critical section minimal:
> lock
>   check state
>     enqueue or not
>       unlock
>         then do the slow close work without the listener lock held.

That makes sense.

Reviewed-by: Breno Leitao <leitao@debian.org>

Re: [PATCH net] net/smc: fix TOCTOU race between smc_listen_out() and listener close

[Dust Li]

On 2026-08-04 12:52:12, Sidraya Jayagond wrote:
>
>
>On 03/08/26 6:24 pm, Breno Leitao wrote:
>>> @@ -1931,11 +1931,12 @@ static void smc_listen_out(struct smc_sock *new_smc)
>>>  		atomic_dec(&lsmc->queued_smc_hs);
>>>  
>>>  	release_sock(newsmcsk); /* lock in smc_listen_work() */
>>> +	lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
>>>  	if (lsmc->sk.sk_state == SMC_LISTEN) {
>>> -		lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
>>>  		smc_accept_enqueue(&lsmc->sk, newsmcsk);
>>>  		release_sock(&lsmc->sk);
>>>  	} else { /* no longer listening */
>>> +		release_sock(&lsmc->sk);
>>>  		smc_close_non_accepted(newsmcsk);
>>>  	}
>> 
>> Do you need to call smc_close_non_accepted() without the lock? otherwise
>> you can have the lock around the whole if/else clause.
>
>Yes, smc_close_non_accepted() calls __smc_release() in turn calls
>smc_close_active(), which in the SMC_ACTIVE case hits
>smc_close_stream_wait() a blocking wait that can sleep up to
>SMC_MAX_STREAM_WAIT_TIMEOUT (2 minutes). Holding the listener lock
>across that would stall any concurrent smc_accept(), smc_listen_out(),
>or smc_release() on the listener for the same duration.
>
>The release_sock() is intentionally placed before
>smc_close_non_accepted() to keep the critical section minimal:
>lock
>  check state
>    enqueue or not
>      unlock
>        then do the slow close work without the listener lock held.


That patch looks good to me.

Reviewed-by: Dust Li <dust.li@linux.alibaba.com>


Two corrections to the justification though:

1. This newsmcsk sock have never been given to userspace, so
   smc_close_stream_wait() should return immediately. I think the stronger
   argument for keeping smc_close_non_accepted() outside the listener
   lock is what follows in smc_close_active()/__smc_release():
   smc_close_final() can wait up to SMC_WR_TX_WAIT_FREE_SLOT_TIME (10s)
   for a WR slot, and smc_clcsock_release() tears down the TCP
   socket.

2. SMC_MAX_STREAM_WAIT_TIMEOUT is 2 * HZ, i.e. 2 seconds, not 2 minutes.


Best regards,
Dust

Re: [PATCH net] net/smc: fix TOCTOU race between smc_listen_out() and listener close

[Sidraya Jayagond]

On 05/08/26 12:51 pm, Dust Li wrote:
> On 2026-08-04 12:52:12, Sidraya Jayagond wrote:
>>
>>
>> On 03/08/26 6:24 pm, Breno Leitao wrote:
>>>> @@ -1931,11 +1931,12 @@ static void smc_listen_out(struct smc_sock *new_smc)
>>>>  		atomic_dec(&lsmc->queued_smc_hs);
>>>>  
>>>>  	release_sock(newsmcsk); /* lock in smc_listen_work() */
>>>> +	lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
>>>>  	if (lsmc->sk.sk_state == SMC_LISTEN) {
>>>> -		lock_sock_nested(&lsmc->sk, SINGLE_DEPTH_NESTING);
>>>>  		smc_accept_enqueue(&lsmc->sk, newsmcsk);
>>>>  		release_sock(&lsmc->sk);
>>>>  	} else { /* no longer listening */
>>>> +		release_sock(&lsmc->sk);
>>>>  		smc_close_non_accepted(newsmcsk);
>>>>  	}
>>>
>>> Do you need to call smc_close_non_accepted() without the lock? otherwise
>>> you can have the lock around the whole if/else clause.
>>
>> Yes, smc_close_non_accepted() calls __smc_release() in turn calls
>> smc_close_active(), which in the SMC_ACTIVE case hits
>> smc_close_stream_wait() a blocking wait that can sleep up to
>> SMC_MAX_STREAM_WAIT_TIMEOUT (2 minutes). Holding the listener lock
>> across that would stall any concurrent smc_accept(), smc_listen_out(),
>> or smc_release() on the listener for the same duration.
>>
>> The release_sock() is intentionally placed before
>> smc_close_non_accepted() to keep the critical section minimal:
>> lock
>>  check state
>>    enqueue or not
>>      unlock
>>        then do the slow close work without the listener lock held.
> 
> 
> That patch looks good to me.
> 
> Reviewed-by: Dust Li <dust.li@linux.alibaba.com>
> 
> 
> Two corrections to the justification though:
> 
> 1. This newsmcsk sock have never been given to userspace, so
>    smc_close_stream_wait() should return immediately. I think the stronger
>    argument for keeping smc_close_non_accepted() outside the listener
>    lock is what follows in smc_close_active()/__smc_release():
>    smc_close_final() can wait up to SMC_WR_TX_WAIT_FREE_SLOT_TIME (10s)
>    for a WR slot, and smc_clcsock_release() tears down the TCP
>    socket.
> 
> 2. SMC_MAX_STREAM_WAIT_TIMEOUT is 2 * HZ, i.e. 2 seconds, not 2 minutes.
> 

Thank you for the corrections,Dust. Both points are valid.
> 
> Best regards,
> Dust
> 

Re: [PATCH net] net/smc: fix TOCTOU race between smc_listen_out() and listener close

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon,  3 Aug 2026 09:07:01 +0200 you wrote:
> smc_listen_out() reads lsmc->sk.sk_state without the listener lock,
> then acquires lock_sock_nested() only after the check passes. This
> opens a window where smc_close_active() can transition the listener
> to SMC_CLOSED, call smc_close_cleanup_listen() to drain the accept
> queue, and release the lock, all between the lockless read and the
> delayed lock acquisition:
> 
> [...]

Here is the summary with links:
  - [net] net/smc: fix TOCTOU race between smc_listen_out() and listener close
    https://git.kernel.org/netdev/net/c/185a4caeecab

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html