[PATCH 0/2] ksmbd: pair ida_init() with ida_destroy() in cleanup paths

[PATCH 0/2] ksmbd: pair ida_init() with ida_destroy() in cleanup paths

[DaeMyung Kang]

Two small IDA cleanup fixes.  Both instances share the same history:
when the per-object IDA was converted from a dynamically allocated
ksmbd_ida (which had ksmbd_ida_free() called at destruction) to an
embedded struct ida initialised with ida_init(), the matching
ida_destroy() was not added to the teardown path.  The enclosing
object is freed with the IDA's backing xarray still intact.

  1/2  ksmbd_session_destroy() frees the session without destroying
       sess->tree_conn_ida.  This patch also moves ida_init() to
       right after the session allocation so that the init/destroy
       pairing holds on the early error paths of __session_create()
       as well.

  2/2  ksmbd_conn_free() frees the connection without destroying
       conn->async_ida.  ksmbd_conn_alloc() has no failure path
       after ida_init(), so no init-site move is required.  The
       destroy is placed inside the final refcount branch (next to
       kfree(conn)) rather than with the unconditional field teardown
       because async_ida is embedded in struct ksmbd_conn and its
       storage must stay valid while other refcount holders
       (oplock / vfs durable handles) still reference the struct.

No leak has been observed in testing; both are pairing fixes to match
IDA lifetime rules, not responses to reproduced regressions.  Cc:
stable is intentionally omitted for the same reason.

Tested on top of current linux-next inside virtme-ng with
CONFIG_PROVE_LOCKING, CONFIG_DEBUG_OBJECTS, CONFIG_DEBUG_KMEMLEAK,
CONFIG_DEBUG_ATOMIC_SLEEP and CONFIG_FAILSLAB enabled.  Exercises:

 * 25x mount / umount via loopback cifs client with 3% failslab
   injection to cover ksmbd_init_file_table() and
   __init_smb2_session() failures on the __session_create() error
   path (which now reaches ksmbd_session_destroy() with an
   already-initialised tree_conn_ida).
 * Concurrent 32 MiB SMB2 reads followed by ksmbd.control
   --shutdown to drive async request teardown.
 * rmmod ksmbd afterwards.

No splats (BUG:, WARNING:, UBSAN, ODEBUG, task hung, inconsistent
lock, suspicious RCU) and no kmemleak unreferenced objects.

DaeMyung Kang (2):
  ksmbd: destroy tree_conn_ida in ksmbd_session_destroy()
  ksmbd: destroy async_ida in ksmbd_conn_free()

 fs/smb/server/connection.c        | 9 +++++++++
 fs/smb/server/mgmt/user_session.c | 5 +++--
 2 files changed, 12 insertions(+), 2 deletions(-)

--
2.43.0

[PATCH 1/2] ksmbd: destroy tree_conn_ida in ksmbd_session_destroy()

[DaeMyung Kang]

When per-session tree_conn_ida was converted from a dynamically
allocated ksmbd_ida to an embedded struct ida, ksmbd_ida_free() was
removed from ksmbd_session_destroy() but no matching ida_destroy()
was added.  The session is therefore freed with the IDA's backing
xarray still intact.

The kernel IDA API expects ida_init() and ida_destroy() to be paired
over an object's lifetime, so add the missing cleanup before the
enclosing session is freed.

Also move ida_init() to right after the session is allocated so that
it is always paired with the destroy call even on the early error
paths of __session_create() (ksmbd_init_file_table() or
__init_smb2_session() failures), both of which jump to the error
label and invoke ksmbd_session_destroy() on a partially initialised
session.

No leak has been observed in testing; this is a pairing fix to match
the IDA lifetime rules, not a response to a reproduced regression.

Fixes: d40012a83f87 ("cifsd: declare ida statically")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/smb/server/mgmt/user_session.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/smb/server/mgmt/user_session.c b/fs/smb/server/mgmt/user_session.c
index a86589408835..0dd9e6c976ac 100644
--- a/fs/smb/server/mgmt/user_session.c
+++ b/fs/smb/server/mgmt/user_session.c
@@ -391,6 +391,7 @@ void ksmbd_session_destroy(struct ksmbd_session *sess)
 	free_channel_list(sess);
 	kfree(sess->Preauth_HashValue);
 	ksmbd_release_id(&session_ida, sess->id);
+	ida_destroy(&sess->tree_conn_ida);
 	kfree(sess);
 }
 
@@ -665,6 +666,8 @@ static struct ksmbd_session *__session_create(int protocol)
 	if (!sess)
 		return NULL;
 
+	ida_init(&sess->tree_conn_ida);
+
 	if (ksmbd_init_file_table(&sess->file_table))
 		goto error;
 
@@ -684,8 +687,6 @@ static struct ksmbd_session *__session_create(int protocol)
 	if (ret)
 		goto error;
 
-	ida_init(&sess->tree_conn_ida);
-
 	down_write(&sessions_table_lock);
 	hash_add(sessions_table, &sess->hlist, sess->id);
 	up_write(&sessions_table_lock);
-- 
2.43.0

[PATCH 2/2] ksmbd: destroy async_ida in ksmbd_conn_free()

[DaeMyung Kang]

When per-connection async_ida was converted from a dynamically
allocated ksmbd_ida to an embedded struct ida, ksmbd_ida_free() was
removed from the connection teardown path but no matching
ida_destroy() was added.  The connection is therefore freed with the
IDA's backing xarray still intact.

The kernel IDA API expects ida_init() and ida_destroy() to be paired
over an object's lifetime, so add the missing cleanup before the
connection is freed.

No leak has been observed in testing; this is a pairing fix to match
the IDA lifetime rules, not a response to a reproduced regression.

Fixes: d40012a83f87 ("cifsd: declare ida statically")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
---
 fs/smb/server/connection.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/smb/server/connection.c b/fs/smb/server/connection.c
index a4110c6cce37..8bbfe27387e3 100644
--- a/fs/smb/server/connection.c
+++ b/fs/smb/server/connection.c
@@ -98,6 +98,15 @@ void ksmbd_conn_free(struct ksmbd_conn *conn)
 	kfree(conn->preauth_info);
 	kfree(conn->mechToken);
 	if (atomic_dec_and_test(&conn->refcnt)) {
+		/*
+		 * async_ida is embedded in struct ksmbd_conn, so pair
+		 * ida_destroy() with the final kfree() rather than with
+		 * the unconditional field teardown above.  This keeps
+		 * the IDA valid for the entire lifetime of the struct,
+		 * even while other refcount holders (oplock / vfs
+		 * durable handles) still reference the connection.
+		 */
+		ida_destroy(&conn->async_ida);
 		conn->transport->ops->free_transport(conn->transport);
 		kfree(conn);
 	}
-- 
2.43.0

Re: [PATCH 1/2] ksmbd: destroy tree_conn_ida in ksmbd_session_destroy()

[Namjae Jeon]

On Sun, Apr 19, 2026 at 8:03 PM DaeMyung Kang <charsyam@gmail.com> wrote:
>
> When per-session tree_conn_ida was converted from a dynamically
> allocated ksmbd_ida to an embedded struct ida, ksmbd_ida_free() was
> removed from ksmbd_session_destroy() but no matching ida_destroy()
> was added.  The session is therefore freed with the IDA's backing
> xarray still intact.
>
> The kernel IDA API expects ida_init() and ida_destroy() to be paired
> over an object's lifetime, so add the missing cleanup before the
> enclosing session is freed.
>
> Also move ida_init() to right after the session is allocated so that
> it is always paired with the destroy call even on the early error
> paths of __session_create() (ksmbd_init_file_table() or
> __init_smb2_session() failures), both of which jump to the error
> label and invoke ksmbd_session_destroy() on a partially initialised
> session.
>
> No leak has been observed in testing; this is a pairing fix to match
> the IDA lifetime rules, not a response to a reproduced regression.
>
> Fixes: d40012a83f87 ("cifsd: declare ida statically")
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
Applied it to #ksmbd-for-next-next.
Thanks!

Re: [PATCH 2/2] ksmbd: destroy async_ida in ksmbd_conn_free()

[Namjae Jeon]

On Sun, Apr 19, 2026 at 8:03 PM DaeMyung Kang <charsyam@gmail.com> wrote:
>
> When per-connection async_ida was converted from a dynamically
> allocated ksmbd_ida to an embedded struct ida, ksmbd_ida_free() was
> removed from the connection teardown path but no matching
> ida_destroy() was added.  The connection is therefore freed with the
> IDA's backing xarray still intact.
>
> The kernel IDA API expects ida_init() and ida_destroy() to be paired
> over an object's lifetime, so add the missing cleanup before the
> connection is freed.
>
> No leak has been observed in testing; this is a pairing fix to match
> the IDA lifetime rules, not a response to a reproduced regression.
>
> Fixes: d40012a83f87 ("cifsd: declare ida statically")
> Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
Applied it to #ksmbd-for-next-next.
Thanks!