* [PATCH net v13] net/smc: fix lgr/lnk lifetime vs diag reader race
@ 2026-10-23 15:03 Mahanta Jambigi
2026-10-23 15:06 ` netdev-bot+sinfo
2026-10-24 15:03 ` sashiko-bot
0 siblings, 2 replies; 4+ messages in thread
From: Mahanta Jambigi @ 2026-10-23 15:03 UTC (permalink / raw)
To: andrew+netdev, davem, edumazet, kuba, pabeni, alibuda, dust.li,
sidraya
Cc: hidayath, pasic, horms, tonylu, guwen, stable, netdev, linux-s390,
linux-rdma, Mahanta Jambigi
The diag dump walks the socket hash table under a read_lock and
dereferences conn->lgr and conn->lnk. Four paths expose a window where a
hashed socket has freed or partially freed conn/lgr/lnk state.
First, smc_conn_abort() calls smc_conn_free() during early handshake
aborts while the socket is still hashed in SMC_INIT, synchronously
freeing the lgr on the smc_lgr_cleanup_early() path without holding the
hash write_lock. A concurrent diag reader can load a non-NULL conn->lgr
and dereference lgr->is_smcd after the kfree. Skip the connection, link
group and DMB attributes in __smc_diag_dump() for sockets in SMC_INIT or
SMC_CLOSED, and for sockets that have fallen back to TCP, and do not
look at conn->lgr to determine diag_mode in SMC_INIT.
This changes the sock_diag output for such sockets. SMC_DIAG_CONNINFO,
SMC_DIAG_LGRINFO and SMC_DIAG_DMBINFO are not read while a
socket is in SMC_INIT or SMC_CLOSED or has fallen back to TCP, and
diag_mode is SMC_DIAG_MODE_SMCR for a non-fallback SMC-D socket in
SMC_INIT, as it already was before smc_conn_create() had run. smcss does
not use these attributes for such sockets. A fallback socket is a plain
TCP socket with no SMC connection state to report; the stale conn fields
left behind by smc_conn_abort() must not be read. SMC_DIAG_FALLBACK is
still reported. Use smp_load_acquire() / smp_store_release() on sk_state
to order these checks on weakly ordered CPUs.
Second, in smc_conn_kill() on SMC-D with dmb_nocopy, smcd_buf_detach()
NULLs and frees conn->sndbuf_desc while the socket is still hashed. A
concurrent diag reader entering the CONNINFO block can dereference the
freed descriptor. Unhash the socket right before smcd_buf_detach(). As a
result, killed ACTIVE and APPCLOSEWAIT sockets that are still open are
no longer visible to smcss on loopback-ism devices.
Third, in the terminal teardown branches of smc_close_active_abort()
(PEERCLOSEWAIT and PROCESSABORT groups) and in smc_close_passive_work(),
smc_conn_free() drops lgr and lnk references while the socket is still
hashed. Unhash immediately before each smc_conn_free() call at those two
sites. Sockets killed in the PEERCLOSEWAIT1/2, PEERFINCLOSEWAIT,
PROCESSABORT and APPFINCLOSEWAIT states are therefore unhashed, and
their inuse accounting dropped, before close(), even if the fd is still
open. Whether a killed socket remains visible to smcss thus depends on
its close state and, for SMC-D, on the device type.
Fourth, smc_listen_out_err() sets sk_state = SMC_CLOSED with a plain
store after smc_conn_abort() -> smc_conn_free() has already cleared
alert_token_local under write_lock_bh(). On weakly ordered CPUs a
concurrent diag reader using smp_load_acquire() on sk_state can observe
SMC_CLOSED while the preceding store to alert_token_local has not yet
become visible, causing it to dereference an lgr whose reference was
already dropped. Use smp_store_release() to order the alert_token_local
clear before the sk_state = SMC_CLOSED store.
Additionally, in __smc_diag_dump(), use the cached link->ibname instead
of link->smcibdev->ibdev->name to avoid a NULL pointer dereference or a
use-after-free of the IB device if __smcr_link_clear() zeroes the link
and the device is removed during concurrent SMC-R link failover. The
link data is still read without conns_lock or send_lock, so a dump
racing with a link failover may transiently report zeroed or mixed link
fields (ibname, ibport, link_id, GIDs). This does not affect memory
safety and the next dump reports the new link. Like the SMC netlink link
dump and the tracepoints, this reports the name at link creation time
and does not follow a later ib_device_rename().
Fixes: f16a7dd5cf27 ("smc: netlink interface for SMC sockets")
Fixes: 9dbe086c69b8 ("net/smc: fix invalid link access in dumping SMC-R connections")
Fixes: ae2be35cbed2 ("net/smc: {at|de}tach sndbuf to peer DMB if supported")
Cc: stable@vger.kernel.org
Signed-off-by: Mahanta Jambigi <mjambigi@linux.ibm.com>
---
Changes in v13:
- smc_diag.c: also skip the CONNINFO/LGRINFO/DMBINFO blocks for sockets
with use_fallback set; the SMC_DIAG_FALLBACK attribute is still
reported. After smc_lgr_terminate_sched() emptied lgr->list,
smc_conn_free() skips smc_lgr_unregister_conn(), so a socket that then
falls back and reaches SMC_ACTIVE can keep a stale alert_token_local
and a diag reader could dereference the freed sndbuf/rmb/lgr; a
fallback socket is a plain TCP socket with no SMC connection state to
report
- smc_core.c: drop the v11 and v12 changes (unconditional
smc_lgr_unregister_conn() in smc_conn_free() and the
!lgr->terminating check in smc_lgr_schedule_free_work()); the v12
check was racy against __smc_lgr_terminate() and the changes moved the
patch away from the diag reader race, so smc_core.c is back to the v10
change only; the Fixes tag for 2a0674fffb6b added in v11 is dropped
with them
- commit message: state that using the cached link->ibname avoids the
NULL pointer dereference and the IB device use-after-free, and that
link fields may be transiently zeroed or mixed in a dump racing with a
link failover, without affecting memory safety
- commit message: drop the claim that other kill paths leave the socket
hashed; every smc_conn_kill() ends in smc_close_active_abort(), so
sockets killed in the PEERCLOSEWAIT1/2, PEERFINCLOSEWAIT, PROCESSABORT
and APPFINCLOSEWAIT states are unhashed before close() even if the fd
is still open, and the dmb_nocopy unhash hides ACTIVE/APPCLOSEWAIT
sockets on loopback-ism; smcss visibility thus depends on close state
and device type; this supersedes the v10 entry below
Changes in v12:
- smc_core.c: guard smc_lgr_schedule_free_work() with !lgr->terminating
in addition to !lgr->freeing, preventing free_work from being re-armed
on a terminating LGR after smc_lgr_cleanup_early() has called
__smc_lgr_terminate() (which sets terminating=1 and cancels free_work
without setting freeing=1); without this a concurrent smc_conn_free()
decrementing conns_num to 0 would re-arm free_work, and the subsequent
kfree(lgr) in smc_lgr_free() would leave the armed work pointing at
freed memory
Changes in v11:
- smc_core.c: unregister connection unconditionally in smc_conn_free()
rather than gating it on !list_empty(&lgr->list), ensuring
alert_token_local is always reset to 0
- commit message: add Fixes tag for 2a0674fffb6b and clarify that
copying cached link->ibname eliminates the NULL dereference / UAF
during concurrent link clearing
Changes in v10:
- smc_core.c: move the unhash out of the top of smc_conn_kill() into the
smc_ism_support_dmb_nocopy() branch, right before smcd_buf_detach(); the
other kill paths leave ACTIVE/APPCLOSEWAIT sockets open, so they must
stay visible to smcss and keep their inuse accounting
- commit message: document the sock_diag output changes for sockets in
SMC_INIT and SMC_CLOSED (extension attributes omitted, diag_mode of
SMC-D in SMC_INIT) and that ibname is the name cached at link creation
Changes in v9:
- af_smc.c: change plain store in smc_listen_out_err() to
smp_store_release(&newsmcsk->sk_state, SMC_CLOSED) to order the
preceding alert_token_local clear (done under write_lock_bh() inside
smc_conn_free()) before the sk_state write; pairs with the
smp_load_acquire() in __smc_diag_dump() to close a use-after-free on
weakly ordered CPUs where a diag reader could observe SMC_CLOSED while
conn->lgr still appeared live
Changes in v8:
- smc_diag.c: use smp_load_acquire(&sk->sk_state) to prevent
speculative load reordering on weakly ordered CPUs (such as arm64)
during handshake abort fallback
- af_smc.c: pair with smp_store_release(&sk->sk_state, SMC_ACTIVE)
when transitioning out of SMC_INIT
- smc_diag.c: copy cached link->ibname instead of chasing
link->smcibdev->ibdev->name to prevent NULL pointer dereference / UAF
if __smcr_link_clear() zeroes the link during link failover
Changes in v7:
- no code changes; commit message only
- document the SMC_CLOSED leg of the early-exit guard and the rationale
that smcss already skips CLOSED sockets on the userspace side; add
Fixes: ae2be35cbed2 for the smc_conn_kill() hunk since
smcd_buf_detach() was introduced by that commit
Changes in v6:
- smc_diag.c: inline sk->sk_state != SMC_INIT into the else-if
condition so conn->lgr->is_smcd is never loaded for SMC_INIT sockets;
retain the SMC_INIT || SMC_CLOSED early-exit guard and out: label
from v5 to defensively skip the CONNINFO/LGRINFO/DMBINFO blocks for
sockets with no live connection
- smc_close.c: remove the unconditional unhash from the top of
smc_close_active_abort(); place it scoped immediately before each of
the two smc_conn_free() calls in the PEERCLOSEWAIT and PROCESSABORT
branches, and before smc_conn_free() in smc_close_passive_work();
this avoids prematurely hiding live SMC_ACTIVE/APPCLOSEWAIT sockets
from smcss
- smc_core.c: unhash at the top of smc_conn_kill(), before
smcd_buf_detach(), so the socket is off the hash table before
conn->sndbuf_desc is freed on the dmb_nocopy path
Changes in v5:
- guard conn/lgr/lnk blocks in __smc_diag_dump() with an
(SMC_INIT || SMC_CLOSED) state check placed after the
SMC_DIAG_FALLBACK nla_put; SMC_INIT can coexist with fallback
(smc_sendmsg() MSG_FASTOPEN path), so the guard must not suppress
the fallback reason
Changes in v4:
- dropped smc_conn_unhash() wrapper, conn->unhashed flag, and all
changes to af_smc.c, smc.h, smc_core.c and smc_core.h;
smc_unhash_sk() is already idempotent via sk_hashed(), so direct
calls at the two teardown sites in smc_close.c are sufficient
- dropped the __smc_release() hunk: it needs no change since the
subsequent unhash there is already a safe no-op
- fixed premature-unhash issue present in v3: smc_conn_free() must not
unhash because smc_conn_abort() calls it before
smc_switch_to_fallback() in both smc_listen_decline() and
smc_connect_rdma() error paths; unhashing there would make live
fallback sockets invisible to smcss
- likewise, the ISM/RDMA retry loops call smc_conn_abort() on a failed
attempt and then smc_conn_create() on the next device; unhashing in
smc_conn_free() would permanently hide the established connection from
smc_diag since smc_conn_create() does not re-hash the socket
Changes in v3:
- redesigned as a single patch; dropped the lgr_lnk_lock spinlock
approach and the 2-patch split
Changes in v2:
- split into a 2-patch series; patch 1/2 adds per-connection
lgr_lnk_lock infrastructure to smc_core, patch 2/2 fixes the diag
dump path using it
net/smc/af_smc.c | 10 +++++-----
net/smc/smc_close.c | 3 +++
net/smc/smc_core.c | 4 +++-
net/smc/smc_diag.c | 13 +++++++++----
4 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index XXXXXXX..XXXXXXX 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -976,7 +976,7 @@ static int smc_connect_fallback(struct smc_sock *smc, int reason_code)
smc_copy_sock_settings_to_clc(smc);
smc->connect_nonblock = 0;
if (smc->sk.sk_state == SMC_INIT)
- smc->sk.sk_state = SMC_ACTIVE;
+ smp_store_release(&smc->sk.sk_state, SMC_ACTIVE);
return 0;
}
@@ -1381,7 +1381,7 @@ static int smc_connect_rdma(struct smc_sock *smc,
smc_copy_sock_settings_to_clc(smc);
smc->connect_nonblock = 0;
if (smc->sk.sk_state == SMC_INIT)
- smc->sk.sk_state = SMC_ACTIVE;
+ smp_store_release(&smc->sk.sk_state, SMC_ACTIVE);
return 0;
connect_abort:
@@ -1484,7 +1484,7 @@ static int smc_connect_ism(struct smc_sock *smc,
smc_copy_sock_settings_to_clc(smc);
smc->connect_nonblock = 0;
if (smc->sk.sk_state == SMC_INIT)
- smc->sk.sk_state = SMC_ACTIVE;
+ smp_store_release(&smc->sk.sk_state, SMC_ACTIVE);
return 0;
connect_abort:
@@ -1951,7 +1951,7 @@ static void smc_listen_out_connected(struct smc_sock *new_smc)
struct sock *newsmcsk = &new_smc->sk;
if (newsmcsk->sk_state == SMC_INIT)
- newsmcsk->sk_state = SMC_ACTIVE;
+ smp_store_release(&newsmcsk->sk_state, SMC_ACTIVE);
smc_listen_out(new_smc);
}
@@ -1965,7 +1965,7 @@ static void smc_listen_out_err(struct smc_sock *new_smc)
this_cpu_inc(net->smc.smc_stats->srv_hshake_err_cnt);
if (newsmcsk->sk_state == SMC_INIT)
sock_put(&new_smc->sk); /* passive closing */
- newsmcsk->sk_state = SMC_CLOSED;
+ smp_store_release(&newsmcsk->sk_state, SMC_CLOSED);
smc_listen_out(new_smc);
}
diff --git a/net/smc/smc_close.c b/net/smc/smc_close.c
index XXXXXXX..XXXXXXX 100644
--- a/net/smc/smc_close.c
+++ b/net/smc/smc_close.c
@@ -154,6 +154,7 @@ void smc_close_active_abort(struct smc_sock *smc)
if (sk->sk_state != SMC_PEERABORTWAIT)
break;
sk->sk_state = SMC_CLOSED;
+ sk->sk_prot->unhash(sk);
smc_conn_free(&smc->conn);
release_clcsock = true;
sock_put(sk); /* passive closing */
@@ -165,6 +166,7 @@ void smc_close_active_abort(struct smc_sock *smc)
if (sk->sk_state != SMC_PEERABORTWAIT)
break;
sk->sk_state = SMC_CLOSED;
+ sk->sk_prot->unhash(sk);
smc_conn_free(&smc->conn);
release_clcsock = true;
break;
@@ -433,6 +435,7 @@ wakeup:
sk->sk_state_change(sk);
if ((sk->sk_state == SMC_CLOSED) &&
(sock_flag(sk, SOCK_DEAD) || !sk->sk_socket)) {
+ sk->sk_prot->unhash(sk);
smc_conn_free(conn);
if (smc->clcsock)
release_clcsock = true;
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index XXXXXXX..XXXXXXX 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -1530,8 +1530,10 @@ static void smc_conn_kill(struct smc_connection *conn, bool soft)
tasklet_kill(&conn->rx_tsklet);
else
tasklet_unlock_wait(&conn->rx_tsklet);
- if (smc_ism_support_dmb_nocopy(conn->lgr->smcd))
+ if (smc_ism_support_dmb_nocopy(conn->lgr->smcd)) {
+ smc->sk.sk_prot->unhash(&smc->sk);
smcd_buf_detach(conn);
+ }
} else {
smc_cdc_wait_pend_tx_wr(conn);
}
diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
index XXXXXXX..XXXXXXX 100644
--- a/net/smc/smc_diag.c
+++ b/net/smc/smc_diag.c
@@ -87,10 +87,11 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
r = nlmsg_data(nlh);
smc_diag_msg_common_fill(r, sk);
- r->diag_state = sk->sk_state;
+ r->diag_state = smp_load_acquire(&sk->sk_state);
if (smc->use_fallback)
r->diag_mode = SMC_DIAG_MODE_FALLBACK_TCP;
- else if (smc_conn_lgr_valid(&smc->conn) && smc->conn.lgr->is_smcd)
+ else if (r->diag_state != SMC_INIT &&
+ smc_conn_lgr_valid(&smc->conn) && smc->conn.lgr->is_smcd)
r->diag_mode = SMC_DIAG_MODE_SMCD;
else
r->diag_mode = SMC_DIAG_MODE_SMCR;
@@ -103,6 +104,10 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
if (nla_put(skb, SMC_DIAG_FALLBACK, sizeof(fallback), &fallback) < 0)
goto errout;
+ if (r->diag_state == SMC_INIT || r->diag_state == SMC_CLOSED ||
+ smc->use_fallback)
+ goto out;
+
if ((req->diag_ext & (1 << (SMC_DIAG_CONNINFO - 1))) &&
smc->conn.alert_token_local) {
struct smc_connection *conn = &smc->conn;
@@ -154,8 +159,7 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
.lnk[0].link_id = link->link_id,
};
- memcpy(linfo.lnk[0].ibname, link->smcibdev->ibdev->name,
- sizeof(link->smcibdev->ibdev->name));
+ memcpy(linfo.lnk[0].ibname, link->ibname, sizeof(link->ibname));
smc_gid_be16_convert(linfo.lnk[0].gid, link->gid);
smc_gid_be16_convert(linfo.lnk[0].peer_gid, link->peer_gid);
@@ -185,6 +189,7 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
goto errout;
}
+out:
nlmsg_end(skb, nlh);
return 0;
--
2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net v13] net/smc: fix lgr/lnk lifetime vs diag reader race
2026-10-23 15:03 [PATCH net v13] net/smc: fix lgr/lnk lifetime vs diag reader race Mahanta Jambigi
@ 2026-10-23 15:06 ` netdev-bot+sinfo
2026-10-23 17:18 ` Mahanta Jambigi
2026-10-24 15:03 ` sashiko-bot
1 sibling, 1 reply; 4+ messages in thread
From: netdev-bot+sinfo @ 2026-10-23 15:06 UTC (permalink / raw)
To: Mahanta Jambigi
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, alibuda, dust.li,
sidraya, hidayath, pasic, horms, tonylu, guwen, stable, netdev,
linux-s390, linux-rdma
Hi!
This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:
- How the issue was discovered, e.g. hit in production, hit during
development, syzbot report, manual code inspection, LLM or static
analysis tool scan.
- Whether the issue was actually triggered, or is only theoretical
(e.g. found by code inspection). If it was triggered please include
the symptoms, like the stack trace or error messages.
Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.
The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v13] net/smc: fix lgr/lnk lifetime vs diag reader race
2026-10-23 15:06 ` netdev-bot+sinfo
@ 2026-10-23 17:18 ` Mahanta Jambigi
0 siblings, 0 replies; 4+ messages in thread
From: Mahanta Jambigi @ 2026-10-23 17:18 UTC (permalink / raw)
To: netdev-bot+sinfo
Cc: andrew+netdev, davem, edumazet, kuba, pabeni, alibuda, dust.li,
sidraya, hidayath, pasic, horms, tonylu, guwen, stable, netdev,
linux-s390, linux-rdma
On 23/10/26 8:36 pm, netdev-bot+sinfo@kernel.org wrote:
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
> - How the issue was discovered, e.g. hit in production, hit during
> development, syzbot report, manual code inspection, LLM or static
> analysis tool scan.
The issue was found by manual code inspection while reviewing the diag
dump path in net/smc/smc_diag.c.
>
> - Whether the issue was actually triggered, or is only theoretical
> (e.g. found by code inspection). If it was triggered please include
> the symptoms, like the stack trace or error messages.
The races are theoretical — none of the three windows were triggered in
practice. The smc_conn_kill() / smcd_buf_detach() race requires an
active smcss poll to coincide with an SMC-D lgr termination on the
dmb_nocopy path; the smc_close_active_abort() and
smc_close_passive_work() races require a concurrent diag dump during the
narrow teardown window between sk_state = SMC_CLOSED and
smc_conn_free(). No stack trace or error message is available since the
bugs were not hit in a running system.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net v13] net/smc: fix lgr/lnk lifetime vs diag reader race
2026-10-23 15:03 [PATCH net v13] net/smc: fix lgr/lnk lifetime vs diag reader race Mahanta Jambigi
2026-10-23 15:06 ` netdev-bot+sinfo
@ 2026-10-24 15:03 ` sashiko-bot
1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-10-24 15:03 UTC (permalink / raw)
To: Mahanta Jambigi
Cc: linux-s390, linux-rdma, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] net/smc: use-after-free of conn->lgr in __smc_diag_dump() for SMC_CLOSED
--
commit 899c06b63c039c1819c9691a7be591a10540701e
Author: Mahanta Jambigi <mjambigi@linux.ibm.com>
net/smc: fix lgr/lnk lifetime vs diag reader race
This commit addresses races between sock_diag dumps and connection, link
group, or link teardown by unhashing sockets before freeing connection
resources, ordering sk_state transitions with acquire/release barriers, and
skipping extended diag attributes for SMC_INIT, SMC_CLOSED, and fallback
sockets.
> diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
> index bf0beaa23bdb6..84c45aabc196c 100644
> --- a/net/smc/smc_diag.c
> +++ b/net/smc/smc_diag.c
> @@ -87,10 +87,11 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
>
> r = nlmsg_data(nlh);
> smc_diag_msg_common_fill(r, sk);
> - r->diag_state = sk->sk_state;
> + r->diag_state = smp_load_acquire(&sk->sk_state);
> if (smc->use_fallback)
> r->diag_mode = SMC_DIAG_MODE_FALLBACK_TCP;
> - else if (smc_conn_lgr_valid(&smc->conn) && smc->conn.lgr->is_smcd)
> + else if (r->diag_state != SMC_INIT &&
> + smc_conn_lgr_valid(&smc->conn) && smc->conn.lgr->is_smcd)
> r->diag_mode = SMC_DIAG_MODE_SMCD;
> else
> r->diag_mode = SMC_DIAG_MODE_SMCR;
[Severity: Critical]
Can smc->conn.lgr->is_smcd still be dereferenced after smc->conn.lgr has been
freed when r->diag_state is SMC_CLOSED and smc->use_fallback is false?
Further down in __smc_diag_dump(), the extended attribute blocks check for
both SMC_INIT and SMC_CLOSED:
if (r->diag_state == SMC_INIT || r->diag_state == SMC_CLOSED ||
smc->use_fallback)
goto out;
However, the diag_mode check above only excludes r->diag_state == SMC_INIT.
If a socket reaches SMC_CLOSED without falling back to TCP while still hashed
in smc_hash, __smc_diag_dump() will still evaluate
smc_conn_lgr_valid(&smc->conn) and dereference smc->conn.lgr->is_smcd.
This appears reachable in two ways:
1. When a link group is scheduled for termination (for example via
smc_lgr_terminate_sched() or smc_smcd_terminate()), list_del_init(&lgr->list)
empties lgr->list. If a handshake subsequently aborts via
smc_conn_abort() -> smc_conn_free():
net/smc/smc_core.c:smc_conn_free() {
...
if (!list_empty(&lgr->list)) {
smc_buf_unuse(conn, lgr); /* allow buffer reuse */
smc_lgr_unregister_conn(conn);
}
...
smc_lgr_put(lgr); /* lgr_hold in smc_conn_create() */
}
smc_conn_free() skips smc_lgr_unregister_conn(conn), leaving
conn->alert_token_local non-zero while dropping the connection's reference to
lgr. If fallback does not happen (for instance in smc_listen_decline() when
reason_code < 0 or smc_switch_to_fallback() fails), new_smc->use_fallback
remains false and smc_listen_out_err() sets sk_state to SMC_CLOSED while the
socket stays hashed:
net/smc/af_smc.c:smc_listen_decline() {
...
smc_conn_abort(new_smc, local_first);
if (reason_code < 0 ||
smc_switch_to_fallback(new_smc, reason_code)) {
/* error, no fallback possible */
smc_listen_out_err(new_smc);
return;
}
}
Once the link group is freed, a concurrent __smc_diag_dump() reader seeing
r->diag_state == SMC_CLOSED and a non-zero alert_token_local can dereference
the freed smc->conn.lgr->is_smcd.
2. On the client side, if smc_connect_rdma() or smc_connect_ism() aborts
after smc_conn_create() with first_contact_local set, smc_conn_abort() clears
alert_token_local and frees lgr via smc_lgr_cleanup_early(). If
smc_connect_decline_fallback() fails without setting use_fallback, sk_state
remains SMC_INIT until the socket is closed via
__smc_release() -> smc_close_active():
net/smc/smc_close.c:smc_close_active() {
...
switch (sk->sk_state) {
case SMC_INIT:
sk->sk_state = SMC_CLOSED;
break;
...
}
Because smc_close_active() writes SMC_CLOSED with a plain store before
__smc_release() calls sk->sk_prot->unhash(sk), on weakly ordered CPUs a
concurrent __smc_diag_dump() reader using smp_load_acquire(&sk->sk_state) can
observe r->diag_state == SMC_CLOSED before the alert_token_local = 0 store is
visible, and dereference the freed smc->conn.lgr->is_smcd.
Should the diag_mode condition also check r->diag_state != SMC_CLOSED?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261023150317.1163397-1-mjambigi@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-24 15:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-23 15:03 [PATCH net v13] net/smc: fix lgr/lnk lifetime vs diag reader race Mahanta Jambigi
2026-10-23 15:06 ` netdev-bot+sinfo
2026-10-23 17:18 ` Mahanta Jambigi
2026-10-24 15:03 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox