[PATCH net v2] net/smc: fix abort_work termination in smc_conn_free()

[PATCH net v2] net/smc: fix abort_work termination in smc_conn_free()

[Hidayath Khan]

smc_conn_free() disposes of a pending conn->abort_work, but it gets three
things wrong:

1. Deadlock: smc_conn_free() runs with the socket lock held and calls
   cancel_work_sync(), while smc_conn_abort_work() takes the same lock.
   If the work has already started on another CPU and is waiting for that
   lock, the cancel waits for the work and the work waits for the caller.
   The current_work() test only stops the work from cancelling itself,
   not when the two run on different CPUs.

2. Reference leak: Schedulers of abort_work take a socket reference, and
   smc_conn_abort_work() drops it when it runs. If cancel_work_sync()
   removes a pending work item before it runs, that reference is never
   returned and the socket is never freed.

Both are fixed the way smc_close_cancel_work() handles close_work: drop
the socket lock around the cancel, and release the reference when the
cancel reports that it removed a pending item.

3. Late-queued work race: smc_cdc_rx_handler() finds the connection and
   drops lgr->conns_lock before smc_cdc_msg_validate() decides to queue:

     CPU0 (smc_conn_free)              CPU1 (smc_cdc_rx_handler)
                                       conn = smc_lgr_find_conn()
                                       sock_hold()
                                       read_unlock_bh(&lgr->conns_lock)
     cancel_work_sync()                  /* nothing queued yet */
     smc_buf_unuse()
                                       smc_cdc_msg_validate()
                                         queue_work(&conn->abort_work)

   cancel_work_sync() only guarantees that the work is not pending or
   running when it returns; a racing enqueue lands after that. The work
   then calls smc_conn_kill() on a connection whose buffers have already
   been returned.

Nothing smc_conn_free() does can prevent that enqueue, because the
receiver already holds the connection pointer. Make the late work
harmless instead: smc_conn_free() sets conn->freed with the socket lock
held before it releases anything, and smc_conn_abort_work() takes the same
lock. Check conn->freed inside smc_conn_abort_work() to skip
smc_conn_kill() if teardown has started. The work still drops its socket
reference.

Fixes: b286a0651e44 ("net/smc: handle incoming CDC validation message")
Cc: stable@vger.kernel.org
Reviewed-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>
---
v2:
- Extended the fix to cover the deadlock and racing enqueue issues flagged
  during v1 review.
- Moved the cancel into a helper smc_conn_cancel_abort_work() that drops
  the socket lock around cancel_work_sync().
- Added a check for conn->freed under lock_sock in smc_conn_abort_work() to
  safely handle late-queued work items without fragile reordering.
- Updated patch subject to reflect the broader termination fix.
  Link: https://lore.kernel.org/netdev/20260806081549.595001-1-hidayath@linux.ibm.com/

 net/smc/smc_core.c | 29 ++++++++++++++++++++++++++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 04aedd957543..9a109eae73b9 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -1251,6 +1251,25 @@ static void smc_buf_unuse(struct smc_connection *conn,
 	}
 }
 
+/* Cancel a pending abort work item.  smc_conn_abort_work() takes the socket
+ * lock, so the lock has to be dropped here.  Otherwise cancel_work_sync()
+ * waits for a worker that is itself blocked on the caller.  This is the idiom
+ * smc_close_cancel_work() already uses for close_work.
+ */
+static void smc_conn_cancel_abort_work(struct smc_connection *conn)
+{
+	struct smc_sock *smc = container_of(conn, struct smc_sock, conn);
+	struct sock *sk = &smc->sk;
+
+	if (current_work() == &conn->abort_work)
+		return;
+
+	release_sock(sk);
+	if (cancel_work_sync(&conn->abort_work))
+		sock_put(sk); /* sock_hold done by schedulers of abort_work */
+	lock_sock(sk);
+}
+
 /* remove a finished connection from its link group */
 void smc_conn_free(struct smc_connection *conn)
 {
@@ -1276,8 +1295,7 @@ void smc_conn_free(struct smc_connection *conn)
 			smcd_buf_detach(conn);
 	} else {
 		smc_cdc_wait_pend_tx_wr(conn);
-		if (current_work() != &conn->abort_work)
-			cancel_work_sync(&conn->abort_work);
+		smc_conn_cancel_abort_work(conn);
 	}
 	if (!list_empty(&lgr->list)) {
 		smc_buf_unuse(conn, lgr); /* allow buffer reuse */
@@ -1750,7 +1768,12 @@ static void smc_conn_abort_work(struct work_struct *work)
 	struct smc_sock *smc = container_of(conn, struct smc_sock, conn);
 
 	lock_sock(&smc->sk);
-	smc_conn_kill(conn, true);
+	/* smc_conn_free() sets freed with this lock held and before it
+	 * releases anything, so a work item queued after the cancel has
+	 * nothing left to do.
+	 */
+	if (!conn->freed)
+		smc_conn_kill(conn, true);
 	release_sock(&smc->sk);
 	sock_put(&smc->sk); /* sock_hold done by schedulers of abort_work */
 }

base-commit: e9abfc6803fcd57ecca1a647638df773b6429eb9
-- 
2.52.0

Re: [PATCH net v2] net/smc: fix abort_work termination in smc_conn_free()

[Dust Li]

On 2026-09-07 13:47:21, Hidayath Khan wrote:
>smc_conn_free() disposes of a pending conn->abort_work, but it gets three
>things wrong:
>
>1. Deadlock: smc_conn_free() runs with the socket lock held and calls
>   cancel_work_sync(), while smc_conn_abort_work() takes the same lock.
>   If the work has already started on another CPU and is waiting for that
>   lock, the cancel waits for the work and the work waits for the caller.
>   The current_work() test only stops the work from cancelling itself,
>   not when the two run on different CPUs.
>
>2. Reference leak: Schedulers of abort_work take a socket reference, and
>   smc_conn_abort_work() drops it when it runs. If cancel_work_sync()
>   removes a pending work item before it runs, that reference is never
>   returned and the socket is never freed.
>
>Both are fixed the way smc_close_cancel_work() handles close_work: drop
>the socket lock around the cancel, and release the reference when the
>cancel reports that it removed a pending item.
>
>3. Late-queued work race: smc_cdc_rx_handler() finds the connection and
>   drops lgr->conns_lock before smc_cdc_msg_validate() decides to queue:
>
>     CPU0 (smc_conn_free)              CPU1 (smc_cdc_rx_handler)
>                                       conn = smc_lgr_find_conn()
>                                       sock_hold()
>                                       read_unlock_bh(&lgr->conns_lock)
>     cancel_work_sync()                  /* nothing queued yet */
>     smc_buf_unuse()
>                                       smc_cdc_msg_validate()
>                                         queue_work(&conn->abort_work)
>
>   cancel_work_sync() only guarantees that the work is not pending or
>   running when it returns; a racing enqueue lands after that. The work
>   then calls smc_conn_kill() on a connection whose buffers have already
>   been returned.
>
>Nothing smc_conn_free() does can prevent that enqueue, because the
>receiver already holds the connection pointer. Make the late work
>harmless instead: smc_conn_free() sets conn->freed with the socket lock
>held before it releases anything, and smc_conn_abort_work() takes the same
>lock. Check conn->freed inside smc_conn_abort_work() to skip
>smc_conn_kill() if teardown has started. The work still drops its socket
>reference.
>
>Fixes: b286a0651e44 ("net/smc: handle incoming CDC validation message")
>Cc: stable@vger.kernel.org
>Reviewed-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
>Signed-off-by: Hidayath Khan <hidayath@linux.ibm.com>

Hi Hidayath,

Thanks for the fix. I think this is the right fix, and we can go ahead
and fix it this way for now.

Mahanta's smc_diag dump fix may need some rework on top of this one.

I believe the refactor I discussed with Mahanta in another thread would
address the root cause of these bugs more cleanly:
https://lore.kernel.org/netdev/apriDec7yHBkQgNv@linux.alibaba.com/

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

Best regards,
Dust

Re: [PATCH net v2] net/smc: fix abort_work termination in smc_conn_free()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 1 · Low: 0

- [High] New lock-drop window in smc_conn_free() invalidates the
  smc_cdc_wait_pend_tx_wr() barrier before the connection buffers/link…
- [Medium] smc_conn_free() silently changes its locking contract without
  documenting or asserting it, and its callers were not audited for the…

--

net/smc: fix abort_work termination in smc_conn_free()

This moves the abort_work cancel in smc_conn_free() into a new helper
smc_conn_cancel_abort_work() which drops the socket lock around
cancel_work_sync() and drops the socket reference when a pending item was
removed.  It also makes smc_conn_abort_work() skip smc_conn_kill() when
conn->freed is already set.

Link: https://lore.kernel.org/netdev/20260806081549.595001-1-hidayath@linux.ibm.com/

A couple of questions about the new lock drop below.

> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index 04aedd957543a..9a109eae73b92 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -1251,6 +1251,25 @@ static void smc_buf_unuse(struct smc_connection *conn,
>  	}
>  }
>  
> +/* Cancel a pending abort work item.  smc_conn_abort_work() takes the socket
> + * lock, so the lock has to be dropped here.  Otherwise cancel_work_sync()
> + * waits for a worker that is itself blocked on the caller.  This is the idiom
> + * smc_close_cancel_work() already uses for close_work.
> + */
> +static void smc_conn_cancel_abort_work(struct smc_connection *conn)
> +{
> +	struct smc_sock *smc = container_of(conn, struct smc_sock, conn);
> +	struct sock *sk = &smc->sk;
> +
> +	if (current_work() == &conn->abort_work)
> +		return;
> +

[Severity: Medium]
Should the new "caller must hold the socket lock" requirement be documented
or asserted?  smc_conn_free() is declared cross-file in net/smc/smc_core.h
as

    void smc_conn_free(struct smc_connection *conn);

and its comment still reads only "remove a finished connection from its
link group", yet for lgr->is_smcd == false it now unlocks and relocks the
socket.  There is no sock_owned_by_me() or lockdep_assert_held() to catch a
caller that does not hold it.

All current call sites do hold the lock (__smc_release(),
smc_close_active_abort(), smc_close_passive_work(), smc_conn_abort()), so
nothing is unbalanced today.  A backport or a new error path that reaches
smc_conn_free() without the lock would call release_sock() on a lock it does
not own and then return with the lock held.

There is also a caller-visible half.  The tree's idiom around a
lock-dropping cancel revalidates state afterwards, as in
smc_close_active_abort():

    sk->sk_state = SMC_PEERABORTWAIT;
    smc_close_cancel_work(smc);
    if (sk->sk_state != SMC_PEERABORTWAIT)
        break;
    sk->sk_state = SMC_CLOSED;
    smc_conn_free(&smc->conn);
    release_clcsock = true;
    sock_put(sk); /* passive closing */

The smc_conn_free() calls in smc_close_active_abort(),
smc_close_passive_work() and __smc_release() revalidate nothing and act on
state that was observed before the new window (release_clcsock = true, the
"passive closing" sock_put()).

Does this also invalidate the invariant declared in smc_listen_work()?

    lock_sock(&new_smc->sk); /* release in smc_listen_out() */

On an SMC-R handshake abort, smc_listen_work() -> smc_conn_abort() ->
smc_conn_free() now drops that lock in the middle, so another thread (for
example __smc_lgr_terminate(), which does lock_sock() before
smc_conn_kill()) can run against a half-set-up socket.

The changelog describes the change as a local fix to the cancel; would it be
worth stating that the calling convention of smc_conn_free() changed for
every call site?

> +	release_sock(sk);
> +	if (cancel_work_sync(&conn->abort_work))
> +		sock_put(sk); /* sock_hold done by schedulers of abort_work */
> +	lock_sock(sk);
> +}
> +

[Severity: High]
Does dropping the socket lock here void the pending-TX drain that
smc_conn_free() performs immediately before this call?

The SMC-R branch of smc_conn_free() is now:

    smc_cdc_wait_pend_tx_wr(conn);      /* cdc_pend_tx_wr drains to 0 */
    smc_conn_cancel_abort_work(conn);   /* release_sock() ... lock_sock() */
    ...
    smc_buf_unuse(conn, lgr);           /* zeroes sndbuf, used = 0 */
    smc_lgr_unregister_conn(conn);

release_sock() on an SMC socket is not a passive unlock.  sk_prot->release_cb
is smc_release_cb(), and net/smc/af_smc.c has:

    if (smc->conn.tx_in_release_sock) {
        smc_tx_pending(&smc->conn);
        smc->conn.tx_in_release_sock = false;
    }

conn->tx_in_release_sock is set from BH in smc_cdc_tx_handler(), on the very
completion that lets smc_cdc_wait_pend_tx_wr() return, when the socket is
owned by the task:

    if (sock_owned_by_user(&smc->sk))
        conn->tx_in_release_sock = true;
    else
        smc_tx_pending(conn);

    if (unlikely(wq_has_sleeper(&conn->cdc_pend_tx_wq)))
        wake_up(&conn->cdc_pend_tx_wq);

So smc_tx_pending() -> smc_tx_sndbuf_nonempty() -> smcr_tx_sndbuf_nonempty()
can post RDMA writes out of conn->sndbuf_desc and a CDC send, re-incrementing
conn->cdc_pend_tx_wr, inside the window this patch opens.
smc_tx_sndbuf_nonempty() only gates on sk_err, an empty send queue, peer RMBE
space and:

    if (conn->killed ||
        conn->local_rx_ctrl.conn_state_flags.peer_conn_abort) {
        rc = -EPIPE;    /* connection being aborted */
        goto out;
    }

conn->freed is never consulted on that path.

Is there a reachable state where sk_err and conn->killed are both clear while
staged data is still present?  Closing an SMC-R socket with SO_LINGER=0 or
from an exiting task returns from smc_close_stream_wait() immediately:

    if (!timeout)
        return;

    if (!smc_tx_prepared_sends(&smc->conn))
        return;

which leaves prepared sends outstanding and the state at SMC_PEERCLOSEWAIT1,
so __smc_release() does not free the connection.  When the peer closes,
smc_close_passive_work() reaches

    smc_conn_free(conn);

with sk_err == 0, conn->killed == 0 and data still staged.

After lock_sock() returns, smc_buf_unuse() does memzero_explicit() on the
send buffer and sets used = 0, returning the descriptor to the link-group
pool while the HCA may still be reading it, and smcr_link_put() /
smc_lgr_put() then run with cdc_pend_tx_wr non-zero, so a later
smc_cdc_tx_handler() completion dereferences pend->conn and the link WR
structures.

Note also that conn->tx_work is not cancelled here, while
smc_close_cancel_work() does cancel_delayed_work_sync(&conn->tx_work).  A
tx_work armed from BH via mod_delayed_work() in smcr_tx_sndbuf_nonempty()
can take the socket lock in the same window and post the same way.

Before this patch cancel_work_sync() ran with the socket lock held, so no
lock-taking tx path could interpose between the drain and the buffer release.

Would it work to cancel abort_work (and tx_work) before
smc_cdc_wait_pend_tx_wr(), or to repeat the drain after lock_sock() returns
and gate smc_tx_pending() on conn->freed?

>  /* remove a finished connection from its link group */
>  void smc_conn_free(struct smc_connection *conn)
>  {
> @@ -1276,8 +1295,7 @@ void smc_conn_free(struct smc_connection *conn)
>  			smcd_buf_detach(conn);
>  	} else {
>  		smc_cdc_wait_pend_tx_wr(conn);
> -		if (current_work() != &conn->abort_work)
> -			cancel_work_sync(&conn->abort_work);
> +		smc_conn_cancel_abort_work(conn);
>  	}
>  	if (!list_empty(&lgr->list)) {
>  		smc_buf_unuse(conn, lgr); /* allow buffer reuse */

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907114721.1303498-1-hidayath%40linux.ibm.com

Re: [PATCH net v2] net/smc: fix abort_work termination in smc_conn_free()

[Hidayath Khan]

On 09/09/26 5:19 pm, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
>
> Critical: 0 · High: 1 · Medium: 1 · Low: 0
>
> - [High] New lock-drop window in smc_conn_free() invalidates the
>    smc_cdc_wait_pend_tx_wr() barrier before the connection buffers/link…
> - [Medium] smc_conn_free() silently changes its locking contract without
>    documenting or asserting it, and its callers were not audited for the…
>
> --
Thanks for the review. Both the findings are correct, and together they rule
out the approach v2 took rather than asking for an adjustment to it, so v3
drops the lock-drop entirely.
>
> net/smc: fix abort_work termination in smc_conn_free()
>
> This moves the abort_work cancel in smc_conn_free() into a new helper
> smc_conn_cancel_abort_work() which drops the socket lock around
> cancel_work_sync() and drops the socket reference when a pending item was
> removed.  It also makes smc_conn_abort_work() skip smc_conn_kill() when
> conn->freed is already set.
>
> Link: https://lore.kernel.org/netdev/20260806081549.595001-1-hidayath@linux.ibm.com/
>
> A couple of questions about the new lock drop below.
>
>> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
>> index 04aedd957543a..9a109eae73b92 100644
>> --- a/net/smc/smc_core.c
>> +++ b/net/smc/smc_core.c
>> @@ -1251,6 +1251,25 @@ static void smc_buf_unuse(struct smc_connection *conn,
>>   	}
>>   }
>>   
>> +/* Cancel a pending abort work item.  smc_conn_abort_work() takes the socket
>> + * lock, so the lock has to be dropped here.  Otherwise cancel_work_sync()
>> + * waits for a worker that is itself blocked on the caller.  This is the idiom
>> + * smc_close_cancel_work() already uses for close_work.
>> + */
>> +static void smc_conn_cancel_abort_work(struct smc_connection *conn)
>> +{
>> +	struct smc_sock *smc = container_of(conn, struct smc_sock, conn);
>> +	struct sock *sk = &smc->sk;
>> +
>> +	if (current_work() == &conn->abort_work)
>> +		return;
>> +
> [Severity: Medium]
> Should the new "caller must hold the socket lock" requirement be documented
> or asserted?  smc_conn_free() is declared cross-file in net/smc/smc_core.h
> as
>
>      void smc_conn_free(struct smc_connection *conn);
>
> and its comment still reads only "remove a finished connection from its
> link group", yet for lgr->is_smcd == false it now unlocks and relocks the
> socket.  There is no sock_owned_by_me() or lockdep_assert_held() to catch a
> caller that does not hold it.
>
> All current call sites do hold the lock (__smc_release(),
> smc_close_active_abort(), smc_close_passive_work(), smc_conn_abort()), so
> nothing is unbalanced today.  A backport or a new error path that reaches
> smc_conn_free() without the lock would call release_sock() on a lock it does
> not own and then return with the lock held.
>
> There is also a caller-visible half.  The tree's idiom around a
> lock-dropping cancel revalidates state afterwards, as in
> smc_close_active_abort():
>
>      sk->sk_state = SMC_PEERABORTWAIT;
>      smc_close_cancel_work(smc);
>      if (sk->sk_state != SMC_PEERABORTWAIT)
>          break;
>      sk->sk_state = SMC_CLOSED;
>      smc_conn_free(&smc->conn);
>      release_clcsock = true;
>      sock_put(sk); /* passive closing */
>
> The smc_conn_free() calls in smc_close_active_abort(),
> smc_close_passive_work() and __smc_release() revalidate nothing and act on
> state that was observed before the new window (release_clcsock = true, the
> "passive closing" sock_put()).
>
> Does this also invalidate the invariant declared in smc_listen_work()?
>
>      lock_sock(&new_smc->sk); /* release in smc_listen_out() */
>
> On an SMC-R handshake abort, smc_listen_work() -> smc_conn_abort() ->
> smc_conn_free() now drops that lock in the middle, so another thread (for
> example __smc_lgr_terminate(), which does lock_sock() before
> smc_conn_kill()) can run against a half-set-up socket.
>
> The changelog describes the change as a local fix to the cancel; would it be
> worth stating that the calling convention of smc_conn_free() changed for
> every call site?
>
>> +	release_sock(sk);
>> +	if (cancel_work_sync(&conn->abort_work))
>> +		sock_put(sk); /* sock_hold done by schedulers of abort_work */
>> +	lock_sock(sk);
>> +}
>> +
> [Severity: High]
> Does dropping the socket lock here void the pending-TX drain that
> smc_conn_free() performs immediately before this call?

Yes. This is the finding that ended the approach, and the ordering makes it
the natural interleaving rather than a narrow race. In smc_cdc_tx_handler():

     if (atomic_dec_and_test(&conn->cdc_pend_tx_wr)) {
         if (sock_owned_by_user(&smc->sk))
             conn->tx_in_release_sock = true;
         else
             smc_tx_pending(conn);

         if (unlikely(wq_has_sleeper(&conn->cdc_pend_tx_wq)))
             wake_up(&conn->cdc_pend_tx_wq);
     }

conn->tx_in_release_sock is set before the wake_up, on the very completion
that lets smc_cdc_wait_pend_tx_wr() return, and the socket is owned by the
task doing the teardown. So the drain returns with the flag already set,
and the release_sock() immediately after it runs smc_release_cb() ->
smc_tx_pending() -> smc_tx_sndbuf_nonempty(), which posts RDMA writes out of
conn->sndbuf_desc and re-increments conn->cdc_pend_tx_wr. smc_buf_unuse()
then memzero_explicit()s that buffer and sets used = 0, returning the
descriptor to the link group pool - it never clears conn->sndbuf_desc - so
another connection can be handed a buffer the adapter is still reading, and
smcr_link_put()/smc_lgr_put() run with cdc_pend_tx_wr non-zero.
>
> The SMC-R branch of smc_conn_free() is now:
>
>      smc_cdc_wait_pend_tx_wr(conn);      /* cdc_pend_tx_wr drains to 0 */
>      smc_conn_cancel_abort_work(conn);   /* release_sock() ... lock_sock() */
>      ...
>      smc_buf_unuse(conn, lgr);           /* zeroes sndbuf, used = 0 */
>      smc_lgr_unregister_conn(conn);
>
> release_sock() on an SMC socket is not a passive unlock.  sk_prot->release_cb
> is smc_release_cb(), and net/smc/af_smc.c has:
>
>      if (smc->conn.tx_in_release_sock) {
>          smc_tx_pending(&smc->conn);
>          smc->conn.tx_in_release_sock = false;
>      }
>
> conn->tx_in_release_sock is set from BH in smc_cdc_tx_handler(), on the very
> completion that lets smc_cdc_wait_pend_tx_wr() return, when the socket is
> owned by the task:
>
>      if (sock_owned_by_user(&smc->sk))
>          conn->tx_in_release_sock = true;
>      else
>          smc_tx_pending(conn);
>
>      if (unlikely(wq_has_sleeper(&conn->cdc_pend_tx_wq)))
>          wake_up(&conn->cdc_pend_tx_wq);
>
> So smc_tx_pending() -> smc_tx_sndbuf_nonempty() -> smcr_tx_sndbuf_nonempty()
> can post RDMA writes out of conn->sndbuf_desc and a CDC send, re-incrementing
> conn->cdc_pend_tx_wr, inside the window this patch opens.
> smc_tx_sndbuf_nonempty() only gates on sk_err, an empty send queue, peer RMBE
> space and:
>
>      if (conn->killed ||
>          conn->local_rx_ctrl.conn_state_flags.peer_conn_abort) {
>          rc = -EPIPE;    /* connection being aborted */
>          goto out;
>      }
>
> conn->freed is never consulted on that path.
>
> Is there a reachable state where sk_err and conn->killed are both clear while
> staged data is still present?  Closing an SMC-R socket with SO_LINGER=0 or
> from an exiting task returns from smc_close_stream_wait() immediately:
>
>      if (!timeout)
>          return;
>
>      if (!smc_tx_prepared_sends(&smc->conn))
>          return;
>
> which leaves prepared sends outstanding and the state at SMC_PEERCLOSEWAIT1,
> so __smc_release() does not free the connection.  When the peer closes,
> smc_close_passive_work() reaches
>
>      smc_conn_free(conn);
>
> with sk_err == 0, conn->killed == 0 and data still staged.
>
> After lock_sock() returns, smc_buf_unuse() does memzero_explicit() on the
> send buffer and sets used = 0, returning the descriptor to the link-group
> pool while the HCA may still be reading it, and smcr_link_put() /
> smc_lgr_put() then run with cdc_pend_tx_wr non-zero, so a later
> smc_cdc_tx_handler() completion dereferences pend->conn and the link WR
> structures.
>
> Note also that conn->tx_work is not cancelled here, while
> smc_close_cancel_work() does cancel_delayed_work_sync(&conn->tx_work).  A
> tx_work armed from BH via mod_delayed_work() in smcr_tx_sndbuf_nonempty()
> can take the socket lock in the same window and post the same way.
>
> Before this patch cancel_work_sync() ran with the socket lock held, so no
> lock-taking tx path could interpose between the drain and the buffer release.
>
> Would it work to cancel abort_work (and tx_work) before
> smc_cdc_wait_pend_tx_wr(), or to repeat the drain after lock_sock() returns
> and gate smc_tx_pending() on conn->freed?
Cancelling before the drain does not help, because the problem is not the
order of the cancel and the drain - it is that release_sock() on an SMC
socket runs the tx path at all. Moving the cancel earlier moves the window,
it does not close it: the lock is still dropped between the drain and
smc_buf_unuse().

Repeating the drain after lock_sock() has the same shape as the first one.
The completion that ends the second drain can set tx_in_release_sock too.

v3 removes the window: the socket lock is never dropped. smc_conn_free()
sets conn->freed under the socket lock before it releases anything,
and smc_conn_abort_work() takes the same lock, so an instance that is
running is parked on that lock and will find the flag set, and one queued
afterwards finds the same - and then the cancel no longer has to wait for
anything, so cancel_work() replaces cancel_work_sync(). Returning the
reference when it reports it removed a pending item fixes the leak v1
was about.
>
>>   /* remove a finished connection from its link group */
>>   void smc_conn_free(struct smc_connection *conn)
>>   {
>> @@ -1276,8 +1295,7 @@ void smc_conn_free(struct smc_connection *conn)
>>   			smcd_buf_detach(conn);
>>   	} else {
>>   		smc_cdc_wait_pend_tx_wr(conn);
>> -		if (current_work() != &conn->abort_work)
>> -			cancel_work_sync(&conn->abort_work);
>> +		smc_conn_cancel_abort_work(conn);
>>   	}
>>   	if (!list_empty(&lgr->list)) {
>>   		smc_buf_unuse(conn, lgr); /* allow buffer reuse */
> [ ... ]
>