[PATCH net] net/smc: serialize sndbuf descriptor release with diagnostic dumps

[PATCH net] net/smc: serialize sndbuf descriptor release with diagnostic dumps

[Chengfeng Ye]

An SMC-D connection can remain in the socket hash while smc_conn_kill()
tears it down. For devices supporting DMB nocopy, smcd_buf_detach() frees
the send buffer descriptor without taking the hash lock held by the
diagnostic reader.

__smc_diag_dump() can load a non-NULL conn->sndbuf_desc, then a concurrent
smc_conn_kill() can clear the pointer and free the descriptor before the
dump reads its len field. The socket lock held by the teardown path does
not exclude the dump, and clearing the pointer before freeing it does
not protect a reader that has already loaded it.

KASAN reported:

  BUG: KASAN: slab-use-after-free in __smc_diag_dump.constprop.0+0x2477/0x2b10
  Call Trace:
   __smc_diag_dump.constprop.0+0x2477/0x2b10
   smc_diag_dump_proto+0x266/0x390
   smc_diag_dump+0x20/0x70
   netlink_dump+0x489/0x1140
  Allocated by task 70:
   smcd_buf_attach+0x11b/0x310
   smc_listen_work+0x2a62/0x4cf0
  Freed by task 98:
   kfree+0x131/0x3c0
   smcd_buf_detach+0x120/0x280
   smc_conn_kill+0x487/0x720
   __smc_lgr_terminate.part.0+0x231/0x430
   smc_smcd_terminate_all+0x2cf/0x610

Take the hash write lock when removing the descriptor from the
connection. This waits for dumps holding the old pointer and prevents
new dumps from seeing it. Free the descriptor after dropping the lock.

Fixes: ae2be35cbed2 ("net/smc: {at|de}tach sndbuf to peer DMB if supported")
Cc: stable@vger.kernel.org
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
 net/smc/smc_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 9974149659c2..f32fc1bd5bc8 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -1207,6 +1207,8 @@ static void smcr_buf_unuse(struct smc_buf_desc *buf_desc, bool is_rmb,
 
 static void smcd_buf_detach(struct smc_connection *conn)
 {
+	struct smc_sock *smc = container_of(conn, struct smc_sock, conn);
+	struct smc_hashinfo *h = smc->sk.sk_prot->h.smc_hash;
 	struct smcd_dev *smcd = conn->lgr->smcd;
 	u64 peer_token = conn->peer_token;
 	struct smc_buf_desc *buf_desc;
@@ -1216,8 +1218,10 @@ static void smcd_buf_detach(struct smc_connection *conn)
 
 	smc_ism_detach_dmb(smcd, peer_token);
 
+	write_lock_bh(&h->lock);
 	buf_desc = conn->sndbuf_desc;
 	conn->sndbuf_desc = NULL;
+	write_unlock_bh(&h->lock);
 	kfree(buf_desc);
 }
 
-- 
2.43.0

Re: [PATCH net] net/smc: serialize sndbuf descriptor release with diagnostic dumps

[Mahanta Jambigi]

On 27/09/26 1:02 pm, Chengfeng Ye wrote:
> An SMC-D connection can remain in the socket hash while smc_conn_kill()
> tears it down. For devices supporting DMB nocopy, smcd_buf_detach() frees
> the send buffer descriptor without taking the hash lock held by the
> diagnostic reader.
> 
> __smc_diag_dump() can load a non-NULL conn->sndbuf_desc, then a concurrent
> smc_conn_kill() can clear the pointer and free the descriptor before the
> dump reads its len field. The socket lock held by the teardown path does
> not exclude the dump, and clearing the pointer before freeing it does
> not protect a reader that has already loaded it.

Agreed. This is addressed in v6[1] by unhashing the socket at the top of
smc_conn_kill(), before smcd_buf_detach() runs, so no hashed socket can
have its sndbuf_desc freed under a concurrent diag reader.

[1]
https://lore.kernel.org/netdev/20260926065023.1629497-1-mjambigi@linux.ibm.com/

Re: [PATCH net] net/smc: serialize sndbuf descriptor release with diagnostic dumps

[Chengfeng Ye]

On Mon, Sep 28, 2026 at 1:57 PM Mahanta Jambigi <mjambigi@linux.ibm.com> wrote:
>
> Agreed. This is addressed in v6[1] by unhashing the socket at the top of
> smc_conn_kill(), before smcd_buf_detach() runs, so no hashed socket can
> have its sndbuf_desc freed under a concurrent diag reader.
>
> [1]
> https://lore.kernel.org/netdev/20260926065023.1629497-1-mjambigi@linux.ibm.com/
>

Hi Mahanta,

Thanks for pointing this out, I didn't notice there was a patch
working on the same problem. Please drop my patch in favour of yours.

Best Regards,
Chengfeng

Re: [PATCH net] net/smc: serialize sndbuf descriptor release with diagnostic dumps

[netdev-bot+sashiko]

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

Critical: 0 · High: 2 (1) · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix is incomplete: smc_buf_create() still frees
  conn->sndbuf_desc without serializing against __smc_diag_dump().
- [High] The SMC-R release path smc_conn_free() -> smc_buf_unuse() ->
  smcr_buf_unuse() also frees descriptors that the diag reader can still…

Pre-existing issues:
- [High] __smc_diag_dump() checks its guards without any lock:
  smc->conn.alert_token_local (smc_diag.c:107-108) and…

--

net/smc: serialize sndbuf descriptor release with diagnostic dumps

This patch takes the SMC hash write lock in smcd_buf_detach() while it
clears conn->sndbuf_desc. The goal is to stop __smc_diag_dump(), which
reads the descriptor under read_lock(&h->lock), from seeing a freed
descriptor. The descriptor is freed once the lock has been dropped.

> diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
> index 9974149659c2f..f32fc1bd5bc85 100644
> --- a/net/smc/smc_core.c
> +++ b/net/smc/smc_core.c
> @@ -1207,6 +1207,8 @@ static void smcr_buf_unuse(struct smc_buf_desc *buf_desc, bool is_rmb,
>  
>  static void smcd_buf_detach(struct smc_connection *conn)
>  {
> +	struct smc_sock *smc = container_of(conn, struct smc_sock, conn);
> +	struct smc_hashinfo *h = smc->sk.sk_prot->h.smc_hash;
>  	struct smcd_dev *smcd = conn->lgr->smcd;
>  	u64 peer_token = conn->peer_token;
>  	struct smc_buf_desc *buf_desc;
> @@ -1216,8 +1218,10 @@ static void smcd_buf_detach(struct smc_connection *conn)
>  
>  	smc_ism_detach_dmb(smcd, peer_token);
>  
> +	write_lock_bh(&h->lock);
>  	buf_desc = conn->sndbuf_desc;
>  	conn->sndbuf_desc = NULL;
> +	write_unlock_bh(&h->lock);
>  	kfree(buf_desc);
>  }

[Severity: High]
The subject says sndbuf descriptor release is now serialized with diag
dumps. Is smcd_buf_detach() the only place that frees conn->sndbuf_desc
while the socket is visible to __smc_diag_dump()?

smc_buf_create() has an error path for when the RMB fails after the
sndbuf was already created:

net/smc/smc_core.c:smc_buf_create() {
    ...
	rc = __smc_buf_create(smc, is_smcd, true);
	if (rc && smc->conn.sndbuf_desc) {
		down_write(&smc->conn.lgr->sndbufs_lock);
		smc_lgr_buf_list_del(smc->conn.lgr, false,
				     smc->conn.sndbuf_desc);
		up_write(&smc->conn.lgr->sndbufs_lock);
		smc_buf_free(smc->conn.lgr, false, smc->conn.sndbuf_desc);
		smc->conn.sndbuf_desc = NULL;
	}
    ...
}

smc_buf_free() goes to smcr_buf_free() or smcd_buf_free() and kfree()s
the descriptor. The pointer is only cleared after that, and h->lock is
never taken.

At this point smc_sk_init() has already hashed the socket via
sk->sk_prot->hash(sk). smc_conn_create()->smc_lgr_register_conn() has
also set conn->alert_token_local. A dump that holds only
read_lock(&h->lock) can pass the alert_token_local check and then
evaluate this on the freed descriptor:

net/smc/smc_diag.c:__smc_diag_dump() {
    ...
			.sndbuf_size = conn->sndbuf_desc ?
				conn->sndbuf_desc->len : 0,
    ...
}

Could this cause the same slab-use-after-free the commit message
describes? It would affect SMC-R, and SMC-D devices without DMB nocopy.
Four callers can reach it when RMB allocation fails (-ENOMEM, or DMB
exhaustion on SMC-D): smc_connect_rdma(), smc_connect_ism(),
smc_listen_rdma_init() and smc_listen_ism_init(). The value is copied
to userspace as cinfo.sndbuf_size, and the SOCK_DIAG dump has no
capability check.

[Severity: High]
Is there a similar unserialized release on SMC-R through
smc_conn_free()->smc_buf_unuse()->smcr_buf_unuse()?

net/smc/smc_core.c:smcr_buf_unuse() {
    ...
	if (buf_desc->is_reg_err) {
		/* buf registration failed, reuse not possible */
		...
		smc_buf_free(lgr, is_rmb, buf_desc);
    ...
}

This kfree()s the descriptor, but conn->sndbuf_desc (for a vzalloc'ed
sndbuf) or conn->rmb_desc still points at it.

In smc_conn_free() this happens before smc_lgr_unregister_conn(), and
alert_token_local is only cleared inside the unregister:

	if (!list_empty(&lgr->list)) {
		smc_buf_unuse(conn, lgr); /* allow buffer reuse */
		smc_lgr_unregister_conn(conn);
	}

smcr_link_reg_buf() sets is_reg_err when smc_wr_reg_send() fails.
smc_connect_rdma() then jumps to connect_abort, which calls
smc_conn_abort()->smc_conn_free() on a socket that is hashed and still
has its token set. The listen side follows the same pattern.

Would a dump that runs between smc_buf_unuse() and
smc_lgr_unregister_conn() read sndbuf_desc->len or rmb_desc->len from
freed memory? Should this release site also be serialized with h->lock?

[Severity: High]
This is a pre-existing issue, not one introduced by this patch. The new
h->lock section ends before the rest of the teardown in smc_conn_kill():

net/smc/smc_core.c:smc_conn_kill() {
    ...
	smc_lgr_unregister_conn(conn);
	smc_close_active_abort(smc);
}

__smc_diag_dump() checks smc->conn.alert_token_local, smc_conn_lgr_valid()
and list_empty(&lgr->list), and nothing on the teardown side excludes
those checks. After them it dereferences conn->sndbuf_desc->len,
conn->rmb_desc->len, conn->rmb_desc->token and
conn->lgr->smcd->dibs->gid.

The socket can still be hashed in PEERCLOSEWAIT1/2, PEERFINCLOSEWAIT,
PROCESSABORT or APPFINCLOSEWAIT, for example after shutdown(). In that
case smc_close_active_abort() calls smc_conn_free()->smc_lgr_put(), and
the link group can then be freed through __smc_lgr_free():

	smc_lgr_free_bufs(lgr);
	...
	kfree(lgr);

conn->sndbuf_desc, conn->rmb_desc and conn->lgr are not cleared on this
path.

Suppose a dump passes the checks before smc_lgr_unregister_conn() and is
then delayed before the dereferences. read_lock() is preemptible on
PREEMPT_RT, and IRQ or vCPU preemption can also cause this. Can the dump
then read freed descriptors, or load smcd from a freed lgr and
dereference smcd->dibs?

This also affects the commit's own smc_smcd_terminate_all() scenario.
The sndbuf_desc->len read is still exposed on SMC-D devices without
nocopy, and the rmb_desc->len read on the next line is still exposed on
nocopy devices.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927073203.3694279-1-nicoyip.dev%40gmail.com