* [PATCH net v3 1/3] net/smc: serialize link group free work scheduling
2026-10-13 9:17 [PATCH net v3 0/3] net/smc: fix link group scheduling, registration and work lifetime Chengfeng Ye
@ 2026-10-13 9:17 ` Chengfeng Ye
2026-10-13 9:17 ` [PATCH net v3 2/3] net/smc: serialize early link group cleanup with termination Chengfeng Ye
2026-10-13 9:17 ` [PATCH net v3 3/3] net/smc: hold a link group reference for free work Chengfeng Ye
2 siblings, 0 replies; 4+ messages in thread
From: Chengfeng Ye @ 2026-10-13 9:17 UTC (permalink / raw)
To: alibuda, dust.li, sidraya, mjambigi, davem, edumazet, kuba, pabeni
Cc: tonylu, guwen, horms, linux-rdma, linux-s390, netdev,
linux-kernel, stable
smc_lgr_schedule_free_work() checks lgr->freeing without the link group
list lock held by the teardown paths. A concurrent smc_lgr_free_work()
can set freeing and cancel the delayed work between that check and
mod_delayed_work(), leaving a timer pending on a freed link group:
CPU 0 (smc_conn_free) CPU 1 (smc_lgr_free_work)
observe lgr->freeing == 0
set lgr->freeing under lgr_lock
cancel_delayed_work()
smc_lgr_free(): drop lgr reference
mod_delayed_work()
smc_lgr_put(): drop last reference and free lgr
When the timer expires, the timer core accesses the freed memory.
BUG: KASAN: use-after-free in __run_timers+0x86d/0x8d0
Write of size 8 at addr ffff8881186902a8 by task swapper/2/0
Call Trace:
<IRQ>
__run_timers+0x86d/0x8d0
timer_expire_remote+0xd3/0x120
tmigr_handle_remote_up+0x4f4/0xab0
__walk_groups_from+0x40/0x150
tmigr_handle_remote+0x229/0x2c0
run_timer_softirq+0x1f5/0x250
handle_softirqs+0x18d/0x5b0
</IRQ>
Hold the existing link group list lock across the freeing check and
mod_delayed_work(). Set freeing under that lock in early cleanup as well,
before __smc_lgr_terminate() cancels the work. An SMC-D server releases
smc_server_lgr_pending before receiving CONFIRM, so another connection can
reuse the group and race its removal with early cleanup.
Give freeing its own storage. The neighbouring sync_err and terminating
bitfields are written without the list lock; their read-modify-write
updates must not overwrite the freeing transition.
Rearming then either precedes the freeing transition and is caught by the
subsequent cancellation, or sees freeing set and is skipped.
smc_conn_free() retains its link group reference until after scheduling,
keeping the group alive while acquiring the lock.
Fixes: 8e316b9e7260 ("net/smc: improve link group freeing")
Cc: stable@vger.kernel.org
Assisted-by: GPT-6.1-Sol
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
Changes in v3:
- Rebase onto current net/main.
- Correct the cancellation comment without implying that checking freeing
protects an already-running callback. Patch 3 supplies that protection.
Changes in v2:
- Give freeing its own storage so adjacent bitfield updates cannot
overwrite the list-lock-protected transition.
- Set freeing under the list lock during early cleanup to prevent a
concurrent SMC-D connection from rearming after cancellation.
- Keep asynchronous cancellation without adding work-owned references.
- Send with the early-cleanup ownership fix as an ordered two-patch series.
v1: https://lore.kernel.org/r/20260927074520.3694663-1-nicoyip.dev@gmail.com/
v2: https://lore.kernel.org/r/20261003183237.2284245-2-nicoyip.dev@gmail.com/
net/smc/smc_core.c | 8 +++++++-
net/smc/smc_core.h | 2 +-
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 9974149659c2..660cd41ec720 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -80,6 +80,10 @@ static void smc_ibdev_cnt_dec(struct smc_link *lnk)
static void smc_lgr_schedule_free_work(struct smc_link_group *lgr)
{
+ spinlock_t *lgr_lock; /* protects lgr->freeing */
+
+ smc_lgr_list_head(lgr, &lgr_lock);
+ spin_lock_bh(lgr_lock);
/* client link group creation always follows the server link group
* creation. For client use a somewhat higher removal delay time,
* otherwise there is a risk of out-of-sync link groups.
@@ -90,6 +94,7 @@ static void smc_lgr_schedule_free_work(struct smc_link_group *lgr)
SMC_LGR_FREE_DELAY_CLNT :
SMC_LGR_FREE_DELAY_SERV);
}
+ spin_unlock_bh(lgr_lock);
}
/* Register connection's alert token in our lookup structure.
@@ -691,6 +696,7 @@ void smc_lgr_cleanup_early(struct smc_link_group *lgr)
/* do not use this link group for new connections */
if (!list_empty(&lgr->list))
list_del_init(&lgr->list);
+ lgr->freeing = 1;
spin_unlock_bh(lgr_lock);
__smc_lgr_terminate(lgr, true);
}
@@ -1565,7 +1571,7 @@ static void __smc_lgr_terminate(struct smc_link_group *lgr, bool soft)
if (lgr->terminating)
return; /* lgr already terminating */
- /* cancel free_work sync, will terminate when lgr->freeing is set */
+ /* cancel pending free_work */
cancel_delayed_work(&lgr->free_work);
lgr->terminating = 1;
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 5c18f08a4c8a..b8c67f277337 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -303,7 +303,7 @@ struct smc_link_group {
struct workqueue_struct *tx_wq; /* wq for conn. tx workers */
u8 sync_err : 1; /* lgr no longer fits to peer */
u8 terminating : 1;/* lgr is terminating */
- u8 freeing : 1; /* lgr is being freed */
+ bool freeing; /* lgr is being freed */
refcount_t refcnt; /* lgr reference count */
bool is_smcd; /* SMC-R or SMC-D */
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH net v3 2/3] net/smc: serialize early link group cleanup with termination
2026-10-13 9:17 [PATCH net v3 0/3] net/smc: fix link group scheduling, registration and work lifetime Chengfeng Ye
2026-10-13 9:17 ` [PATCH net v3 1/3] net/smc: serialize link group free work scheduling Chengfeng Ye
@ 2026-10-13 9:17 ` Chengfeng Ye
2026-10-13 9:17 ` [PATCH net v3 3/3] net/smc: hold a link group reference for free work Chengfeng Ye
2 siblings, 0 replies; 4+ messages in thread
From: Chengfeng Ye @ 2026-10-13 9:17 UTC (permalink / raw)
To: alibuda, dust.li, sidraya, mjambigi, davem, edumazet, kuba, pabeni
Cc: tonylu, guwen, horms, linux-rdma, linux-s390, netdev,
linux-kernel, stable
smc_lgr_cleanup_early() enters __smc_lgr_terminate() even when another
teardown path has already claimed lgr->freeing. Both callers can observe
lgr->terminating clear before either sets it, then tear down the same
link group, causing use-after-free and duplicate resource release.
BUG: KASAN: use-after-free in __smc_lgr_terminate+0x393/0x3a0
Write of size 1 at addr ffff8880bf2c0300 by task poc/106
Call Trace:
__smc_lgr_terminate+0x393/0x3a0
__smc_connect+0x2e3d/0x4930
smc_connect+0x42c/0x580
__sys_connect+0xfc/0x130
__x64_sys_connect+0x6d/0xb0
Check freeing under the link group list lock before unlinking the group
or entering termination. The existing locked freeing transition then
claims exclusive teardown ownership; an early cleanup that loses the
claim leaves teardown to the existing owner and does not remove the group
from a device teardown's private list.
Keep a temporary link group reference across smc_conn_free() and early
cleanup in smc_conn_abort(). Once the connection is unregistered, a
termination worker can finish without taking its socket lock. The extra
reference keeps the allocation alive until the ownership check returns.
A new group is published before its first connection is registered. Hold
it before publication until registration or failed-registration cleanup
finishes. The reference keeps the allocation alive, but teardown can still
destroy the group's resources before registration. Take the list lock
around the freeing check and first registration, nesting conns_lock as in
the reuse path. Reject a group already claimed for teardown and clear
conn->lgr, since that connection never acquired a group reference.
On success, acquire the connection reference before dropping the temporary
hold. A later terminator sees the registered connection and must acquire
its socket lock before removing it. On failure, keep the temporary hold
through early cleanup and then release it. Reused groups retain their
existing reference protocol.
This depends on "net/smc: serialize link group free work scheduling" for
the standalone freeing bool and the locked early-cleanup transition.
Fixes: f9aab6f2ce57 ("net/smc: immediate freeing in smc_lgr_cleanup_early()")
Cc: stable@vger.kernel.org
Assisted-by: GPT-6.1-Sol
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
Changes in v3:
- Serialize first registration with the teardown claim under the group list
lock, nesting conns_lock in the same order as connection reuse.
- Reject groups already being freed and clear conn->lgr on that path.
- Document the scheduling fix prerequisite in the permanent commit message.
Changes in v2:
- Pin newly created link groups before publication, covering the
registration failure handoff to early cleanup. Drop the temporary hold
after cleanup on failure, or after the connection hold on success.
- Send after the free-work scheduling fix and reuse its standalone
freeing bool and locked early-cleanup transition.
- Pin the link group across the smc_conn_abort() early-cleanup ownership
check.
v1: https://lore.kernel.org/r/20260927063607.3691520-1-nicoyip.dev@gmail.com/
v2: https://lore.kernel.org/r/20261003183237.2284245-3-nicoyip.dev@gmail.com/
net/smc/af_smc.c | 8 ++++++--
net/smc/smc_core.c | 21 ++++++++++++++++++---
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index e9f93b3ab435..97b0a0b51066 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1011,12 +1011,16 @@ static void smc_conn_abort(struct smc_sock *smc, int local_first)
struct smc_link_group *lgr = conn->lgr;
bool lgr_valid = false;
- if (smc_conn_lgr_valid(conn))
+ if (local_first && smc_conn_lgr_valid(conn)) {
lgr_valid = true;
+ smc_lgr_hold(lgr);
+ }
smc_conn_free(conn);
- if (local_first && lgr_valid)
+ if (lgr_valid) {
smc_lgr_cleanup_early(lgr);
+ smc_lgr_put(lgr);
+ }
}
/* check if there is a rdma device available for this connection. */
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index 660cd41ec720..c1bb284c4233 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -693,6 +693,10 @@ void smc_lgr_cleanup_early(struct smc_link_group *lgr)
smc_lgr_list_head(lgr, &lgr_lock);
spin_lock_bh(lgr_lock);
+ if (lgr->freeing) {
+ spin_unlock_bh(lgr_lock);
+ return;
+ }
/* do not use this link group for new connections */
if (!list_empty(&lgr->list))
list_del_init(&lgr->list);
@@ -999,6 +1003,7 @@ static int smc_lgr_create(struct smc_sock *smc, struct smc_init_info *ini)
lgr->buf_type = lgr->net->smc.sysctl_smcr_buf_type;
atomic_inc(&lgr_cnt);
}
+ smc_lgr_hold(lgr); /* lgr_put in smc_conn_create() */
smc->conn.lgr = lgr;
spin_lock_bh(lgr_lock);
list_add_tail(&lgr->list, lgr_list);
@@ -2049,15 +2054,25 @@ int smc_conn_create(struct smc_sock *smc, struct smc_init_info *ini)
if (rc)
goto out;
lgr = conn->lgr;
- write_lock_bh(&lgr->conns_lock);
- rc = smc_lgr_register_conn(conn, true);
- write_unlock_bh(&lgr->conns_lock);
+ spin_lock_bh(lgr_lock);
+ if (lgr->freeing) {
+ conn->lgr = NULL;
+ rc = SMC_CLC_DECL_SYNCERR;
+ } else {
+ write_lock_bh(&lgr->conns_lock);
+ rc = smc_lgr_register_conn(conn, true);
+ write_unlock_bh(&lgr->conns_lock);
+ }
+ spin_unlock_bh(lgr_lock);
if (rc) {
smc_lgr_cleanup_early(lgr);
+ smc_lgr_put(lgr); /* lgr_hold in smc_lgr_create() */
goto out;
}
}
smc_lgr_hold(conn->lgr); /* lgr_put in smc_conn_free() */
+ if (ini->first_contact_local)
+ smc_lgr_put(conn->lgr); /* lgr_hold in smc_lgr_create() */
if (!conn->lgr->is_smcd)
smcr_link_hold(conn->lnk); /* link_put in smc_conn_free() */
conn->freed = 0;
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH net v3 3/3] net/smc: hold a link group reference for free work
2026-10-13 9:17 [PATCH net v3 0/3] net/smc: fix link group scheduling, registration and work lifetime Chengfeng Ye
2026-10-13 9:17 ` [PATCH net v3 1/3] net/smc: serialize link group free work scheduling Chengfeng Ye
2026-10-13 9:17 ` [PATCH net v3 2/3] net/smc: serialize early link group cleanup with termination Chengfeng Ye
@ 2026-10-13 9:17 ` Chengfeng Ye
2 siblings, 0 replies; 4+ messages in thread
From: Chengfeng Ye @ 2026-10-13 9:17 UTC (permalink / raw)
To: alibuda, dust.li, sidraya, mjambigi, davem, edumazet, kuba, pabeni
Cc: tonylu, guwen, horms, linux-rdma, linux-s390, netdev,
linux-kernel, stable
__smc_lgr_terminate() cancels free_work asynchronously. If the callback
has already started, it can still access the group after the terminator
drops the last reference. The callback has to dereference the group to
find its list lock and check freeing, so that check cannot protect its
own access to the allocation. This lifetime gap was identified during
code review of the scheduling and early-cleanup fixes.
Take a group reference before mod_delayed_work(). Keep it when a new
execution is queued, including when a previous callback is still running.
If the work was already pending, release the extra reference because that
pending execution already owns one. Release the queued reference when
cancellation succeeds, or at every callback exit when it executes.
Use the reference-aware cancellation helper for connection reuse,
termination and cancellation of a rearmed instance by the free worker.
The callback's reference keeps the group alive even after smc_lgr_free()
drops the teardown owner's reference.
Keep cancellation asynchronous. Waiting for free_work from connection
setup can deadlock: the free worker drains tx_wq, whose worker needs the
socket lock held by setup.
This builds on "net/smc: serialize link group free work scheduling" and
"net/smc: serialize early link group cleanup with termination". Together
they close admission before cancellation and give resource teardown a
single owner; the work reference then protects callbacks that are already
running when that owner cancels pending work.
Link: https://lore.kernel.org/r/179113886433.434549.10069135669585050618@kernel.org/
Fixes: 13085e1b5cab ("net/smc: fix deadlock triggered by cancel_delayed_work_syn()")
Cc: stable@vger.kernel.org
Assisted-by: GPT-6.1-Sol
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
New in v3:
- Address the independent running-free_work lifetime issue reported against
both v2 patches by sashiko, with the complete reference protocol in this commit.
- Cover new scheduling, rearming, all three cancellation sites and every
callback exit while retaining asynchronous cancellation.
Review: https://lore.kernel.org/r/179113886433.434549.10069135669585050618@kernel.org/
Review: https://lore.kernel.org/r/179113886579.434549.12546097168507601074@kernel.org/
net/smc/smc_core.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index c1bb284c4233..9b54f873c6a9 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -89,14 +89,22 @@ static void smc_lgr_schedule_free_work(struct smc_link_group *lgr)
* otherwise there is a risk of out-of-sync link groups.
*/
if (!lgr->freeing) {
- mod_delayed_work(system_percpu_wq, &lgr->free_work,
- (!lgr->is_smcd && lgr->role == SMC_CLNT) ?
- SMC_LGR_FREE_DELAY_CLNT :
- SMC_LGR_FREE_DELAY_SERV);
+ smc_lgr_hold(lgr); /* released by the worker or cancellation */
+ if (mod_delayed_work(system_percpu_wq, &lgr->free_work,
+ (!lgr->is_smcd && lgr->role == SMC_CLNT) ?
+ SMC_LGR_FREE_DELAY_CLNT :
+ SMC_LGR_FREE_DELAY_SERV))
+ smc_lgr_put(lgr); /* pending work already owns a reference */
}
spin_unlock_bh(lgr_lock);
}
+static void smc_lgr_cancel_free_work(struct smc_link_group *lgr)
+{
+ if (cancel_delayed_work(&lgr->free_work))
+ smc_lgr_put(lgr); /* cancelled work will not release its reference */
+}
+
/* Register connection's alert token in our lookup structure.
* To use rbtrees we have to implement our own insert core.
* Requires @conns_lock
@@ -733,19 +741,19 @@ static void smc_lgr_free_work(struct work_struct *work)
spin_lock_bh(lgr_lock);
if (lgr->freeing) {
spin_unlock_bh(lgr_lock);
- return;
+ goto out;
}
read_lock_bh(&lgr->conns_lock);
conns = RB_EMPTY_ROOT(&lgr->conns_all);
read_unlock_bh(&lgr->conns_lock);
if (!conns) { /* number of lgr connections is no longer zero */
spin_unlock_bh(lgr_lock);
- return;
+ goto out;
}
list_del_init(&lgr->list); /* remove from smc_lgr_list */
lgr->freeing = 1; /* this instance does the freeing, no new schedule */
spin_unlock_bh(lgr_lock);
- cancel_delayed_work(&lgr->free_work);
+ smc_lgr_cancel_free_work(lgr);
if (!lgr->is_smcd && !lgr->terminating)
smc_llc_send_link_delete_all(lgr, true,
@@ -755,6 +763,8 @@ static void smc_lgr_free_work(struct work_struct *work)
if (!lgr->is_smcd)
smcr_lgr_link_deactivate_all(lgr);
smc_lgr_free(lgr);
+out:
+ smc_lgr_put(lgr); /* smc_lgr_hold in smc_lgr_schedule_free_work() */
}
static void smc_lgr_terminate_work(struct work_struct *work)
@@ -1576,8 +1586,8 @@ static void __smc_lgr_terminate(struct smc_link_group *lgr, bool soft)
if (lgr->terminating)
return; /* lgr already terminating */
- /* cancel pending free_work */
- cancel_delayed_work(&lgr->free_work);
+ /* A running free_work holds its own link group reference. */
+ smc_lgr_cancel_free_work(lgr);
lgr->terminating = 1;
/* kill remaining link group connections */
@@ -2029,8 +2039,8 @@ int smc_conn_create(struct smc_sock *smc, struct smc_init_info *ini)
conn->lgr = lgr;
rc = smc_lgr_register_conn(conn, false);
write_unlock_bh(&lgr->conns_lock);
- if (!rc && delayed_work_pending(&lgr->free_work))
- cancel_delayed_work(&lgr->free_work);
+ if (!rc)
+ smc_lgr_cancel_free_work(lgr);
break;
}
write_unlock_bh(&lgr->conns_lock);
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread