Re: [PATCH net] net/smc: fix TOCTOU race between smc_listen_out() and listener close
Breno Leitao <[email protected]> Tue, 4 Aug 2026 01:02:30 -0700
| Newsgroups | gmane.linux.network |
|---|---|
| Message-ID | <[email protected]> |
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 <[email protected]>