FAILED: patch "[PATCH] ksmbd: rewrite stop_sessions() with restartable iteration" failed to apply to 6.6-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 6.6-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
git checkout FETCH_HEAD
git cherry-pick -x c444139cb747bf6de1922b39900fdf02281490f4
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026050720-idealist-crayon-e443@gregkh' --subject-prefix 'PATCH 6.6.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From c444139cb747bf6de1922b39900fdf02281490f4 Mon Sep 17 00:00:00 2001
From: DaeMyung Kang <[email protected]>
Date: Sat, 25 Apr 2026 18:38:28 +0900
Subject: [PATCH] ksmbd: rewrite stop_sessions() with restartable iteration
stop_sessions() walks conn_list with hash_for_each() and, for every entry, drops conn_list_lock across the transport ->shutdown() call before re-acquiring the read lock to continue the loop. The hash walk relies on cross-iteration state (the current bucket and the hlist position), which is not preserved across unlock/relock: if another thread performs a list mutation during the unlocked window, the ongoing iteration becomes unreliable and can re-visit connections that have already been handled or skip connections that have not. The outer if (!hash_empty(conn_list)) goto again; retry masks the symptom in the common case but does not address the unsafe iteration itself.
Reframe the loop so it never relies on iterator state across unlock/relock. Under conn_list_lock held for read, pick the first connection whose ->shutdown() has not yet been issued by this path, pin it by taking an extra reference, record that fact on the connection and mark it EXITING while still inside the locked walk, then drop the lock. Then call ->shutdown() outside the lock, drop the pin (freeing the connection if the handler already released its reference), and restart from the top.
Use a new per-connection flag, conn->stop_called, as the "shutdown issued from stop_sessions()" marker rather than reusing the status state. ksmbd_conn_set_exiting() is also invoked by ksmbd_sessions_deregister() on sibling channels of a multichannel session without issuing a transport shutdown, so treating KSMBD_SESS_EXITING as "already handled here" would skip connections that still need shutdown() to wake their handler out of recv(), leaving the outer retry waiting indefinitely for the hash to drain. stop_sessions() is serialised by init_lock in ksmbd_conn_transport_destroy(), so writing stop_called under the read lock has no other writer.
Set EXITING inside the locked walk so the selection, the stop_called marker, and the status transition all happen together, and guard against regressing a connection that has already advanced to KSMBD_SESS_RELEASING on its own (for example, if the handler exited its receive loop for an unrelated reason between teardown steps).
When the pin drop is the last put, release the transport and pair ida_destroy(&target->async_ida) with the ida_init() done in ksmbd_conn_alloc(), so stop_sessions() retiring a connection on its own does not leak the xarray backing of the embedded async_ida.
The outer retry with msleep() is kept to wait for handler threads to reach ksmbd_conn_free() and drain the hash.
Observed with an instrumented build that logs one line per visit and widens the unlocked window before ->shutdown() by 200 ms, under five concurrent cifs mounts (nosharesock, one connection each):
* Current code: the same connection address is revisited many times during a single stop_sessions() call and ->shutdown() is invoked well beyond the number of live connections before the hash finally drains.
* Rewritten code: each live connection produces exactly one
->shutdown() call; the function returns as soon as the hash is
empty.Functional teardown via ksmbd.control --shutdown with the same five mounts completes cleanly on the rewritten path.
Performance is observably unchanged. Tearing down N concurrent nosharesock cifs connections with ksmbd.control --shutdown + rmmod ksmbd takes essentially the same wall time before and after the rewrite:
N before after
10 4.93s 5.34s
30 7.34s 7.03s
50 7.31s 7.01s (3-run avg: 7.04s vs 7.25s)
100 6.98s 6.78s
200 6.77s 6.89sand the number of ->shutdown() calls equals the number of live connections on both paths when the race is not widened. The teardown is dominated by the msleep(100)-based outer retry waiting for handler threads to run ksmbd_conn_free(), not by the iteration itself; the restartable loop's worst-case O(N^2) visit cost is in the microseconds even at N=200 and sits far below the msleep(100) granularity.
Applied alone on top of ksmbd-for-next-next, this patch does not introduce a new leak site. Under the same reproducer (10x concurrent-holders + ss -K + ksmbd.control --shutdown + rmmod), the tree still shows the pre-existing per-connection transport leak count that arises when the last refcount drop lands in one of ksmbd_conn_r_count_dec(), __free_opinfo() or session_fd_check() - all of which end with a bare kfree() today. kmemleak backtraces for the unreferenced objects point into the TCP accept path (sk_clone -> inet_csk_clone_lock, sock_alloc_inode) and none involve stop_sessions(). Plugging those bare-kfree sites is the responsibility of the follow-up patch.
fs/smb/server/connection.c +39 -9
@@ -540,24 +540,54 @@ int ksmbd_conn_transport_init(void)static void stop_sessions (void ) { -struct ksmbd_conn *conn; +struct ksmbd_conn *conn, *target;struct ksmbd_transport *t; +bool any;int bkt; +/* +* Serialised via init_lock; no concurrent stop_sessions() can +* touch conn->stop_called, so writing it under the read lock is +* safe. +*/ again: + target =NULL ; + any = false;down_read (&conn_list_lock);hash_for_each (conn_list, bkt, conn, hlist) { - t = conn->transport; -ksmbd_conn_set_exiting (conn); -if (t->ops->shutdown) { -up_read (&conn_list_lock); - t->ops->shutdown (t); -down_read (&conn_list_lock); - } + any = true; +if (conn->stop_called) +continue ; +atomic_inc (&conn->refcnt); + conn->stop_called = true; +/* +* Mark the connection EXITING while still holding the +* read lock so the selection and the status transition +* happen together. Do not regress a connection that has +* already advanced to RELEASING on its own (e.g. the +* handler exited its receive loop for an unrelated +* reason). +*/ +if (READ_ONCE (conn->status) !=KSMBD_SESS_RELEASING ) +ksmbd_conn_set_exiting (conn); + target = conn; +break ; }up_read (&conn_list_lock); -if (!hash_empty (conn_list)) { +if (target) { + t = target->transport; +if (t->ops->shutdown) + t->ops->shutdown (t); +if (atomic_dec_and_test (&target->refcnt)) { +ida_destroy (&target->async_ida); + t->ops->free_transport (t); +kfree (target); + } + goto again; + } + +if (any) {msleep (100 ); goto again; }
fs/smb/server/connection.h +1 -0
@@ -49,6 +49,7 @@ struct ksmbd_conn {struct mutex srv_mutex;int status;unsigned int cli_cap; +bool stop_called;union {__be32 inet_addr;#if IS_ENABLED (CONFIG_IPV6 )
Sasha Levin
maintainerPATCH 1/1 ksmbd: rewrite stop_sessions() with restartable iteration
From: DaeMyung Kang <[email protected]>
[ Upstream commit c444139cb747bf6de1922b39900fdf02281490f4 ]
stop_sessions() walks conn_list with hash_for_each() and, for every entry, drops conn_list_lock across the transport ->shutdown() call before re-acquiring the read lock to continue the loop. The hash walk relies on cross-iteration state (the current bucket and the hlist position), which is not preserved across unlock/relock: if another thread performs a list mutation during the unlocked window, the ongoing iteration becomes unreliable and can re-visit connections that have already been handled or skip connections that have not. The outer if (!hash_empty(conn_list)) goto again; retry masks the symptom in the common case but does not address the unsafe iteration itself.
Reframe the loop so it never relies on iterator state across unlock/relock. Under conn_list_lock held for read, pick the first connection whose ->shutdown() has not yet been issued by this path, pin it by taking an extra reference, record that fact on the connection and mark it EXITING while still inside the locked walk, then drop the lock. Then call ->shutdown() outside the lock, drop the pin (freeing the connection if the handler already released its reference), and restart from the top.
Use a new per-connection flag, conn->stop_called, as the "shutdown issued from stop_sessions()" marker rather than reusing the status state. ksmbd_conn_set_exiting() is also invoked by ksmbd_sessions_deregister() on sibling channels of a multichannel session without issuing a transport shutdown, so treating KSMBD_SESS_EXITING as "already handled here" would skip connections that still need shutdown() to wake their handler out of recv(), leaving the outer retry waiting indefinitely for the hash to drain. stop_sessions() is serialised by init_lock in ksmbd_conn_transport_destroy(), so writing stop_called under the read lock has no other writer.
Set EXITING inside the locked walk so the selection, the stop_called marker, and the status transition all happen together, and guard against regressing a connection that has already advanced to KSMBD_SESS_RELEASING on its own (for example, if the handler exited its receive loop for an unrelated reason between teardown steps).
When the pin drop is the last put, release the transport and pair ida_destroy(&target->async_ida) with the ida_init() done in ksmbd_conn_alloc(), so stop_sessions() retiring a connection on its own does not leak the xarray backing of the embedded async_ida.
The outer retry with msleep() is kept to wait for handler threads to reach ksmbd_conn_free() and drain the hash.
Observed with an instrumented build that logs one line per visit and widens the unlocked window before ->shutdown() by 200 ms, under five concurrent cifs mounts (nosharesock, one connection each):
* Current code: the same connection address is revisited many times during a single stop_sessions() call and ->shutdown() is invoked well beyond the number of live connections before the hash finally drains.
* Rewritten code: each live connection produces exactly one
->shutdown() call; the function returns as soon as the hash is
empty.Functional teardown via ksmbd.control --shutdown with the same five mounts completes cleanly on the rewritten path.
Performance is observably unchanged. Tearing down N concurrent nosharesock cifs connections with ksmbd.control --shutdown + rmmod ksmbd takes essentially the same wall time before and after the rewrite:
N before after
10 4.93s 5.34s
30 7.34s 7.03s
50 7.31s 7.01s (3-run avg: 7.04s vs 7.25s)
100 6.98s 6.78s
200 6.77s 6.89sand the number of ->shutdown() calls equals the number of live connections on both paths when the race is not widened. The teardown is dominated by the msleep(100)-based outer retry waiting for handler threads to run ksmbd_conn_free(), not by the iteration itself; the restartable loop's worst-case O(N^2) visit cost is in the microseconds even at N=200 and sits far below the msleep(100) granularity.
Applied alone on top of ksmbd-for-next-next, this patch does not introduce a new leak site. Under the same reproducer (10x concurrent-holders + ss -K + ksmbd.control --shutdown + rmmod), the tree still shows the pre-existing per-connection transport leak count that arises when the last refcount drop lands in one of ksmbd_conn_r_count_dec(), __free_opinfo() or session_fd_check() - all of which end with a bare kfree() today. kmemleak backtraces for the unreferenced objects point into the TCP accept path (sk_clone -> inet_csk_clone_lock, sock_alloc_inode) and none involve stop_sessions(). Plugging those bare-kfree sites is the responsibility of the follow-up patch.
--- fs/smb/server/connection.c | 46 +++++++++++++++++++++++++++++++------- fs/smb/server/connection.h | 1 + 2 files changed, 39 insertions(+), 8 deletions(-)
fs/smb/server/connection.c +38 -8
@@ -478,23 +478,53 @@ int ksmbd_conn_transport_init(void)static void stop_sessions (void ) { -struct ksmbd_conn *conn; +struct ksmbd_conn *conn, *target;struct ksmbd_transport *t; +bool any; +/* +* Serialised via init_lock; no concurrent stop_sessions() can +* touch conn->stop_called, so writing it under the read lock is +* safe. +*/ again: + target =NULL ; + any = false;down_read (&conn_list_lock);list_for_each_entry (conn, &conn_list, conns_list) { - t = conn->transport; -ksmbd_conn_set_exiting (conn); -if (t->ops->shutdown) { -up_read (&conn_list_lock); + any = true; +if (conn->stop_called) +continue ; +atomic_inc (&conn->refcnt); + conn->stop_called = true; +/* +* Mark the connection EXITING while still holding the +* read lock so the selection and the status transition +* happen together. Do not regress a connection that has +* already advanced to RELEASING on its own (e.g. the +* handler exited its receive loop for an unrelated +* reason). +*/ +if (READ_ONCE (conn->status) !=KSMBD_SESS_RELEASING ) +ksmbd_conn_set_exiting (conn); + target = conn; +break ; + } +up_read (&conn_list_lock); + +if (target) { + t = target->transport; +if (t->ops->shutdown) t->ops->shutdown (t); -down_read (&conn_list_lock); +if (atomic_dec_and_test (&target->refcnt)) { +ida_destroy (&target->async_ida); + t->ops->free_transport (t); +kfree (target); } + goto again; } -up_read (&conn_list_lock); -if (!list_empty (&conn_list)) { +if (any) {schedule_timeout_interruptible (HZ /10 );/* 100ms */ goto again; }
fs/smb/server/connection.h +1 -0
@@ -46,6 +46,7 @@ struct ksmbd_conn {struct mutex srv_mutex;int status;unsigned int cli_cap; +bool stop_called;union {__be32 inet_addr;#if IS_ENABLED (CONFIG_IPV6 )
Signature · 1 line
2.53.0
Re: [PATCH 6.6.y] ksmbd: rewrite stop_sessions() with restartable iteration
Quoted 193 lines · Sasha Levin
From: DaeMyung Kang <[email protected]>
[ Upstream commit
c444139cb747bf6de1922b39900fdf02281490f4]
stop_sessions()walksconn_listwithhash_for_each()and, for every entry, dropsconn_list_lockacross the transport ->shutdown() call before re-acquiring the read lock to continue the loop. The hash walk relies on cross-iteration state (the current bucket and the hlist position), which is not preserved across unlock/relock: if another thread performs a list mutation during the unlocked window, the ongoing iteration becomes unreliable and can re-visit connections that have already been handled or skip connections that have not. The outerif (!hash_empty(conn_list)) goto again;retry masks the symptom in the common case but does not address the unsafe iteration itself.Reframe the loop so it never relies on iterator state across unlock/relock. Under
conn_list_lockheld for read, pick the first connection whose ->shutdown() has not yet been issued by this path, pin it by taking an extra reference, record that fact on the connection and mark it EXITING while still inside the locked walk, then drop the lock. Then call ->shutdown() outside the lock, drop the pin (freeing the connection if the handler already released its reference), and restart from the top.Use a new per-connection flag,
conn->stop_called, as the "shutdown issued fromstop_sessions()" marker rather than reusing the status state.ksmbd_conn_set_exiting()is also invoked byksmbd_sessions_deregister()on sibling channels of a multichannel session without issuing a transport shutdown, so treatingKSMBD_SESS_EXITINGas "already handled here" would skip connections that still needshutdown()to wake their handler out ofrecv(), leaving the outer retry waiting indefinitely for the hash to drain.stop_sessions()is serialised byinit_lockinksmbd_conn_transport_destroy(), so writingstop_calledunder the read lock has no other writer.Set EXITING inside the locked walk so the selection, the
stop_calledmarker, and the status transition all happen together, and guard against regressing a connection that has already advanced toKSMBD_SESS_RELEASINGon its own (for example, if the handler exited its receive loop for an unrelated reason between teardown steps).When the pin drop is the last put, release the transport and pair
ida_destroy(&target->async_ida)with theida_init()done inksmbd_conn_alloc(), sostop_sessions()retiring a connection on its own does not leak the xarray backing of the embeddedasync_ida.The outer retry with
msleep()is kept to wait for handler threads to reachksmbd_conn_free()and drain the hash.Observed with an instrumented build that logs one line per visit and widens the unlocked window before ->shutdown() by 200 ms, under five concurrent cifs mounts (nosharesock, one connection each):
* Current code: the same connection address is revisited many times during a single
stop_sessions()call and ->shutdown() is invoked well beyond the number of live connections before the hash finally drains.* Rewritten code: each live connection produces exactly one
->shutdown() call; the function returns as soon as the hash is empty.Functional teardown via
ksmbd.control --shutdownwith the same five mounts completes cleanly on the rewritten path.Performance is observably unchanged. Tearing down N concurrent nosharesock cifs connections with
ksmbd.control --shutdown+rmmod ksmbdtakes essentially the same wall time before and after the rewrite:N before after 10 4.93s 5.34s 30 7.34s 7.03s 50 7.31s 7.01s (3-run avg: 7.04s vs 7.25s) 100 6.98s 6.78s 200 6.77s 6.89sand the number of ->shutdown() calls equals the number of live connections on both paths when the race is not widened. The teardown is dominated by the msleep(100)-based outer retry waiting for handler threads to run
ksmbd_conn_free(), not by the iteration itself; the restartable loop's worst-case O(N^2) visit cost is in the microseconds even at N=200 and sits far below the msleep(100) granularity.Applied alone on top of ksmbd-for-next-next, this patch does not introduce a new leak site. Under the same reproducer (10x concurrent-holders + ss -K + ksmbd.control --shutdown + rmmod), the tree still shows the pre-existing per-connection transport leak count that arises when the last refcount drop lands in one of
ksmbd_conn_r_count_dec(),__free_opinfo()orsession_fd_check()- all of which end with a barekfree()today. kmemleak backtraces for the unreferenced objects point into the TCP accept path (sk_clone->inet_csk_clone_lock,sock_alloc_inode) and none involvestop_sessions(). Plugging those bare-kfree sites is the responsibility of the follow-up patch.Fixes e2f34481b24d ("cifsd: add server-side procedures for SMB3")Signed-off-by DaeMyung Kang <[email protected]>Acked-by Namjae Jeon <[email protected]>Signed-off-by Steve French <[email protected]>[ kept list_for_each_entry iteration and schedule_timeout_interruptible(HZ/10) ]Signed-off-by Sasha Levin <[email protected]>--- fs/smb/server/connection.c | 46 +++++++++++++++++++++++++++++++------- fs/smb/server/connection.h | 1 + 2 files changed, 39 insertions(+), 8 deletions(-)fs/smb/server/connection.c +38
-8@@ -478,23 +478,53 @@ int ksmbd_conn_transport_init(void)static void stop_sessions (void ) { -struct ksmbd_conn *conn; +struct ksmbd_conn *conn, *target;struct ksmbd_transport *t; +bool any; +/* +* Serialised via init_lock; no concurrent stop_sessions() can +* touch conn->stop_called, so writing it under the read lock is +* safe. +*/ again: + target =NULL ; + any = false;down_read (&conn_list_lock);list_for_each_entry (conn, &conn_list, conns_list) { - t = conn->transport; -ksmbd_conn_set_exiting (conn); -if (t->ops->shutdown) { -up_read (&conn_list_lock); + any = true; +if (conn->stop_called) +continue ; +atomic_inc (&conn->refcnt); + conn->stop_called = true; +/* +* Mark the connection EXITING while still holding the +* read lock so the selection and the status transition +* happen together. Do not regress a connection that has +* already advanced to RELEASING on its own (e.g. the +* handler exited its receive loop for an unrelated +* reason). +*/ +if (READ_ONCE (conn->status) !=KSMBD_SESS_RELEASING ) +ksmbd_conn_set_exiting (conn); + target = conn; +break ; + } +up_read (&conn_list_lock); + +if (target) { + t = target->transport; +if (t->ops->shutdown) t->ops->shutdown (t); -down_read (&conn_list_lock); +if (atomic_dec_and_test (&target->refcnt)) { +ida_destroy (&target->async_ida); + t->ops->free_transport (t); +kfree (target); } + goto again; } -up_read (&conn_list_lock); -if (!list_empty (&conn_list)) { +if (any) {schedule_timeout_interruptible (HZ /10 );/* 100ms */ goto again; }fs/smb/server/connection.h +1
-0@@ -46,6 +46,7 @@ struct ksmbd_conn {struct mutex srv_mutex;int status;unsigned int cli_cap; +bool stop_called;union {__be32 inet_addr;#if IS_ENABLED (CONFIG_IPV6 )Signature · 1 line
2.53.0
Does not apply :(
Re: [PATCH 6.6.y] ksmbd: rewrite stop_sessions() with restartable iteration
Hi Greg, Sasha
The AUTOSEL backport no longer matches the current 6.6.y queue because the queue already contains the two commits that c444139cb747 is based on in mainline:
f75f8bdd4ff4 ("ksmbd: use msleep instaed of
schedule_timeout_interruptible()")
0bcc831be535 ("ksmbd: replace connection list with hash table")
In the queue I checked, these are queue-6.6/series entries 367 and 368.
With those two patches applied, 6.6.y stop_sessions() matches the upstream preimage of c444139cb747, so no list-based backport adaptation is needed anymore. I verified that git am of the unmodified upstream patch for c444139cb747 on top of v6.6.139 plus those two queued patches applies cleanly.
So the list-adapted AUTOSEL version can be dropped. Please queue upstream commit c444139cb747 for 6.6.y after 0bcc831be535 instead.
Thanks,
DaeMyung
2026년 5월 15일 (금) 오후 7:31, Greg KH <[email protected]>님이 작성:
Quoted 197 lines · Sasha Levin
Quoted 190 lines
From: DaeMyung Kang <[email protected]>
[ Upstream commit
c444139cb747bf6de1922b39900fdf02281490f4]
stop_sessions()walksconn_listwithhash_for_each()and, for every entry, dropsconn_list_lockacross the transport ->shutdown() call before re-acquiring the read lock to continue the loop. The hash walk relies on cross-iteration state (the current bucket and the hlist position), which is not preserved across unlock/relock: if another thread performs a list mutation during the unlocked window, the ongoing iteration becomes unreliable and can re-visit connections that have already been handled or skip connections that have not. The outerif (!hash_empty(conn_list)) goto again;retry masks the symptom in the common case but does not address the unsafe iteration itself.Reframe the loop so it never relies on iterator state across unlock/relock. Under
conn_list_lockheld for read, pick the first connection whose ->shutdown() has not yet been issued by this path, pin it by taking an extra reference, record that fact on the connection and mark it EXITING while still inside the locked walk, then drop the lock. Then call ->shutdown() outside the lock, drop the pin (freeing the connection if the handler already released its reference), and restart from the top.Use a new per-connection flag,
conn->stop_called, as the "shutdown issued fromstop_sessions()" marker rather than reusing the status state.ksmbd_conn_set_exiting()is also invoked byksmbd_sessions_deregister()on sibling channels of a multichannel session without issuing a transport shutdown, so treatingKSMBD_SESS_EXITINGas "already handled here" would skip connections that still needshutdown()to wake their handler out ofrecv(), leaving the outer retry waiting indefinitely for the hash to drain.stop_sessions()is serialised byinit_lockinksmbd_conn_transport_destroy(), so writingstop_calledunder the read lock has no other writer.Set EXITING inside the locked walk so the selection, the
stop_calledmarker, and the status transition all happen together, and guard against regressing a connection that has already advanced toKSMBD_SESS_RELEASINGon its own (for example, if the handler exited its receive loop for an unrelated reason between teardown steps).When the pin drop is the last put, release the transport and pair
ida_destroy(&target->async_ida)with theida_init()done inksmbd_conn_alloc(), sostop_sessions()retiring a connection on its own does not leak the xarray backing of the embeddedasync_ida.The outer retry with
msleep()is kept to wait for handler threads to reachksmbd_conn_free()and drain the hash.Observed with an instrumented build that logs one line per visit and widens the unlocked window before ->shutdown() by 200 ms, under five concurrent cifs mounts (nosharesock, one connection each):
* Current code: the same connection address is revisited many times during a single
stop_sessions()call and ->shutdown() is invoked well beyond the number of live connections before the hash finally drains.* Rewritten code: each live connection produces exactly one
->shutdown() call; the function returns as soon as the hash is empty.Functional teardown via
ksmbd.control --shutdownwith the same five mounts completes cleanly on the rewritten path.Performance is observably unchanged. Tearing down N concurrent nosharesock cifs connections with
ksmbd.control --shutdown+rmmod ksmbdtakes essentially the same wall time before and after the rewrite:N before after 10 4.93s 5.34s 30 7.34s 7.03s 50 7.31s 7.01s (3-run avg: 7.04s vs 7.25s) 100 6.98s 6.78s 200 6.77s 6.89sand the number of ->shutdown() calls equals the number of live connections on both paths when the race is not widened. The teardown is dominated by the msleep(100)-based outer retry waiting for handler threads to run
ksmbd_conn_free(), not by the iteration itself; the restartable loop's worst-case O(N^2) visit cost is in the microseconds even at N=200 and sits far below the msleep(100) granularity.Applied alone on top of ksmbd-for-next-next, this patch does not introduce a new leak site. Under the same reproducer (10x concurrent-holders + ss -K + ksmbd.control --shutdown + rmmod), the tree still shows the pre-existing per-connection transport leak count that arises when the last refcount drop lands in one of
ksmbd_conn_r_count_dec(),__free_opinfo()orsession_fd_check()- all of which end with a barekfree()today. kmemleak backtraces for the unreferenced objects point into the TCP accept path (sk_clone->inet_csk_clone_lock,sock_alloc_inode) and none involvestop_sessions(). Plugging those bare-kfree sites is the responsibility of the follow-up patch.Fixes e2f34481b24d ("cifsd: add server-side procedures for SMB3")Signed-off-by DaeMyung Kang <[email protected]>Acked-by Namjae Jeon <[email protected]>Signed-off-by Steve French <[email protected]>[ kept list_for_each_entry iteration and schedule_timeout_interruptible(HZ/10) ]Signed-off-by Sasha Levin <[email protected]>--- fs/smb/server/connection.c | 46 +++++++++++++++++++++++++++++++------- fs/smb/server/connection.h | 1 + 2 files changed, 39 insertions(+), 8 deletions(-)fs/smb/server/connection.c +38
-8@@ -478,23 +478,53 @@ int ksmbd_conn_transport_init(void)static void stop_sessions (void ) { -struct ksmbd_conn *conn; +struct ksmbd_conn *conn, *target;struct ksmbd_transport *t; +bool any; +/* +* Serialised via init_lock; no concurrent stop_sessions() can +* touch conn->stop_called, so writing it under the read lock is +* safe. +*/ again: + target =NULL ; + any = false;down_read (&conn_list_lock);list_for_each_entry (conn, &conn_list, conns_list) { - t = conn->transport; -ksmbd_conn_set_exiting (conn); -if (t->ops->shutdown) { -up_read (&conn_list_lock); + any = true; +if (conn->stop_called) +continue ; +atomic_inc (&conn->refcnt); + conn->stop_called = true; +/* +* Mark the connection EXITING while still holding the +* read lock so the selection and the status transition +* happen together. Do not regress a connection that has +* already advanced to RELEASING on its own (e.g. the +* handler exited its receive loop for an unrelated +* reason). +*/ +if (READ_ONCE (conn->status) !=KSMBD_SESS_RELEASING ) +ksmbd_conn_set_exiting (conn); + target = conn; +break ; + } +up_read (&conn_list_lock); + +if (target) { + t = target->transport; +if (t->ops->shutdown) t->ops->shutdown (t); -down_read (&conn_list_lock); +if (atomic_dec_and_test (&target->refcnt)) { +ida_destroy (&target->async_ida); + t->ops->free_transport (t); +kfree (target); } + goto again; } -up_read (&conn_list_lock); -if (!list_empty (&conn_list)) { +if (any) {schedule_timeout_interruptible (HZ /10 );/* 100ms */ goto again; }fs/smb/server/connection.h +1
-1@@ -46,6 +46,7 @@ struct ksmbd_conn {struct mutex srv_mutex;int status;unsigned int cli_cap; +bool stop_called;union {__be32 inet_addr;#if IS_ENABLED (CONFIG_IPV6 ) --Quoted 3 lines
2.53.0
Does not apply :(