[PATCH 1/5] ksmbd: replace sessions list in connection with xarray

[PATCH 1/5] ksmbd: replace sessions list in connection with xarray

[Namjae Jeon]

Replace sessions list in connection with xarray.

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/connection.c        |  3 ++-
 fs/ksmbd/connection.h        |  2 +-
 fs/ksmbd/mgmt/user_session.c | 31 +++++++------------------------
 fs/ksmbd/mgmt/user_session.h |  5 ++---
 fs/ksmbd/smb2pdu.c           | 13 +++++++++----
 5 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
index e8f476c5f189..ce23cc89046e 100644
--- a/fs/ksmbd/connection.c
+++ b/fs/ksmbd/connection.c
@@ -36,6 +36,7 @@ void ksmbd_conn_free(struct ksmbd_conn *conn)
 	list_del(&conn->conns_list);
 	write_unlock(&conn_list_lock);
 
+	xa_destroy(&conn->sessions);
 	kvfree(conn->request_buf);
 	kfree(conn->preauth_info);
 	kfree(conn);
@@ -66,12 +67,12 @@ struct ksmbd_conn *ksmbd_conn_alloc(void)
 
 	init_waitqueue_head(&conn->req_running_q);
 	INIT_LIST_HEAD(&conn->conns_list);
-	INIT_LIST_HEAD(&conn->sessions);
 	INIT_LIST_HEAD(&conn->requests);
 	INIT_LIST_HEAD(&conn->async_requests);
 	spin_lock_init(&conn->request_lock);
 	spin_lock_init(&conn->credits_lock);
 	ida_init(&conn->async_ida);
+	xa_init(&conn->sessions);
 
 	spin_lock_init(&conn->llist_lock);
 	INIT_LIST_HEAD(&conn->lock_list);
diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h
index 98c1cbe45ec9..5b39f0bdeff8 100644
--- a/fs/ksmbd/connection.h
+++ b/fs/ksmbd/connection.h
@@ -55,7 +55,7 @@ struct ksmbd_conn {
 	struct nls_table		*local_nls;
 	struct list_head		conns_list;
 	/* smb session 1 per user */
-	struct list_head		sessions;
+	struct xarray			sessions;
 	unsigned long			last_active;
 	/* How many request are running currently */
 	atomic_t			req_running;
diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c
index 8d8ffd8c6f19..3a44e66456fc 100644
--- a/fs/ksmbd/mgmt/user_session.c
+++ b/fs/ksmbd/mgmt/user_session.c
@@ -152,8 +152,6 @@ void ksmbd_session_destroy(struct ksmbd_session *sess)
 	if (!atomic_dec_and_test(&sess->refcnt))
 		return;
 
-	list_del(&sess->sessions_entry);
-
 	down_write(&sessions_table_lock);
 	hash_del(&sess->hlist);
 	up_write(&sessions_table_lock);
@@ -181,42 +179,28 @@ static struct ksmbd_session *__session_lookup(unsigned long long id)
 	return NULL;
 }
 
-void ksmbd_session_register(struct ksmbd_conn *conn,
-			    struct ksmbd_session *sess)
+int ksmbd_session_register(struct ksmbd_conn *conn,
+			   struct ksmbd_session *sess)
 {
 	sess->conn = conn;
-	list_add(&sess->sessions_entry, &conn->sessions);
+	return xa_err(xa_store(&conn->sessions, sess->id, sess, GFP_KERNEL));
 }
 
 void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
 {
 	struct ksmbd_session *sess;
+	unsigned long id;
 
-	while (!list_empty(&conn->sessions)) {
-		sess = list_entry(conn->sessions.next,
-				  struct ksmbd_session,
-				  sessions_entry);
-
+	xa_for_each(&conn->sessions, id, sess) {
+		xa_erase(&conn->sessions, sess->id);
 		ksmbd_session_destroy(sess);
 	}
 }
 
-static bool ksmbd_session_id_match(struct ksmbd_session *sess,
-				   unsigned long long id)
-{
-	return sess->id == id;
-}
-
 struct ksmbd_session *ksmbd_session_lookup(struct ksmbd_conn *conn,
 					   unsigned long long id)
 {
-	struct ksmbd_session *sess = NULL;
-
-	list_for_each_entry(sess, &conn->sessions, sessions_entry) {
-		if (ksmbd_session_id_match(sess, id))
-			return sess;
-	}
-	return NULL;
+	return xa_load(&conn->sessions, id);
 }
 
 int get_session(struct ksmbd_session *sess)
@@ -314,7 +298,6 @@ static struct ksmbd_session *__session_create(int protocol)
 		goto error;
 
 	set_session_flag(sess, protocol);
-	INIT_LIST_HEAD(&sess->sessions_entry);
 	xa_init(&sess->tree_conns);
 	INIT_LIST_HEAD(&sess->ksmbd_chann_list);
 	INIT_LIST_HEAD(&sess->rpc_handle_list);
diff --git a/fs/ksmbd/mgmt/user_session.h b/fs/ksmbd/mgmt/user_session.h
index e241f16a3851..8b08189be3fc 100644
--- a/fs/ksmbd/mgmt/user_session.h
+++ b/fs/ksmbd/mgmt/user_session.h
@@ -57,7 +57,6 @@ struct ksmbd_session {
 	__u8				smb3decryptionkey[SMB3_ENC_DEC_KEY_SIZE];
 	__u8				smb3signingkey[SMB3_SIGN_KEY_SIZE];
 
-	struct list_head		sessions_entry;
 	struct ksmbd_file_table		file_table;
 	atomic_t			refcnt;
 };
@@ -84,8 +83,8 @@ void ksmbd_session_destroy(struct ksmbd_session *sess);
 struct ksmbd_session *ksmbd_session_lookup_slowpath(unsigned long long id);
 struct ksmbd_session *ksmbd_session_lookup(struct ksmbd_conn *conn,
 					   unsigned long long id);
-void ksmbd_session_register(struct ksmbd_conn *conn,
-			    struct ksmbd_session *sess);
+int ksmbd_session_register(struct ksmbd_conn *conn,
+			   struct ksmbd_session *sess);
 void ksmbd_sessions_deregister(struct ksmbd_conn *conn);
 struct ksmbd_session *ksmbd_session_lookup_all(struct ksmbd_conn *conn,
 					       unsigned long long id);
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 94ab1dcd80e7..04d20a2e6dee 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -588,7 +588,8 @@ int smb2_check_user_session(struct ksmbd_work *work)
 	return -EINVAL;
 }
 
-static void destroy_previous_session(struct ksmbd_user *user, u64 id)
+static void destroy_previous_session(struct ksmbd_conn *conn,
+				     struct ksmbd_user *user, u64 id)
 {
 	struct ksmbd_session *prev_sess = ksmbd_session_lookup_slowpath(id);
 	struct ksmbd_user *prev_user;
@@ -607,6 +608,7 @@ static void destroy_previous_session(struct ksmbd_user *user, u64 id)
 	}
 
 	put_session(prev_sess);
+	xa_erase(&conn->sessions, prev_sess->id);
 	ksmbd_session_destroy(prev_sess);
 }
 
@@ -1439,7 +1441,7 @@ static int ntlm_authenticate(struct ksmbd_work *work)
 	/* Check for previous session */
 	prev_id = le64_to_cpu(req->PreviousSessionId);
 	if (prev_id && prev_id != sess->id)
-		destroy_previous_session(user, prev_id);
+		destroy_previous_session(conn, user, prev_id);
 
 	if (sess->state == SMB2_SESSION_VALID) {
 		/*
@@ -1561,7 +1563,7 @@ static int krb5_authenticate(struct ksmbd_work *work)
 	/* Check previous session */
 	prev_sess_id = le64_to_cpu(req->PreviousSessionId);
 	if (prev_sess_id && prev_sess_id != sess->id)
-		destroy_previous_session(sess->user, prev_sess_id);
+		destroy_previous_session(conn, sess->user, prev_sess_id);
 
 	if (sess->state == SMB2_SESSION_VALID)
 		ksmbd_free_user(sess->user);
@@ -1650,7 +1652,9 @@ int smb2_sess_setup(struct ksmbd_work *work)
 			goto out_err;
 		}
 		rsp->hdr.SessionId = cpu_to_le64(sess->id);
-		ksmbd_session_register(conn, sess);
+		rc = ksmbd_session_register(conn, sess);
+		if (rc)
+			goto out_err;
 	} else if (conn->dialect >= SMB30_PROT_ID &&
 		   (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB3_MULTICHANNEL) &&
 		   req->Flags & SMB2_SESSION_REQ_FLAG_BINDING) {
@@ -1828,6 +1832,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
 			if (sess->user && sess->user->flags & KSMBD_USER_FLAG_DELAY_SESSION)
 				try_delay = true;
 
+			xa_erase(&conn->sessions, sess->id);
 			ksmbd_session_destroy(sess);
 			work->sess = NULL;
 			if (try_delay)
-- 
2.25.1

[PATCH 2/5] ksmbd: add channel rwlock

[Namjae Jeon]

Add missing rwlock for channel list in session.

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/mgmt/user_session.c |  3 +++
 fs/ksmbd/mgmt/user_session.h |  1 +
 fs/ksmbd/smb2pdu.c           | 20 ++++++++++++++++++--
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c
index 3a44e66456fc..25e9ba3b7550 100644
--- a/fs/ksmbd/mgmt/user_session.c
+++ b/fs/ksmbd/mgmt/user_session.c
@@ -32,11 +32,13 @@ static void free_channel_list(struct ksmbd_session *sess)
 {
 	struct channel *chann, *tmp;
 
+	write_lock(&sess->chann_lock);
 	list_for_each_entry_safe(chann, tmp, &sess->ksmbd_chann_list,
 				 chann_list) {
 		list_del(&chann->chann_list);
 		kfree(chann);
 	}
+	write_unlock(&sess->chann_lock);
 }
 
 static void __session_rpc_close(struct ksmbd_session *sess,
@@ -303,6 +305,7 @@ static struct ksmbd_session *__session_create(int protocol)
 	INIT_LIST_HEAD(&sess->rpc_handle_list);
 	sess->sequence_number = 1;
 	atomic_set(&sess->refcnt, 1);
+	rwlock_init(&sess->chann_lock);
 
 	switch (protocol) {
 	case CIFDS_SESSION_FLAG_SMB2:
diff --git a/fs/ksmbd/mgmt/user_session.h b/fs/ksmbd/mgmt/user_session.h
index 8b08189be3fc..1ec659f0151b 100644
--- a/fs/ksmbd/mgmt/user_session.h
+++ b/fs/ksmbd/mgmt/user_session.h
@@ -48,6 +48,7 @@ struct ksmbd_session {
 	char				sess_key[CIFS_KEY_SIZE];
 
 	struct hlist_node		hlist;
+	rwlock_t			chann_lock;
 	struct list_head		ksmbd_chann_list;
 	struct xarray			tree_conns;
 	struct ida			tree_conn_ida;
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 04d20a2e6dee..5a0328a070dc 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -1512,7 +1512,9 @@ static int ntlm_authenticate(struct ksmbd_work *work)
 
 binding_session:
 	if (conn->dialect >= SMB30_PROT_ID) {
+		read_lock(&sess->chann_lock);
 		chann = lookup_chann_list(sess, conn);
+		read_unlock(&sess->chann_lock);
 		if (!chann) {
 			chann = kmalloc(sizeof(struct channel), GFP_KERNEL);
 			if (!chann)
@@ -1520,7 +1522,9 @@ static int ntlm_authenticate(struct ksmbd_work *work)
 
 			chann->conn = conn;
 			INIT_LIST_HEAD(&chann->chann_list);
+			write_lock(&sess->chann_lock);
 			list_add(&chann->chann_list, &sess->ksmbd_chann_list);
+			write_unlock(&sess->chann_lock);
 		}
 	}
 
@@ -1594,7 +1598,9 @@ static int krb5_authenticate(struct ksmbd_work *work)
 	}
 
 	if (conn->dialect >= SMB30_PROT_ID) {
+		read_lock(&sess->chann_lock);
 		chann = lookup_chann_list(sess, conn);
+		read_unlock(&sess->chann_lock);
 		if (!chann) {
 			chann = kmalloc(sizeof(struct channel), GFP_KERNEL);
 			if (!chann)
@@ -1602,7 +1608,9 @@ static int krb5_authenticate(struct ksmbd_work *work)
 
 			chann->conn = conn;
 			INIT_LIST_HEAD(&chann->chann_list);
+			write_lock(&sess->chann_lock);
 			list_add(&chann->chann_list, &sess->ksmbd_chann_list);
+			write_unlock(&sess->chann_lock);
 		}
 	}
 
@@ -8351,10 +8359,14 @@ int smb3_check_sign_req(struct ksmbd_work *work)
 	if (le16_to_cpu(hdr->Command) == SMB2_SESSION_SETUP_HE) {
 		signing_key = work->sess->smb3signingkey;
 	} else {
+		read_lock(&work->sess->chann_lock);
 		chann = lookup_chann_list(work->sess, conn);
-		if (!chann)
+		if (!chann) {
+			read_unlock(&work->sess->chann_lock);
 			return 0;
+		}
 		signing_key = chann->smb3signingkey;
+		read_unlock(&work->sess->chann_lock);
 	}
 
 	if (!signing_key) {
@@ -8414,10 +8426,14 @@ void smb3_set_sign_rsp(struct ksmbd_work *work)
 	    le16_to_cpu(hdr->Command) == SMB2_SESSION_SETUP_HE) {
 		signing_key = work->sess->smb3signingkey;
 	} else {
+		read_lock(&work->sess->chann_lock);
 		chann = lookup_chann_list(work->sess, work->conn);
-		if (!chann)
+		if (!chann) {
+			read_unlock(&work->sess->chann_lock);
 			return;
+		}
 		signing_key = chann->smb3signingkey;
+		read_unlock(&work->sess->chann_lock);
 	}
 
 	if (!signing_key)
-- 
2.25.1

Re: [PATCH 2/5] ksmbd: add channel rwlock

[Hyunchul Lee]

2022년 7월 22일 (금) 오후 12:04, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>
> Add missing rwlock for channel list in session.
>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>

Looks good to me.
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

> ---
>  fs/ksmbd/mgmt/user_session.c |  3 +++
>  fs/ksmbd/mgmt/user_session.h |  1 +
>  fs/ksmbd/smb2pdu.c           | 20 ++++++++++++++++++--
>  3 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c
> index 3a44e66456fc..25e9ba3b7550 100644
> --- a/fs/ksmbd/mgmt/user_session.c
> +++ b/fs/ksmbd/mgmt/user_session.c
> @@ -32,11 +32,13 @@ static void free_channel_list(struct ksmbd_session *sess)
>  {
>         struct channel *chann, *tmp;
>
> +       write_lock(&sess->chann_lock);
>         list_for_each_entry_safe(chann, tmp, &sess->ksmbd_chann_list,
>                                  chann_list) {
>                 list_del(&chann->chann_list);
>                 kfree(chann);
>         }
> +       write_unlock(&sess->chann_lock);
>  }
>
>  static void __session_rpc_close(struct ksmbd_session *sess,
> @@ -303,6 +305,7 @@ static struct ksmbd_session *__session_create(int protocol)
>         INIT_LIST_HEAD(&sess->rpc_handle_list);
>         sess->sequence_number = 1;
>         atomic_set(&sess->refcnt, 1);
> +       rwlock_init(&sess->chann_lock);
>
>         switch (protocol) {
>         case CIFDS_SESSION_FLAG_SMB2:
> diff --git a/fs/ksmbd/mgmt/user_session.h b/fs/ksmbd/mgmt/user_session.h
> index 8b08189be3fc..1ec659f0151b 100644
> --- a/fs/ksmbd/mgmt/user_session.h
> +++ b/fs/ksmbd/mgmt/user_session.h
> @@ -48,6 +48,7 @@ struct ksmbd_session {
>         char                            sess_key[CIFS_KEY_SIZE];
>
>         struct hlist_node               hlist;
> +       rwlock_t                        chann_lock;
>         struct list_head                ksmbd_chann_list;
>         struct xarray                   tree_conns;
>         struct ida                      tree_conn_ida;
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index 04d20a2e6dee..5a0328a070dc 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -1512,7 +1512,9 @@ static int ntlm_authenticate(struct ksmbd_work *work)
>
>  binding_session:
>         if (conn->dialect >= SMB30_PROT_ID) {
> +               read_lock(&sess->chann_lock);
>                 chann = lookup_chann_list(sess, conn);
> +               read_unlock(&sess->chann_lock);
>                 if (!chann) {
>                         chann = kmalloc(sizeof(struct channel), GFP_KERNEL);
>                         if (!chann)
> @@ -1520,7 +1522,9 @@ static int ntlm_authenticate(struct ksmbd_work *work)
>
>                         chann->conn = conn;
>                         INIT_LIST_HEAD(&chann->chann_list);
> +                       write_lock(&sess->chann_lock);
>                         list_add(&chann->chann_list, &sess->ksmbd_chann_list);
> +                       write_unlock(&sess->chann_lock);
>                 }
>         }
>
> @@ -1594,7 +1598,9 @@ static int krb5_authenticate(struct ksmbd_work *work)
>         }
>
>         if (conn->dialect >= SMB30_PROT_ID) {
> +               read_lock(&sess->chann_lock);
>                 chann = lookup_chann_list(sess, conn);
> +               read_unlock(&sess->chann_lock);
>                 if (!chann) {
>                         chann = kmalloc(sizeof(struct channel), GFP_KERNEL);
>                         if (!chann)
> @@ -1602,7 +1608,9 @@ static int krb5_authenticate(struct ksmbd_work *work)
>
>                         chann->conn = conn;
>                         INIT_LIST_HEAD(&chann->chann_list);
> +                       write_lock(&sess->chann_lock);
>                         list_add(&chann->chann_list, &sess->ksmbd_chann_list);
> +                       write_unlock(&sess->chann_lock);
>                 }
>         }
>
> @@ -8351,10 +8359,14 @@ int smb3_check_sign_req(struct ksmbd_work *work)
>         if (le16_to_cpu(hdr->Command) == SMB2_SESSION_SETUP_HE) {
>                 signing_key = work->sess->smb3signingkey;
>         } else {
> +               read_lock(&work->sess->chann_lock);
>                 chann = lookup_chann_list(work->sess, conn);
> -               if (!chann)
> +               if (!chann) {
> +                       read_unlock(&work->sess->chann_lock);
>                         return 0;
> +               }
>                 signing_key = chann->smb3signingkey;
> +               read_unlock(&work->sess->chann_lock);
>         }
>
>         if (!signing_key) {
> @@ -8414,10 +8426,14 @@ void smb3_set_sign_rsp(struct ksmbd_work *work)
>             le16_to_cpu(hdr->Command) == SMB2_SESSION_SETUP_HE) {
>                 signing_key = work->sess->smb3signingkey;
>         } else {
> +               read_lock(&work->sess->chann_lock);
>                 chann = lookup_chann_list(work->sess, work->conn);
> -               if (!chann)
> +               if (!chann) {
> +                       read_unlock(&work->sess->chann_lock);
>                         return;
> +               }
>                 signing_key = chann->smb3signingkey;
> +               read_unlock(&work->sess->chann_lock);
>         }
>
>         if (!signing_key)
> --
> 2.25.1
>


-- 
Thanks,
Hyunchul

[PATCH 3/5] ksmbd: fix kernel oops from idr_remove()

[Namjae Jeon]

There is a report that kernel oops happen from idr_remove().

kernel: BUG: kernel NULL pointer dereference, address: 0000000000000010
kernel: RIP: 0010:idr_remove+0x1/0x20
kernel:  __ksmbd_close_fd+0xb2/0x2d0 [ksmbd]
kernel:  ksmbd_vfs_read+0x91/0x190 [ksmbd]
kernel:  ksmbd_fd_put+0x29/0x40 [ksmbd]
kernel:  smb2_read+0x210/0x390 [ksmbd]
kernel:  __process_request+0xa4/0x180 [ksmbd]
kernel:  __handle_ksmbd_work+0xf0/0x290 [ksmbd]
kernel:  handle_ksmbd_work+0x2d/0x50 [ksmbd]
kernel:  process_one_work+0x21d/0x3f0
kernel:  worker_thread+0x50/0x3d0
kernel:  rescuer_thread+0x390/0x390
kernel:  kthread+0xee/0x120
kthread_complete_and_exit+0x20/0x20
kernel:  ret_from_fork+0x22/0x30

While accessing files, If connection is disconnected, windows send
session setup request included previous session destroy. But while still
processing requests on previous session, this request destroy file
table, which mean file table idr will be freed and set to NULL.
So kernel oops happen from ft->idr in __ksmbd_close_fd().
This patch don't directly destroy session in destroy_previous_session().
It just set to KSMBD_SESS_EXITING so that connection will be
terminated after finishing the rest of requests.

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/mgmt/user_session.c | 2 ++
 fs/ksmbd/smb2pdu.c           | 8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c
index 25e9ba3b7550..b9acb6770b03 100644
--- a/fs/ksmbd/mgmt/user_session.c
+++ b/fs/ksmbd/mgmt/user_session.c
@@ -239,6 +239,8 @@ struct ksmbd_session *ksmbd_session_lookup_all(struct ksmbd_conn *conn,
 	sess = ksmbd_session_lookup(conn, id);
 	if (!sess && conn->binding)
 		sess = ksmbd_session_lookup_slowpath(id);
+	if (sess && sess->state != SMB2_SESSION_VALID)
+		sess = NULL;
 	return sess;
 }
 
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 5a0328a070dc..ae5677a66cb2 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -593,6 +593,7 @@ static void destroy_previous_session(struct ksmbd_conn *conn,
 {
 	struct ksmbd_session *prev_sess = ksmbd_session_lookup_slowpath(id);
 	struct ksmbd_user *prev_user;
+	struct channel *chann;
 
 	if (!prev_sess)
 		return;
@@ -608,8 +609,11 @@ static void destroy_previous_session(struct ksmbd_conn *conn,
 	}
 
 	put_session(prev_sess);
-	xa_erase(&conn->sessions, prev_sess->id);
-	ksmbd_session_destroy(prev_sess);
+	prev_sess->state = SMB2_SESSION_EXPIRED;
+	write_lock(&prev_sess->chann_lock);
+	list_for_each_entry(chann, &prev_sess->ksmbd_chann_list, chann_list)
+		chann->conn->status = KSMBD_SESS_EXITING;
+	write_unlock(&prev_sess->chann_lock);
 }
 
 /**
-- 
2.25.1

Re: [PATCH 3/5] ksmbd: fix kernel oops from idr_remove()

[Hyunchul Lee]

2022년 7월 22일 (금) 오후 12:04, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>
> There is a report that kernel oops happen from idr_remove().
>
> kernel: BUG: kernel NULL pointer dereference, address: 0000000000000010
> kernel: RIP: 0010:idr_remove+0x1/0x20
> kernel:  __ksmbd_close_fd+0xb2/0x2d0 [ksmbd]
> kernel:  ksmbd_vfs_read+0x91/0x190 [ksmbd]
> kernel:  ksmbd_fd_put+0x29/0x40 [ksmbd]
> kernel:  smb2_read+0x210/0x390 [ksmbd]
> kernel:  __process_request+0xa4/0x180 [ksmbd]
> kernel:  __handle_ksmbd_work+0xf0/0x290 [ksmbd]
> kernel:  handle_ksmbd_work+0x2d/0x50 [ksmbd]
> kernel:  process_one_work+0x21d/0x3f0
> kernel:  worker_thread+0x50/0x3d0
> kernel:  rescuer_thread+0x390/0x390
> kernel:  kthread+0xee/0x120
> kthread_complete_and_exit+0x20/0x20
> kernel:  ret_from_fork+0x22/0x30
>
> While accessing files, If connection is disconnected, windows send
> session setup request included previous session destroy. But while still
> processing requests on previous session, this request destroy file
> table, which mean file table idr will be freed and set to NULL.
> So kernel oops happen from ft->idr in __ksmbd_close_fd().
> This patch don't directly destroy session in destroy_previous_session().
> It just set to KSMBD_SESS_EXITING so that connection will be
> terminated after finishing the rest of requests.
>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>

Looks good to me.
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

> ---
>  fs/ksmbd/mgmt/user_session.c | 2 ++
>  fs/ksmbd/smb2pdu.c           | 8 ++++++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c
> index 25e9ba3b7550..b9acb6770b03 100644
> --- a/fs/ksmbd/mgmt/user_session.c
> +++ b/fs/ksmbd/mgmt/user_session.c
> @@ -239,6 +239,8 @@ struct ksmbd_session *ksmbd_session_lookup_all(struct ksmbd_conn *conn,
>         sess = ksmbd_session_lookup(conn, id);
>         if (!sess && conn->binding)
>                 sess = ksmbd_session_lookup_slowpath(id);
> +       if (sess && sess->state != SMB2_SESSION_VALID)
> +               sess = NULL;
>         return sess;
>  }
>
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index 5a0328a070dc..ae5677a66cb2 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -593,6 +593,7 @@ static void destroy_previous_session(struct ksmbd_conn *conn,
>  {
>         struct ksmbd_session *prev_sess = ksmbd_session_lookup_slowpath(id);
>         struct ksmbd_user *prev_user;
> +       struct channel *chann;
>
>         if (!prev_sess)
>                 return;
> @@ -608,8 +609,11 @@ static void destroy_previous_session(struct ksmbd_conn *conn,
>         }
>
>         put_session(prev_sess);
> -       xa_erase(&conn->sessions, prev_sess->id);
> -       ksmbd_session_destroy(prev_sess);
> +       prev_sess->state = SMB2_SESSION_EXPIRED;
> +       write_lock(&prev_sess->chann_lock);
> +       list_for_each_entry(chann, &prev_sess->ksmbd_chann_list, chann_list)
> +               chann->conn->status = KSMBD_SESS_EXITING;
> +       write_unlock(&prev_sess->chann_lock);
>  }
>
>  /**
> --
> 2.25.1
>


-- 
Thanks,
Hyunchul

[PATCH 4/5] ksmbd: use wait_event instead of schedule_timeout()

[Namjae Jeon]

ksmbd threads eating masses of cputime when connection is disconnected.
If connection is disconnected, ksmbd thread waits for pending requests
to be processed using schedule_timeout. schedule_timeout() incorrectly
is used, and it is more efficient to use wait_event/wake_up than to check
r_count every time with timeout.

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/connection.c |  6 +++---
 fs/ksmbd/connection.h |  1 +
 fs/ksmbd/oplock.c     | 21 ++++++++++-----------
 fs/ksmbd/server.c     |  1 +
 4 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
index ce23cc89046e..756ad631c019 100644
--- a/fs/ksmbd/connection.c
+++ b/fs/ksmbd/connection.c
@@ -66,6 +66,7 @@ struct ksmbd_conn *ksmbd_conn_alloc(void)
 	conn->outstanding_credits = 0;
 
 	init_waitqueue_head(&conn->req_running_q);
+	init_waitqueue_head(&conn->r_count_q);
 	INIT_LIST_HEAD(&conn->conns_list);
 	INIT_LIST_HEAD(&conn->requests);
 	INIT_LIST_HEAD(&conn->async_requests);
@@ -165,7 +166,6 @@ int ksmbd_conn_write(struct ksmbd_work *work)
 	struct kvec iov[3];
 	int iov_idx = 0;
 
-	ksmbd_conn_try_dequeue_request(work);
 	if (!work->response_buf) {
 		pr_err("NULL response header\n");
 		return -EINVAL;
@@ -347,8 +347,8 @@ int ksmbd_conn_handler_loop(void *p)
 
 out:
 	/* Wait till all reference dropped to the Server object*/
-	while (atomic_read(&conn->r_count) > 0)
-		schedule_timeout(HZ);
+	wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);
+
 
 	unload_nls(conn->local_nls);
 	if (default_conn_ops.terminate_fn)
diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h
index 5b39f0bdeff8..2e4730457c92 100644
--- a/fs/ksmbd/connection.h
+++ b/fs/ksmbd/connection.h
@@ -65,6 +65,7 @@ struct ksmbd_conn {
 	unsigned int			outstanding_credits;
 	spinlock_t			credits_lock;
 	wait_queue_head_t		req_running_q;
+	wait_queue_head_t		r_count_q;
 	/* Lock to protect requests list*/
 	spinlock_t			request_lock;
 	struct list_head		requests;
diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
index 8b5560574d4c..9bb4fb8b80de 100644
--- a/fs/ksmbd/oplock.c
+++ b/fs/ksmbd/oplock.c
@@ -615,18 +615,13 @@ static void __smb2_oplock_break_noti(struct work_struct *wk)
 	struct ksmbd_file *fp;
 
 	fp = ksmbd_lookup_durable_fd(br_info->fid);
-	if (!fp) {
-		atomic_dec(&conn->r_count);
-		ksmbd_free_work_struct(work);
-		return;
-	}
+	if (!fp)
+		goto out;
 
 	if (allocate_oplock_break_buf(work)) {
 		pr_err("smb2_allocate_rsp_buf failed! ");
-		atomic_dec(&conn->r_count);
 		ksmbd_fd_put(work, fp);
-		ksmbd_free_work_struct(work);
-		return;
+		goto out;
 	}
 
 	rsp_hdr = smb2_get_msg(work->response_buf);
@@ -667,8 +662,11 @@ static void __smb2_oplock_break_noti(struct work_struct *wk)
 
 	ksmbd_fd_put(work, fp);
 	ksmbd_conn_write(work);
+
+out:
 	ksmbd_free_work_struct(work);
 	atomic_dec(&conn->r_count);
+	wake_up_all(&conn->r_count_q);
 }
 
 /**
@@ -731,9 +729,7 @@ static void __smb2_lease_break_noti(struct work_struct *wk)
 
 	if (allocate_oplock_break_buf(work)) {
 		ksmbd_debug(OPLOCK, "smb2_allocate_rsp_buf failed! ");
-		ksmbd_free_work_struct(work);
-		atomic_dec(&conn->r_count);
-		return;
+		goto out;
 	}
 
 	rsp_hdr = smb2_get_msg(work->response_buf);
@@ -771,8 +767,11 @@ static void __smb2_lease_break_noti(struct work_struct *wk)
 	inc_rfc1001_len(work->response_buf, 44);
 
 	ksmbd_conn_write(work);
+
+out:
 	ksmbd_free_work_struct(work);
 	atomic_dec(&conn->r_count);
+	wake_up_all(&conn->r_count_q);
 }
 
 /**
diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c
index 4cd03d661df0..dfddc8dc1919 100644
--- a/fs/ksmbd/server.c
+++ b/fs/ksmbd/server.c
@@ -262,6 +262,7 @@ static void handle_ksmbd_work(struct work_struct *wk)
 	ksmbd_conn_try_dequeue_request(work);
 	ksmbd_free_work_struct(work);
 	atomic_dec(&conn->r_count);
+	wake_up_all(&conn->r_count_q);
 }
 
 /**
-- 
2.25.1

Re: [PATCH 4/5] ksmbd: use wait_event instead of schedule_timeout()

[Hyunchul Lee]

2022년 7월 22일 (금) 오후 12:04, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>
> ksmbd threads eating masses of cputime when connection is disconnected.
> If connection is disconnected, ksmbd thread waits for pending requests
> to be processed using schedule_timeout. schedule_timeout() incorrectly
> is used, and it is more efficient to use wait_event/wake_up than to check
> r_count every time with timeout.
>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
>  fs/ksmbd/connection.c |  6 +++---
>  fs/ksmbd/connection.h |  1 +
>  fs/ksmbd/oplock.c     | 21 ++++++++++-----------
>  fs/ksmbd/server.c     |  1 +
>  4 files changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
> index ce23cc89046e..756ad631c019 100644
> --- a/fs/ksmbd/connection.c
> +++ b/fs/ksmbd/connection.c
> @@ -66,6 +66,7 @@ struct ksmbd_conn *ksmbd_conn_alloc(void)
>         conn->outstanding_credits = 0;
>
>         init_waitqueue_head(&conn->req_running_q);
> +       init_waitqueue_head(&conn->r_count_q);
>         INIT_LIST_HEAD(&conn->conns_list);
>         INIT_LIST_HEAD(&conn->requests);
>         INIT_LIST_HEAD(&conn->async_requests);
> @@ -165,7 +166,6 @@ int ksmbd_conn_write(struct ksmbd_work *work)
>         struct kvec iov[3];
>         int iov_idx = 0;
>
> -       ksmbd_conn_try_dequeue_request(work);
>         if (!work->response_buf) {
>                 pr_err("NULL response header\n");
>                 return -EINVAL;
> @@ -347,8 +347,8 @@ int ksmbd_conn_handler_loop(void *p)
>
>  out:
>         /* Wait till all reference dropped to the Server object*/
> -       while (atomic_read(&conn->r_count) > 0)
> -               schedule_timeout(HZ);
> +       wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);
> +
>
>         unload_nls(conn->local_nls);
>         if (default_conn_ops.terminate_fn)
> diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h
> index 5b39f0bdeff8..2e4730457c92 100644
> --- a/fs/ksmbd/connection.h
> +++ b/fs/ksmbd/connection.h
> @@ -65,6 +65,7 @@ struct ksmbd_conn {
>         unsigned int                    outstanding_credits;
>         spinlock_t                      credits_lock;
>         wait_queue_head_t               req_running_q;
> +       wait_queue_head_t               r_count_q;
>         /* Lock to protect requests list*/
>         spinlock_t                      request_lock;
>         struct list_head                requests;
> diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
> index 8b5560574d4c..9bb4fb8b80de 100644
> --- a/fs/ksmbd/oplock.c
> +++ b/fs/ksmbd/oplock.c
> @@ -615,18 +615,13 @@ static void __smb2_oplock_break_noti(struct work_struct *wk)
>         struct ksmbd_file *fp;
>
>         fp = ksmbd_lookup_durable_fd(br_info->fid);
> -       if (!fp) {
> -               atomic_dec(&conn->r_count);
> -               ksmbd_free_work_struct(work);
> -               return;
> -       }
> +       if (!fp)
> +               goto out;
>
>         if (allocate_oplock_break_buf(work)) {
>                 pr_err("smb2_allocate_rsp_buf failed! ");
> -               atomic_dec(&conn->r_count);
>                 ksmbd_fd_put(work, fp);
> -               ksmbd_free_work_struct(work);
> -               return;
> +               goto out;
>         }
>
>         rsp_hdr = smb2_get_msg(work->response_buf);
> @@ -667,8 +662,11 @@ static void __smb2_oplock_break_noti(struct work_struct *wk)
>
>         ksmbd_fd_put(work, fp);
>         ksmbd_conn_write(work);
> +
> +out:
>         ksmbd_free_work_struct(work);
>         atomic_dec(&conn->r_count);
> +       wake_up_all(&conn->r_count_q);

I think calling wake_up_all is better if atomic_dec_return(&conn->r_count) == 0.
Otherwise, Looks good to me.

>  }
>
>  /**
> @@ -731,9 +729,7 @@ static void __smb2_lease_break_noti(struct work_struct *wk)
>
>         if (allocate_oplock_break_buf(work)) {
>                 ksmbd_debug(OPLOCK, "smb2_allocate_rsp_buf failed! ");
> -               ksmbd_free_work_struct(work);
> -               atomic_dec(&conn->r_count);
> -               return;
> +               goto out;
>         }
>
>         rsp_hdr = smb2_get_msg(work->response_buf);
> @@ -771,8 +767,11 @@ static void __smb2_lease_break_noti(struct work_struct *wk)
>         inc_rfc1001_len(work->response_buf, 44);
>
>         ksmbd_conn_write(work);
> +
> +out:
>         ksmbd_free_work_struct(work);
>         atomic_dec(&conn->r_count);
> +       wake_up_all(&conn->r_count_q);
>  }
>
>  /**
> diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c
> index 4cd03d661df0..dfddc8dc1919 100644
> --- a/fs/ksmbd/server.c
> +++ b/fs/ksmbd/server.c
> @@ -262,6 +262,7 @@ static void handle_ksmbd_work(struct work_struct *wk)
>         ksmbd_conn_try_dequeue_request(work);
>         ksmbd_free_work_struct(work);
>         atomic_dec(&conn->r_count);
> +       wake_up_all(&conn->r_count_q);
>  }
>
>  /**
> --
> 2.25.1
>


-- 
Thanks,
Hyunchul

Re: [PATCH 4/5] ksmbd: use wait_event instead of schedule_timeout()

[Namjae Jeon]

2022-07-25 9:48 GMT+09:00, Hyunchul Lee <hyc.lee@gmail.com>:
> 2022년 7월 22일 (금) 오후 12:04, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>>
>> ksmbd threads eating masses of cputime when connection is disconnected.
>> If connection is disconnected, ksmbd thread waits for pending requests
>> to be processed using schedule_timeout. schedule_timeout() incorrectly
>> is used, and it is more efficient to use wait_event/wake_up than to check
>> r_count every time with timeout.
>>
>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>> ---
>>  fs/ksmbd/connection.c |  6 +++---
>>  fs/ksmbd/connection.h |  1 +
>>  fs/ksmbd/oplock.c     | 21 ++++++++++-----------
>>  fs/ksmbd/server.c     |  1 +
>>  4 files changed, 15 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
>> index ce23cc89046e..756ad631c019 100644
>> --- a/fs/ksmbd/connection.c
>> +++ b/fs/ksmbd/connection.c
>> @@ -66,6 +66,7 @@ struct ksmbd_conn *ksmbd_conn_alloc(void)
>>         conn->outstanding_credits = 0;
>>
>>         init_waitqueue_head(&conn->req_running_q);
>> +       init_waitqueue_head(&conn->r_count_q);
>>         INIT_LIST_HEAD(&conn->conns_list);
>>         INIT_LIST_HEAD(&conn->requests);
>>         INIT_LIST_HEAD(&conn->async_requests);
>> @@ -165,7 +166,6 @@ int ksmbd_conn_write(struct ksmbd_work *work)
>>         struct kvec iov[3];
>>         int iov_idx = 0;
>>
>> -       ksmbd_conn_try_dequeue_request(work);
>>         if (!work->response_buf) {
>>                 pr_err("NULL response header\n");
>>                 return -EINVAL;
>> @@ -347,8 +347,8 @@ int ksmbd_conn_handler_loop(void *p)
>>
>>  out:
>>         /* Wait till all reference dropped to the Server object*/
>> -       while (atomic_read(&conn->r_count) > 0)
>> -               schedule_timeout(HZ);
>> +       wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);
>> +
>>
>>         unload_nls(conn->local_nls);
>>         if (default_conn_ops.terminate_fn)
>> diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h
>> index 5b39f0bdeff8..2e4730457c92 100644
>> --- a/fs/ksmbd/connection.h
>> +++ b/fs/ksmbd/connection.h
>> @@ -65,6 +65,7 @@ struct ksmbd_conn {
>>         unsigned int                    outstanding_credits;
>>         spinlock_t                      credits_lock;
>>         wait_queue_head_t               req_running_q;
>> +       wait_queue_head_t               r_count_q;
>>         /* Lock to protect requests list*/
>>         spinlock_t                      request_lock;
>>         struct list_head                requests;
>> diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
>> index 8b5560574d4c..9bb4fb8b80de 100644
>> --- a/fs/ksmbd/oplock.c
>> +++ b/fs/ksmbd/oplock.c
>> @@ -615,18 +615,13 @@ static void __smb2_oplock_break_noti(struct
>> work_struct *wk)
>>         struct ksmbd_file *fp;
>>
>>         fp = ksmbd_lookup_durable_fd(br_info->fid);
>> -       if (!fp) {
>> -               atomic_dec(&conn->r_count);
>> -               ksmbd_free_work_struct(work);
>> -               return;
>> -       }
>> +       if (!fp)
>> +               goto out;
>>
>>         if (allocate_oplock_break_buf(work)) {
>>                 pr_err("smb2_allocate_rsp_buf failed! ");
>> -               atomic_dec(&conn->r_count);
>>                 ksmbd_fd_put(work, fp);
>> -               ksmbd_free_work_struct(work);
>> -               return;
>> +               goto out;
>>         }
>>
>>         rsp_hdr = smb2_get_msg(work->response_buf);
>> @@ -667,8 +662,11 @@ static void __smb2_oplock_break_noti(struct
>> work_struct *wk)
>>
>>         ksmbd_fd_put(work, fp);
>>         ksmbd_conn_write(work);
>> +
>> +out:
>>         ksmbd_free_work_struct(work);
>>         atomic_dec(&conn->r_count);
>> +       wake_up_all(&conn->r_count_q);
>
> I think calling wake_up_all is better if atomic_dec_return(&conn->r_count)
> == 0.
Are there cases where r_count inc/dec doesn't pair?

> Otherwise, Looks good to me.
>
>>  }
>>
>>  /**
>> @@ -731,9 +729,7 @@ static void __smb2_lease_break_noti(struct work_struct
>> *wk)
>>
>>         if (allocate_oplock_break_buf(work)) {
>>                 ksmbd_debug(OPLOCK, "smb2_allocate_rsp_buf failed! ");
>> -               ksmbd_free_work_struct(work);
>> -               atomic_dec(&conn->r_count);
>> -               return;
>> +               goto out;
>>         }
>>
>>         rsp_hdr = smb2_get_msg(work->response_buf);
>> @@ -771,8 +767,11 @@ static void __smb2_lease_break_noti(struct
>> work_struct *wk)
>>         inc_rfc1001_len(work->response_buf, 44);
>>
>>         ksmbd_conn_write(work);
>> +
>> +out:
>>         ksmbd_free_work_struct(work);
>>         atomic_dec(&conn->r_count);
>> +       wake_up_all(&conn->r_count_q);
>>  }
>>
>>  /**
>> diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c
>> index 4cd03d661df0..dfddc8dc1919 100644
>> --- a/fs/ksmbd/server.c
>> +++ b/fs/ksmbd/server.c
>> @@ -262,6 +262,7 @@ static void handle_ksmbd_work(struct work_struct *wk)
>>         ksmbd_conn_try_dequeue_request(work);
>>         ksmbd_free_work_struct(work);
>>         atomic_dec(&conn->r_count);
>> +       wake_up_all(&conn->r_count_q);
>>  }
>>
>>  /**
>> --
>> 2.25.1
>>
>
>
> --
> Thanks,
> Hyunchul
>

[PATCH 5/5] ksmbd: fix racy issue while destroying session on multichannel

[Namjae Jeon]

After multi-channel connection with windows, Several channels of
session are connected. Among them, if there is a problem in one channel,
Windows connects again after disconnecting the channel. In this process,
the session is released and a kernel oop can occurs while processing
requests to other channels. When the channel is disconnected, if other
channels still exist in the session after deleting the channel from
the channel list in the session, the session should not be released.
Finally, the session will be released after all channels are disconnected.

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
---
 fs/ksmbd/auth.c              | 56 ++++++++++++++++-------------
 fs/ksmbd/auth.h              | 11 +++---
 fs/ksmbd/connection.h        |  7 ----
 fs/ksmbd/mgmt/tree_connect.c |  5 +--
 fs/ksmbd/mgmt/tree_connect.h |  4 ++-
 fs/ksmbd/mgmt/user_session.c | 68 ++++++++++++++++++++++++------------
 fs/ksmbd/mgmt/user_session.h |  7 ++--
 fs/ksmbd/oplock.c            | 11 +++---
 fs/ksmbd/smb2pdu.c           | 21 +++++------
 fs/ksmbd/smb_common.h        |  2 +-
 fs/ksmbd/vfs.c               |  3 +-
 fs/ksmbd/vfs_cache.c         |  2 +-
 12 files changed, 111 insertions(+), 86 deletions(-)

diff --git a/fs/ksmbd/auth.c b/fs/ksmbd/auth.c
index 911444d21267..c5a5c7b90d72 100644
--- a/fs/ksmbd/auth.c
+++ b/fs/ksmbd/auth.c
@@ -121,8 +121,8 @@ static int ksmbd_gen_sess_key(struct ksmbd_session *sess, char *hash,
 	return rc;
 }
 
-static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash,
-			    char *dname)
+static int calc_ntlmv2_hash(struct ksmbd_conn *conn, struct ksmbd_session *sess,
+			    char *ntlmv2_hash, char *dname)
 {
 	int ret, len, conv_len;
 	wchar_t *domain = NULL;
@@ -158,7 +158,7 @@ static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash,
 	}
 
 	conv_len = smb_strtoUTF16(uniname, user_name(sess->user), len,
-				  sess->conn->local_nls);
+				  conn->local_nls);
 	if (conv_len < 0 || conv_len > len) {
 		ret = -EINVAL;
 		goto out;
@@ -182,7 +182,7 @@ static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash,
 	}
 
 	conv_len = smb_strtoUTF16((__le16 *)domain, dname, len,
-				  sess->conn->local_nls);
+				  conn->local_nls);
 	if (conv_len < 0 || conv_len > len) {
 		ret = -EINVAL;
 		goto out;
@@ -215,8 +215,9 @@ static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash,
  *
  * Return:	0 on success, error number on error
  */
-int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp *ntlmv2,
-		      int blen, char *domain_name, char *cryptkey)
+int ksmbd_auth_ntlmv2(struct ksmbd_conn *conn, struct ksmbd_session *sess,
+		      struct ntlmv2_resp *ntlmv2, int blen, char *domain_name,
+		      char *cryptkey)
 {
 	char ntlmv2_hash[CIFS_ENCPWD_SIZE];
 	char ntlmv2_rsp[CIFS_HMAC_MD5_HASH_SIZE];
@@ -230,7 +231,7 @@ int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp *ntlmv2,
 		return -ENOMEM;
 	}
 
-	rc = calc_ntlmv2_hash(sess, ntlmv2_hash, domain_name);
+	rc = calc_ntlmv2_hash(conn, sess, ntlmv2_hash, domain_name);
 	if (rc) {
 		ksmbd_debug(AUTH, "could not get v2 hash rc %d\n", rc);
 		goto out;
@@ -333,7 +334,8 @@ int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob,
 	/* process NTLMv2 authentication */
 	ksmbd_debug(AUTH, "decode_ntlmssp_authenticate_blob dname%s\n",
 		    domain_name);
-	ret = ksmbd_auth_ntlmv2(sess, (struct ntlmv2_resp *)((char *)authblob + nt_off),
+	ret = ksmbd_auth_ntlmv2(conn, sess,
+				(struct ntlmv2_resp *)((char *)authblob + nt_off),
 				nt_len - CIFS_ENCPWD_SIZE,
 				domain_name, conn->ntlmssp.cryptkey);
 	kfree(domain_name);
@@ -659,8 +661,9 @@ struct derivation {
 	bool binding;
 };
 
-static int generate_key(struct ksmbd_session *sess, struct kvec label,
-			struct kvec context, __u8 *key, unsigned int key_size)
+static int generate_key(struct ksmbd_conn *conn, struct ksmbd_session *sess,
+			struct kvec label, struct kvec context, __u8 *key,
+			unsigned int key_size)
 {
 	unsigned char zero = 0x0;
 	__u8 i[4] = {0, 0, 0, 1};
@@ -720,8 +723,8 @@ static int generate_key(struct ksmbd_session *sess, struct kvec label,
 		goto smb3signkey_ret;
 	}
 
-	if (sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
-	    sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM)
+	if (conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
+	    conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM)
 		rc = crypto_shash_update(CRYPTO_HMACSHA256(ctx), L256, 4);
 	else
 		rc = crypto_shash_update(CRYPTO_HMACSHA256(ctx), L128, 4);
@@ -756,17 +759,17 @@ static int generate_smb3signingkey(struct ksmbd_session *sess,
 	if (!chann)
 		return 0;
 
-	if (sess->conn->dialect >= SMB30_PROT_ID && signing->binding)
+	if (conn->dialect >= SMB30_PROT_ID && signing->binding)
 		key = chann->smb3signingkey;
 	else
 		key = sess->smb3signingkey;
 
-	rc = generate_key(sess, signing->label, signing->context, key,
+	rc = generate_key(conn, sess, signing->label, signing->context, key,
 			  SMB3_SIGN_KEY_SIZE);
 	if (rc)
 		return rc;
 
-	if (!(sess->conn->dialect >= SMB30_PROT_ID && signing->binding))
+	if (!(conn->dialect >= SMB30_PROT_ID && signing->binding))
 		memcpy(chann->smb3signingkey, key, SMB3_SIGN_KEY_SIZE);
 
 	ksmbd_debug(AUTH, "dumping generated AES signing keys\n");
@@ -820,30 +823,31 @@ struct derivation_twin {
 	struct derivation decryption;
 };
 
-static int generate_smb3encryptionkey(struct ksmbd_session *sess,
+static int generate_smb3encryptionkey(struct ksmbd_conn *conn,
+				      struct ksmbd_session *sess,
 				      const struct derivation_twin *ptwin)
 {
 	int rc;
 
-	rc = generate_key(sess, ptwin->encryption.label,
+	rc = generate_key(conn, sess, ptwin->encryption.label,
 			  ptwin->encryption.context, sess->smb3encryptionkey,
 			  SMB3_ENC_DEC_KEY_SIZE);
 	if (rc)
 		return rc;
 
-	rc = generate_key(sess, ptwin->decryption.label,
+	rc = generate_key(conn, sess, ptwin->decryption.label,
 			  ptwin->decryption.context,
 			  sess->smb3decryptionkey, SMB3_ENC_DEC_KEY_SIZE);
 	if (rc)
 		return rc;
 
 	ksmbd_debug(AUTH, "dumping generated AES encryption keys\n");
-	ksmbd_debug(AUTH, "Cipher type   %d\n", sess->conn->cipher_type);
+	ksmbd_debug(AUTH, "Cipher type   %d\n", conn->cipher_type);
 	ksmbd_debug(AUTH, "Session Id    %llu\n", sess->id);
 	ksmbd_debug(AUTH, "Session Key   %*ph\n",
 		    SMB2_NTLMV2_SESSKEY_SIZE, sess->sess_key);
-	if (sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
-	    sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM) {
+	if (conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
+	    conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM) {
 		ksmbd_debug(AUTH, "ServerIn Key  %*ph\n",
 			    SMB3_GCM256_CRYPTKEY_SIZE, sess->smb3encryptionkey);
 		ksmbd_debug(AUTH, "ServerOut Key %*ph\n",
@@ -857,7 +861,8 @@ static int generate_smb3encryptionkey(struct ksmbd_session *sess,
 	return 0;
 }
 
-int ksmbd_gen_smb30_encryptionkey(struct ksmbd_session *sess)
+int ksmbd_gen_smb30_encryptionkey(struct ksmbd_conn *conn,
+				  struct ksmbd_session *sess)
 {
 	struct derivation_twin twin;
 	struct derivation *d;
@@ -874,10 +879,11 @@ int ksmbd_gen_smb30_encryptionkey(struct ksmbd_session *sess)
 	d->context.iov_base = "ServerIn ";
 	d->context.iov_len = 10;
 
-	return generate_smb3encryptionkey(sess, &twin);
+	return generate_smb3encryptionkey(conn, sess, &twin);
 }
 
-int ksmbd_gen_smb311_encryptionkey(struct ksmbd_session *sess)
+int ksmbd_gen_smb311_encryptionkey(struct ksmbd_conn *conn,
+				   struct ksmbd_session *sess)
 {
 	struct derivation_twin twin;
 	struct derivation *d;
@@ -894,7 +900,7 @@ int ksmbd_gen_smb311_encryptionkey(struct ksmbd_session *sess)
 	d->context.iov_base = sess->Preauth_HashValue;
 	d->context.iov_len = 64;
 
-	return generate_smb3encryptionkey(sess, &twin);
+	return generate_smb3encryptionkey(conn, sess, &twin);
 }
 
 int ksmbd_gen_preauth_integrity_hash(struct ksmbd_conn *conn, char *buf,
diff --git a/fs/ksmbd/auth.h b/fs/ksmbd/auth.h
index 95629651cf26..25b772653de0 100644
--- a/fs/ksmbd/auth.h
+++ b/fs/ksmbd/auth.h
@@ -38,8 +38,9 @@ struct kvec;
 int ksmbd_crypt_message(struct ksmbd_conn *conn, struct kvec *iov,
 			unsigned int nvec, int enc);
 void ksmbd_copy_gss_neg_header(void *buf);
-int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp *ntlmv2,
-		      int blen, char *domain_name, char *cryptkey);
+int ksmbd_auth_ntlmv2(struct ksmbd_conn *conn, struct ksmbd_session *sess,
+		      struct ntlmv2_resp *ntlmv2, int blen, char *domain_name,
+		      char *cryptkey);
 int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob,
 				   int blob_len, struct ksmbd_conn *conn,
 				   struct ksmbd_session *sess);
@@ -58,8 +59,10 @@ int ksmbd_gen_smb30_signingkey(struct ksmbd_session *sess,
 			       struct ksmbd_conn *conn);
 int ksmbd_gen_smb311_signingkey(struct ksmbd_session *sess,
 				struct ksmbd_conn *conn);
-int ksmbd_gen_smb30_encryptionkey(struct ksmbd_session *sess);
-int ksmbd_gen_smb311_encryptionkey(struct ksmbd_session *sess);
+int ksmbd_gen_smb30_encryptionkey(struct ksmbd_conn *conn,
+				  struct ksmbd_session *sess);
+int ksmbd_gen_smb311_encryptionkey(struct ksmbd_conn *conn,
+				   struct ksmbd_session *sess);
 int ksmbd_gen_preauth_integrity_hash(struct ksmbd_conn *conn, char *buf,
 				     __u8 *pi_hash);
 int ksmbd_gen_sd_hash(struct ksmbd_conn *conn, char *sd_buf, int len,
diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h
index 2e4730457c92..e7f7d5707951 100644
--- a/fs/ksmbd/connection.h
+++ b/fs/ksmbd/connection.h
@@ -20,13 +20,6 @@
 
 #define KSMBD_SOCKET_BACKLOG		16
 
-/*
- * WARNING
- *
- * This is nothing but a HACK. Session status should move to channel
- * or to session. As of now we have 1 tcp_conn : 1 ksmbd_session, but
- * we need to change it to 1 tcp_conn : N ksmbd_sessions.
- */
 enum {
 	KSMBD_SESS_NEW = 0,
 	KSMBD_SESS_GOOD,
diff --git a/fs/ksmbd/mgmt/tree_connect.c b/fs/ksmbd/mgmt/tree_connect.c
index 0d28e723a28c..b35ea6a6abc5 100644
--- a/fs/ksmbd/mgmt/tree_connect.c
+++ b/fs/ksmbd/mgmt/tree_connect.c
@@ -16,7 +16,8 @@
 #include "user_session.h"
 
 struct ksmbd_tree_conn_status
-ksmbd_tree_conn_connect(struct ksmbd_session *sess, char *share_name)
+ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session *sess,
+			char *share_name)
 {
 	struct ksmbd_tree_conn_status status = {-EINVAL, NULL};
 	struct ksmbd_tree_connect_response *resp = NULL;
@@ -41,7 +42,7 @@ ksmbd_tree_conn_connect(struct ksmbd_session *sess, char *share_name)
 		goto out_error;
 	}
 
-	peer_addr = KSMBD_TCP_PEER_SOCKADDR(sess->conn);
+	peer_addr = KSMBD_TCP_PEER_SOCKADDR(conn);
 	resp = ksmbd_ipc_tree_connect_request(sess,
 					      sc,
 					      tree_conn,
diff --git a/fs/ksmbd/mgmt/tree_connect.h b/fs/ksmbd/mgmt/tree_connect.h
index 18e2a996e0aa..71e50271dccf 100644
--- a/fs/ksmbd/mgmt/tree_connect.h
+++ b/fs/ksmbd/mgmt/tree_connect.h
@@ -12,6 +12,7 @@
 
 struct ksmbd_share_config;
 struct ksmbd_user;
+struct ksmbd_conn;
 
 struct ksmbd_tree_connect {
 	int				id;
@@ -40,7 +41,8 @@ static inline int test_tree_conn_flag(struct ksmbd_tree_connect *tree_conn,
 struct ksmbd_session;
 
 struct ksmbd_tree_conn_status
-ksmbd_tree_conn_connect(struct ksmbd_session *sess, char *share_name);
+ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session *sess,
+			char *share_name);
 
 int ksmbd_tree_conn_disconnect(struct ksmbd_session *sess,
 			       struct ksmbd_tree_connect *tree_conn);
diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c
index b9acb6770b03..ce32fdd66807 100644
--- a/fs/ksmbd/mgmt/user_session.c
+++ b/fs/ksmbd/mgmt/user_session.c
@@ -151,9 +151,6 @@ void ksmbd_session_destroy(struct ksmbd_session *sess)
 	if (!sess)
 		return;
 
-	if (!atomic_dec_and_test(&sess->refcnt))
-		return;
-
 	down_write(&sessions_table_lock);
 	hash_del(&sess->hlist);
 	up_write(&sessions_table_lock);
@@ -184,16 +181,59 @@ static struct ksmbd_session *__session_lookup(unsigned long long id)
 int ksmbd_session_register(struct ksmbd_conn *conn,
 			   struct ksmbd_session *sess)
 {
-	sess->conn = conn;
+	sess->dialect = conn->dialect;
+	memcpy(sess->ClientGUID, conn->ClientGUID, SMB2_CLIENT_GUID_SIZE);
 	return xa_err(xa_store(&conn->sessions, sess->id, sess, GFP_KERNEL));
 }
 
+static int ksmbd_chann_del(struct ksmbd_conn *conn, struct ksmbd_session *sess)
+{
+	struct channel *chann, *tmp;
+
+	write_lock(&sess->chann_lock);
+	list_for_each_entry_safe(chann, tmp, &sess->ksmbd_chann_list,
+				 chann_list) {
+		if (chann->conn == conn) {
+			list_del(&chann->chann_list);
+			kfree(chann);
+			write_unlock(&sess->chann_lock);
+			return 0;
+		}
+	}
+	write_unlock(&sess->chann_lock);
+
+	return -ENOENT;
+}
+
 void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
 {
 	struct ksmbd_session *sess;
-	unsigned long id;
 
-	xa_for_each(&conn->sessions, id, sess) {
+	if (conn->binding) {
+		struct hlist_node *tmp;
+		int bkt;
+
+		down_write(&sessions_table_lock);
+		hash_for_each_safe(sessions_table, bkt, tmp, sess, hlist) {
+			if (!ksmbd_chann_del(conn, sess)) {
+				up_write(&sessions_table_lock);
+				goto sess_destroy;
+			}
+		}
+		up_write(&sessions_table_lock);
+	} else {
+		unsigned long id;
+
+		xa_for_each(&conn->sessions, id, sess) {
+			if (!ksmbd_chann_del(conn, sess))
+				goto sess_destroy;
+		}
+	}
+
+	return;
+
+sess_destroy:
+	if (list_empty(&sess->ksmbd_chann_list)) {
 		xa_erase(&conn->sessions, sess->id);
 		ksmbd_session_destroy(sess);
 	}
@@ -205,27 +245,12 @@ struct ksmbd_session *ksmbd_session_lookup(struct ksmbd_conn *conn,
 	return xa_load(&conn->sessions, id);
 }
 
-int get_session(struct ksmbd_session *sess)
-{
-	return atomic_inc_not_zero(&sess->refcnt);
-}
-
-void put_session(struct ksmbd_session *sess)
-{
-	if (atomic_dec_and_test(&sess->refcnt))
-		pr_err("get/%s seems to be mismatched.", __func__);
-}
-
 struct ksmbd_session *ksmbd_session_lookup_slowpath(unsigned long long id)
 {
 	struct ksmbd_session *sess;
 
 	down_read(&sessions_table_lock);
 	sess = __session_lookup(id);
-	if (sess) {
-		if (!get_session(sess))
-			sess = NULL;
-	}
 	up_read(&sessions_table_lock);
 
 	return sess;
@@ -306,7 +331,6 @@ static struct ksmbd_session *__session_create(int protocol)
 	INIT_LIST_HEAD(&sess->ksmbd_chann_list);
 	INIT_LIST_HEAD(&sess->rpc_handle_list);
 	sess->sequence_number = 1;
-	atomic_set(&sess->refcnt, 1);
 	rwlock_init(&sess->chann_lock);
 
 	switch (protocol) {
diff --git a/fs/ksmbd/mgmt/user_session.h b/fs/ksmbd/mgmt/user_session.h
index 1ec659f0151b..8934b8ee275b 100644
--- a/fs/ksmbd/mgmt/user_session.h
+++ b/fs/ksmbd/mgmt/user_session.h
@@ -33,8 +33,10 @@ struct preauth_session {
 struct ksmbd_session {
 	u64				id;
 
+	__u16				dialect;
+	char				ClientGUID[SMB2_CLIENT_GUID_SIZE];
+
 	struct ksmbd_user		*user;
-	struct ksmbd_conn		*conn;
 	unsigned int			sequence_number;
 	unsigned int			flags;
 
@@ -59,7 +61,6 @@ struct ksmbd_session {
 	__u8				smb3signingkey[SMB3_SIGN_KEY_SIZE];
 
 	struct ksmbd_file_table		file_table;
-	atomic_t			refcnt;
 };
 
 static inline int test_session_flag(struct ksmbd_session *sess, int bit)
@@ -100,6 +101,4 @@ void ksmbd_release_tree_conn_id(struct ksmbd_session *sess, int id);
 int ksmbd_session_rpc_open(struct ksmbd_session *sess, char *rpc_name);
 void ksmbd_session_rpc_close(struct ksmbd_session *sess, int id);
 int ksmbd_session_rpc_method(struct ksmbd_session *sess, int id);
-int get_session(struct ksmbd_session *sess);
-void put_session(struct ksmbd_session *sess);
 #endif /* __USER_SESSION_MANAGEMENT_H__ */
diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
index 9bb4fb8b80de..ce4c15c3554d 100644
--- a/fs/ksmbd/oplock.c
+++ b/fs/ksmbd/oplock.c
@@ -30,6 +30,7 @@ static DEFINE_RWLOCK(lease_list_lock);
 static struct oplock_info *alloc_opinfo(struct ksmbd_work *work,
 					u64 id, __u16 Tid)
 {
+	struct ksmbd_conn *conn = work->conn;
 	struct ksmbd_session *sess = work->sess;
 	struct oplock_info *opinfo;
 
@@ -38,7 +39,7 @@ static struct oplock_info *alloc_opinfo(struct ksmbd_work *work,
 		return NULL;
 
 	opinfo->sess = sess;
-	opinfo->conn = sess->conn;
+	opinfo->conn = conn;
 	opinfo->level = SMB2_OPLOCK_LEVEL_NONE;
 	opinfo->op_state = OPLOCK_STATE_NONE;
 	opinfo->pending_break = 0;
@@ -971,7 +972,7 @@ int find_same_lease_key(struct ksmbd_session *sess, struct ksmbd_inode *ci,
 	}
 
 	list_for_each_entry(lb, &lease_table_list, l_entry) {
-		if (!memcmp(lb->client_guid, sess->conn->ClientGUID,
+		if (!memcmp(lb->client_guid, sess->ClientGUID,
 			    SMB2_CLIENT_GUID_SIZE))
 			goto found;
 	}
@@ -987,7 +988,7 @@ int find_same_lease_key(struct ksmbd_session *sess, struct ksmbd_inode *ci,
 		rcu_read_unlock();
 		if (opinfo->o_fp->f_ci == ci)
 			goto op_next;
-		err = compare_guid_key(opinfo, sess->conn->ClientGUID,
+		err = compare_guid_key(opinfo, sess->ClientGUID,
 				       lctx->lease_key);
 		if (err) {
 			err = -EINVAL;
@@ -1121,7 +1122,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
 		struct oplock_info *m_opinfo;
 
 		/* is lease already granted ? */
-		m_opinfo = same_client_has_lease(ci, sess->conn->ClientGUID,
+		m_opinfo = same_client_has_lease(ci, sess->ClientGUID,
 						 lctx);
 		if (m_opinfo) {
 			copy_lease(m_opinfo, opinfo);
@@ -1239,7 +1240,7 @@ void smb_break_all_levII_oplock(struct ksmbd_work *work, struct ksmbd_file *fp,
 {
 	struct oplock_info *op, *brk_op;
 	struct ksmbd_inode *ci;
-	struct ksmbd_conn *conn = work->sess->conn;
+	struct ksmbd_conn *conn = work->conn;
 
 	if (!test_share_config_flag(work->tcon->share_conf,
 				    KSMBD_SHARE_FLAG_OPLOCKS))
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index ae5677a66cb2..1f4f2d5217a6 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -603,12 +603,9 @@ static void destroy_previous_session(struct ksmbd_conn *conn,
 	if (!prev_user ||
 	    strcmp(user->name, prev_user->name) ||
 	    user->passkey_sz != prev_user->passkey_sz ||
-	    memcmp(user->passkey, prev_user->passkey, user->passkey_sz)) {
-		put_session(prev_sess);
+	    memcmp(user->passkey, prev_user->passkey, user->passkey_sz))
 		return;
-	}
 
-	put_session(prev_sess);
 	prev_sess->state = SMB2_SESSION_EXPIRED;
 	write_lock(&prev_sess->chann_lock);
 	list_for_each_entry(chann, &prev_sess->ksmbd_chann_list, chann_list)
@@ -1499,7 +1496,7 @@ static int ntlm_authenticate(struct ksmbd_work *work)
 
 	if (smb3_encryption_negotiated(conn) &&
 			!(req->Flags & SMB2_SESSION_REQ_FLAG_BINDING)) {
-		rc = conn->ops->generate_encryptionkey(sess);
+		rc = conn->ops->generate_encryptionkey(conn, sess);
 		if (rc) {
 			ksmbd_debug(SMB,
 					"SMB3 encryption key generation failed\n");
@@ -1590,7 +1587,7 @@ static int krb5_authenticate(struct ksmbd_work *work)
 		sess->sign = true;
 
 	if (smb3_encryption_negotiated(conn)) {
-		retval = conn->ops->generate_encryptionkey(sess);
+		retval = conn->ops->generate_encryptionkey(conn, sess);
 		if (retval) {
 			ksmbd_debug(SMB,
 				    "SMB3 encryption key generation failed\n");
@@ -1678,7 +1675,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
 			goto out_err;
 		}
 
-		if (conn->dialect != sess->conn->dialect) {
+		if (conn->dialect != sess->dialect) {
 			rc = -EINVAL;
 			goto out_err;
 		}
@@ -1688,7 +1685,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
 			goto out_err;
 		}
 
-		if (strncmp(conn->ClientGUID, sess->conn->ClientGUID,
+		if (strncmp(conn->ClientGUID, sess->ClientGUID,
 			    SMB2_CLIENT_GUID_SIZE)) {
 			rc = -ENOENT;
 			goto out_err;
@@ -1890,7 +1887,7 @@ int smb2_tree_connect(struct ksmbd_work *work)
 	ksmbd_debug(SMB, "tree connect request for tree %s treename %s\n",
 		    name, treename);
 
-	status = ksmbd_tree_conn_connect(sess, name);
+	status = ksmbd_tree_conn_connect(conn, sess, name);
 	if (status.ret == KSMBD_TREE_CONN_STATUS_OK)
 		rsp->hdr.Id.SyncId.TreeId = cpu_to_le32(status.tree_conn->id);
 	else
@@ -4875,7 +4872,7 @@ static int smb2_get_info_filesystem(struct ksmbd_work *work,
 				    struct smb2_query_info_rsp *rsp)
 {
 	struct ksmbd_session *sess = work->sess;
-	struct ksmbd_conn *conn = sess->conn;
+	struct ksmbd_conn *conn = work->conn;
 	struct ksmbd_share_config *share = work->tcon->share_conf;
 	int fsinfoclass = 0;
 	struct kstatfs stfs;
@@ -5793,7 +5790,7 @@ static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp,
 	}
 next:
 	return smb2_rename(work, fp, user_ns, rename_info,
-			   work->sess->conn->local_nls);
+			   work->conn->local_nls);
 }
 
 static int set_file_disposition_info(struct ksmbd_file *fp,
@@ -5925,7 +5922,7 @@ static int smb2_set_info_file(struct ksmbd_work *work, struct ksmbd_file *fp,
 		return smb2_create_link(work, work->tcon->share_conf,
 					(struct smb2_file_link_info *)req->Buffer,
 					buf_len, fp->filp,
-					work->sess->conn->local_nls);
+					work->conn->local_nls);
 	}
 	case FILE_DISPOSITION_INFORMATION:
 	{
diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h
index e1369b4345a9..318c16fa81da 100644
--- a/fs/ksmbd/smb_common.h
+++ b/fs/ksmbd/smb_common.h
@@ -421,7 +421,7 @@ struct smb_version_ops {
 	int (*check_sign_req)(struct ksmbd_work *work);
 	void (*set_sign_rsp)(struct ksmbd_work *work);
 	int (*generate_signingkey)(struct ksmbd_session *sess, struct ksmbd_conn *conn);
-	int (*generate_encryptionkey)(struct ksmbd_session *sess);
+	int (*generate_encryptionkey)(struct ksmbd_conn *conn, struct ksmbd_session *sess);
 	bool (*is_transform_hdr)(void *buf);
 	int (*decrypt_req)(struct ksmbd_work *work);
 	int (*encrypt_resp)(struct ksmbd_work *work);
diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c
index 5d185564aef6..4f75b1436eab 100644
--- a/fs/ksmbd/vfs.c
+++ b/fs/ksmbd/vfs.c
@@ -481,12 +481,11 @@ int ksmbd_vfs_write(struct ksmbd_work *work, struct ksmbd_file *fp,
 		    char *buf, size_t count, loff_t *pos, bool sync,
 		    ssize_t *written)
 {
-	struct ksmbd_session *sess = work->sess;
 	struct file *filp;
 	loff_t	offset = *pos;
 	int err = 0;
 
-	if (sess->conn->connection_type) {
+	if (work->conn->connection_type) {
 		if (!(fp->daccess & FILE_WRITE_DATA_LE)) {
 			pr_err("no right to write(%pd)\n",
 			       fp->filp->f_path.dentry);
diff --git a/fs/ksmbd/vfs_cache.c b/fs/ksmbd/vfs_cache.c
index c4d59d2735f0..da9163b00350 100644
--- a/fs/ksmbd/vfs_cache.c
+++ b/fs/ksmbd/vfs_cache.c
@@ -569,7 +569,7 @@ struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp)
 	atomic_set(&fp->refcount, 1);
 
 	fp->filp		= filp;
-	fp->conn		= work->sess->conn;
+	fp->conn		= work->conn;
 	fp->tcon		= work->tcon;
 	fp->volatile_id		= KSMBD_NO_FID;
 	fp->persistent_id	= KSMBD_NO_FID;
-- 
2.25.1

Re: [PATCH 5/5] ksmbd: fix racy issue while destroying session on multichannel

[Hyunchul Lee]

2022년 7월 22일 (금) 오후 12:04, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>
> After multi-channel connection with windows, Several channels of
> session are connected. Among them, if there is a problem in one channel,
> Windows connects again after disconnecting the channel. In this process,
> the session is released and a kernel oop can occurs while processing
> requests to other channels. When the channel is disconnected, if other
> channels still exist in the session after deleting the channel from
> the channel list in the session, the session should not be released.
> Finally, the session will be released after all channels are disconnected.
>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
> ---
>  fs/ksmbd/auth.c              | 56 ++++++++++++++++-------------
>  fs/ksmbd/auth.h              | 11 +++---
>  fs/ksmbd/connection.h        |  7 ----
>  fs/ksmbd/mgmt/tree_connect.c |  5 +--
>  fs/ksmbd/mgmt/tree_connect.h |  4 ++-
>  fs/ksmbd/mgmt/user_session.c | 68 ++++++++++++++++++++++++------------
>  fs/ksmbd/mgmt/user_session.h |  7 ++--
>  fs/ksmbd/oplock.c            | 11 +++---
>  fs/ksmbd/smb2pdu.c           | 21 +++++------
>  fs/ksmbd/smb_common.h        |  2 +-
>  fs/ksmbd/vfs.c               |  3 +-
>  fs/ksmbd/vfs_cache.c         |  2 +-
>  12 files changed, 111 insertions(+), 86 deletions(-)
>
> diff --git a/fs/ksmbd/auth.c b/fs/ksmbd/auth.c
> index 911444d21267..c5a5c7b90d72 100644
> --- a/fs/ksmbd/auth.c
> +++ b/fs/ksmbd/auth.c
> @@ -121,8 +121,8 @@ static int ksmbd_gen_sess_key(struct ksmbd_session *sess, char *hash,
>         return rc;
>  }
>
> -static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash,
> -                           char *dname)
> +static int calc_ntlmv2_hash(struct ksmbd_conn *conn, struct ksmbd_session *sess,
> +                           char *ntlmv2_hash, char *dname)
>  {
>         int ret, len, conv_len;
>         wchar_t *domain = NULL;
> @@ -158,7 +158,7 @@ static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash,
>         }
>
>         conv_len = smb_strtoUTF16(uniname, user_name(sess->user), len,
> -                                 sess->conn->local_nls);
> +                                 conn->local_nls);
>         if (conv_len < 0 || conv_len > len) {
>                 ret = -EINVAL;
>                 goto out;
> @@ -182,7 +182,7 @@ static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash,
>         }
>
>         conv_len = smb_strtoUTF16((__le16 *)domain, dname, len,
> -                                 sess->conn->local_nls);
> +                                 conn->local_nls);
>         if (conv_len < 0 || conv_len > len) {
>                 ret = -EINVAL;
>                 goto out;
> @@ -215,8 +215,9 @@ static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash,
>   *
>   * Return:     0 on success, error number on error
>   */
> -int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp *ntlmv2,
> -                     int blen, char *domain_name, char *cryptkey)
> +int ksmbd_auth_ntlmv2(struct ksmbd_conn *conn, struct ksmbd_session *sess,
> +                     struct ntlmv2_resp *ntlmv2, int blen, char *domain_name,
> +                     char *cryptkey)
>  {
>         char ntlmv2_hash[CIFS_ENCPWD_SIZE];
>         char ntlmv2_rsp[CIFS_HMAC_MD5_HASH_SIZE];
> @@ -230,7 +231,7 @@ int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp *ntlmv2,
>                 return -ENOMEM;
>         }
>
> -       rc = calc_ntlmv2_hash(sess, ntlmv2_hash, domain_name);
> +       rc = calc_ntlmv2_hash(conn, sess, ntlmv2_hash, domain_name);
>         if (rc) {
>                 ksmbd_debug(AUTH, "could not get v2 hash rc %d\n", rc);
>                 goto out;
> @@ -333,7 +334,8 @@ int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob,
>         /* process NTLMv2 authentication */
>         ksmbd_debug(AUTH, "decode_ntlmssp_authenticate_blob dname%s\n",
>                     domain_name);
> -       ret = ksmbd_auth_ntlmv2(sess, (struct ntlmv2_resp *)((char *)authblob + nt_off),
> +       ret = ksmbd_auth_ntlmv2(conn, sess,
> +                               (struct ntlmv2_resp *)((char *)authblob + nt_off),
>                                 nt_len - CIFS_ENCPWD_SIZE,
>                                 domain_name, conn->ntlmssp.cryptkey);
>         kfree(domain_name);
> @@ -659,8 +661,9 @@ struct derivation {
>         bool binding;
>  };
>
> -static int generate_key(struct ksmbd_session *sess, struct kvec label,
> -                       struct kvec context, __u8 *key, unsigned int key_size)
> +static int generate_key(struct ksmbd_conn *conn, struct ksmbd_session *sess,
> +                       struct kvec label, struct kvec context, __u8 *key,
> +                       unsigned int key_size)
>  {
>         unsigned char zero = 0x0;
>         __u8 i[4] = {0, 0, 0, 1};
> @@ -720,8 +723,8 @@ static int generate_key(struct ksmbd_session *sess, struct kvec label,
>                 goto smb3signkey_ret;
>         }
>
> -       if (sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
> -           sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM)
> +       if (conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
> +           conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM)
>                 rc = crypto_shash_update(CRYPTO_HMACSHA256(ctx), L256, 4);
>         else
>                 rc = crypto_shash_update(CRYPTO_HMACSHA256(ctx), L128, 4);
> @@ -756,17 +759,17 @@ static int generate_smb3signingkey(struct ksmbd_session *sess,
>         if (!chann)
>                 return 0;
>
> -       if (sess->conn->dialect >= SMB30_PROT_ID && signing->binding)
> +       if (conn->dialect >= SMB30_PROT_ID && signing->binding)
>                 key = chann->smb3signingkey;
>         else
>                 key = sess->smb3signingkey;
>
> -       rc = generate_key(sess, signing->label, signing->context, key,
> +       rc = generate_key(conn, sess, signing->label, signing->context, key,
>                           SMB3_SIGN_KEY_SIZE);
>         if (rc)
>                 return rc;
>
> -       if (!(sess->conn->dialect >= SMB30_PROT_ID && signing->binding))
> +       if (!(conn->dialect >= SMB30_PROT_ID && signing->binding))
>                 memcpy(chann->smb3signingkey, key, SMB3_SIGN_KEY_SIZE);
>
>         ksmbd_debug(AUTH, "dumping generated AES signing keys\n");
> @@ -820,30 +823,31 @@ struct derivation_twin {
>         struct derivation decryption;
>  };
>
> -static int generate_smb3encryptionkey(struct ksmbd_session *sess,
> +static int generate_smb3encryptionkey(struct ksmbd_conn *conn,
> +                                     struct ksmbd_session *sess,
>                                       const struct derivation_twin *ptwin)
>  {
>         int rc;
>
> -       rc = generate_key(sess, ptwin->encryption.label,
> +       rc = generate_key(conn, sess, ptwin->encryption.label,
>                           ptwin->encryption.context, sess->smb3encryptionkey,
>                           SMB3_ENC_DEC_KEY_SIZE);
>         if (rc)
>                 return rc;
>
> -       rc = generate_key(sess, ptwin->decryption.label,
> +       rc = generate_key(conn, sess, ptwin->decryption.label,
>                           ptwin->decryption.context,
>                           sess->smb3decryptionkey, SMB3_ENC_DEC_KEY_SIZE);
>         if (rc)
>                 return rc;
>
>         ksmbd_debug(AUTH, "dumping generated AES encryption keys\n");
> -       ksmbd_debug(AUTH, "Cipher type   %d\n", sess->conn->cipher_type);
> +       ksmbd_debug(AUTH, "Cipher type   %d\n", conn->cipher_type);
>         ksmbd_debug(AUTH, "Session Id    %llu\n", sess->id);
>         ksmbd_debug(AUTH, "Session Key   %*ph\n",
>                     SMB2_NTLMV2_SESSKEY_SIZE, sess->sess_key);
> -       if (sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
> -           sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM) {
> +       if (conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
> +           conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM) {
>                 ksmbd_debug(AUTH, "ServerIn Key  %*ph\n",
>                             SMB3_GCM256_CRYPTKEY_SIZE, sess->smb3encryptionkey);
>                 ksmbd_debug(AUTH, "ServerOut Key %*ph\n",
> @@ -857,7 +861,8 @@ static int generate_smb3encryptionkey(struct ksmbd_session *sess,
>         return 0;
>  }
>
> -int ksmbd_gen_smb30_encryptionkey(struct ksmbd_session *sess)
> +int ksmbd_gen_smb30_encryptionkey(struct ksmbd_conn *conn,
> +                                 struct ksmbd_session *sess)
>  {
>         struct derivation_twin twin;
>         struct derivation *d;
> @@ -874,10 +879,11 @@ int ksmbd_gen_smb30_encryptionkey(struct ksmbd_session *sess)
>         d->context.iov_base = "ServerIn ";
>         d->context.iov_len = 10;
>
> -       return generate_smb3encryptionkey(sess, &twin);
> +       return generate_smb3encryptionkey(conn, sess, &twin);
>  }
>
> -int ksmbd_gen_smb311_encryptionkey(struct ksmbd_session *sess)
> +int ksmbd_gen_smb311_encryptionkey(struct ksmbd_conn *conn,
> +                                  struct ksmbd_session *sess)
>  {
>         struct derivation_twin twin;
>         struct derivation *d;
> @@ -894,7 +900,7 @@ int ksmbd_gen_smb311_encryptionkey(struct ksmbd_session *sess)
>         d->context.iov_base = sess->Preauth_HashValue;
>         d->context.iov_len = 64;
>
> -       return generate_smb3encryptionkey(sess, &twin);
> +       return generate_smb3encryptionkey(conn, sess, &twin);
>  }
>
>  int ksmbd_gen_preauth_integrity_hash(struct ksmbd_conn *conn, char *buf,
> diff --git a/fs/ksmbd/auth.h b/fs/ksmbd/auth.h
> index 95629651cf26..25b772653de0 100644
> --- a/fs/ksmbd/auth.h
> +++ b/fs/ksmbd/auth.h
> @@ -38,8 +38,9 @@ struct kvec;
>  int ksmbd_crypt_message(struct ksmbd_conn *conn, struct kvec *iov,
>                         unsigned int nvec, int enc);
>  void ksmbd_copy_gss_neg_header(void *buf);
> -int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp *ntlmv2,
> -                     int blen, char *domain_name, char *cryptkey);
> +int ksmbd_auth_ntlmv2(struct ksmbd_conn *conn, struct ksmbd_session *sess,
> +                     struct ntlmv2_resp *ntlmv2, int blen, char *domain_name,
> +                     char *cryptkey);
>  int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob,
>                                    int blob_len, struct ksmbd_conn *conn,
>                                    struct ksmbd_session *sess);
> @@ -58,8 +59,10 @@ int ksmbd_gen_smb30_signingkey(struct ksmbd_session *sess,
>                                struct ksmbd_conn *conn);
>  int ksmbd_gen_smb311_signingkey(struct ksmbd_session *sess,
>                                 struct ksmbd_conn *conn);
> -int ksmbd_gen_smb30_encryptionkey(struct ksmbd_session *sess);
> -int ksmbd_gen_smb311_encryptionkey(struct ksmbd_session *sess);
> +int ksmbd_gen_smb30_encryptionkey(struct ksmbd_conn *conn,
> +                                 struct ksmbd_session *sess);
> +int ksmbd_gen_smb311_encryptionkey(struct ksmbd_conn *conn,
> +                                  struct ksmbd_session *sess);
>  int ksmbd_gen_preauth_integrity_hash(struct ksmbd_conn *conn, char *buf,
>                                      __u8 *pi_hash);
>  int ksmbd_gen_sd_hash(struct ksmbd_conn *conn, char *sd_buf, int len,
> diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h
> index 2e4730457c92..e7f7d5707951 100644
> --- a/fs/ksmbd/connection.h
> +++ b/fs/ksmbd/connection.h
> @@ -20,13 +20,6 @@
>
>  #define KSMBD_SOCKET_BACKLOG           16
>
> -/*
> - * WARNING
> - *
> - * This is nothing but a HACK. Session status should move to channel
> - * or to session. As of now we have 1 tcp_conn : 1 ksmbd_session, but
> - * we need to change it to 1 tcp_conn : N ksmbd_sessions.
> - */
>  enum {
>         KSMBD_SESS_NEW = 0,
>         KSMBD_SESS_GOOD,
> diff --git a/fs/ksmbd/mgmt/tree_connect.c b/fs/ksmbd/mgmt/tree_connect.c
> index 0d28e723a28c..b35ea6a6abc5 100644
> --- a/fs/ksmbd/mgmt/tree_connect.c
> +++ b/fs/ksmbd/mgmt/tree_connect.c
> @@ -16,7 +16,8 @@
>  #include "user_session.h"
>
>  struct ksmbd_tree_conn_status
> -ksmbd_tree_conn_connect(struct ksmbd_session *sess, char *share_name)
> +ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session *sess,
> +                       char *share_name)
>  {
>         struct ksmbd_tree_conn_status status = {-EINVAL, NULL};
>         struct ksmbd_tree_connect_response *resp = NULL;
> @@ -41,7 +42,7 @@ ksmbd_tree_conn_connect(struct ksmbd_session *sess, char *share_name)
>                 goto out_error;
>         }
>
> -       peer_addr = KSMBD_TCP_PEER_SOCKADDR(sess->conn);
> +       peer_addr = KSMBD_TCP_PEER_SOCKADDR(conn);
>         resp = ksmbd_ipc_tree_connect_request(sess,
>                                               sc,
>                                               tree_conn,
> diff --git a/fs/ksmbd/mgmt/tree_connect.h b/fs/ksmbd/mgmt/tree_connect.h
> index 18e2a996e0aa..71e50271dccf 100644
> --- a/fs/ksmbd/mgmt/tree_connect.h
> +++ b/fs/ksmbd/mgmt/tree_connect.h
> @@ -12,6 +12,7 @@
>
>  struct ksmbd_share_config;
>  struct ksmbd_user;
> +struct ksmbd_conn;
>
>  struct ksmbd_tree_connect {
>         int                             id;
> @@ -40,7 +41,8 @@ static inline int test_tree_conn_flag(struct ksmbd_tree_connect *tree_conn,
>  struct ksmbd_session;
>
>  struct ksmbd_tree_conn_status
> -ksmbd_tree_conn_connect(struct ksmbd_session *sess, char *share_name);
> +ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session *sess,
> +                       char *share_name);
>
>  int ksmbd_tree_conn_disconnect(struct ksmbd_session *sess,
>                                struct ksmbd_tree_connect *tree_conn);
> diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c
> index b9acb6770b03..ce32fdd66807 100644
> --- a/fs/ksmbd/mgmt/user_session.c
> +++ b/fs/ksmbd/mgmt/user_session.c
> @@ -151,9 +151,6 @@ void ksmbd_session_destroy(struct ksmbd_session *sess)
>         if (!sess)
>                 return;
>
> -       if (!atomic_dec_and_test(&sess->refcnt))
> -               return;
> -
>         down_write(&sessions_table_lock);
>         hash_del(&sess->hlist);
>         up_write(&sessions_table_lock);
> @@ -184,16 +181,59 @@ static struct ksmbd_session *__session_lookup(unsigned long long id)
>  int ksmbd_session_register(struct ksmbd_conn *conn,
>                            struct ksmbd_session *sess)
>  {
> -       sess->conn = conn;
> +       sess->dialect = conn->dialect;
> +       memcpy(sess->ClientGUID, conn->ClientGUID, SMB2_CLIENT_GUID_SIZE);
>         return xa_err(xa_store(&conn->sessions, sess->id, sess, GFP_KERNEL));
>  }
>
> +static int ksmbd_chann_del(struct ksmbd_conn *conn, struct ksmbd_session *sess)
> +{
> +       struct channel *chann, *tmp;
> +
> +       write_lock(&sess->chann_lock);
> +       list_for_each_entry_safe(chann, tmp, &sess->ksmbd_chann_list,
> +                                chann_list) {
> +               if (chann->conn == conn) {
> +                       list_del(&chann->chann_list);
> +                       kfree(chann);
> +                       write_unlock(&sess->chann_lock);
> +                       return 0;
> +               }
> +       }
> +       write_unlock(&sess->chann_lock);
> +
> +       return -ENOENT;
> +}
> +

Can we delete free_channel_list() because ksmbd_sessions_deregister()
frees all channels by this function.

>  void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
>  {
>         struct ksmbd_session *sess;
> -       unsigned long id;
>
> -       xa_for_each(&conn->sessions, id, sess) {
> +       if (conn->binding) {
> +               struct hlist_node *tmp;
> +               int bkt;
> +
> +               down_write(&sessions_table_lock);
> +               hash_for_each_safe(sessions_table, bkt, tmp, sess, hlist) {

I think hash_for_each is enough instead of hash_for_each_safe.

> +                       if (!ksmbd_chann_del(conn, sess)) {
> +                               up_write(&sessions_table_lock);
> +                               goto sess_destroy;
> +                       }
> +               }
> +               up_write(&sessions_table_lock);
> +       } else {
> +               unsigned long id;
> +
> +               xa_for_each(&conn->sessions, id, sess) {
> +                       if (!ksmbd_chann_del(conn, sess))
> +                               goto sess_destroy;
> +               }
> +       }
> +
> +       return;
> +
> +sess_destroy:
> +       if (list_empty(&sess->ksmbd_chann_list)) {
>                 xa_erase(&conn->sessions, sess->id);
>                 ksmbd_session_destroy(sess);
>         }
> @@ -205,27 +245,12 @@ struct ksmbd_session *ksmbd_session_lookup(struct ksmbd_conn *conn,
>         return xa_load(&conn->sessions, id);
>  }
>
> -int get_session(struct ksmbd_session *sess)
> -{
> -       return atomic_inc_not_zero(&sess->refcnt);
> -}
> -
> -void put_session(struct ksmbd_session *sess)
> -{
> -       if (atomic_dec_and_test(&sess->refcnt))
> -               pr_err("get/%s seems to be mismatched.", __func__);
> -}
> -
>  struct ksmbd_session *ksmbd_session_lookup_slowpath(unsigned long long id)
>  {
>         struct ksmbd_session *sess;
>
>         down_read(&sessions_table_lock);
>         sess = __session_lookup(id);
> -       if (sess) {
> -               if (!get_session(sess))
> -                       sess = NULL;
> -       }
>         up_read(&sessions_table_lock);
>
>         return sess;
> @@ -306,7 +331,6 @@ static struct ksmbd_session *__session_create(int protocol)
>         INIT_LIST_HEAD(&sess->ksmbd_chann_list);
>         INIT_LIST_HEAD(&sess->rpc_handle_list);
>         sess->sequence_number = 1;
> -       atomic_set(&sess->refcnt, 1);
>         rwlock_init(&sess->chann_lock);
>
>         switch (protocol) {
> diff --git a/fs/ksmbd/mgmt/user_session.h b/fs/ksmbd/mgmt/user_session.h
> index 1ec659f0151b..8934b8ee275b 100644
> --- a/fs/ksmbd/mgmt/user_session.h
> +++ b/fs/ksmbd/mgmt/user_session.h
> @@ -33,8 +33,10 @@ struct preauth_session {
>  struct ksmbd_session {
>         u64                             id;
>
> +       __u16                           dialect;
> +       char                            ClientGUID[SMB2_CLIENT_GUID_SIZE];
> +
>         struct ksmbd_user               *user;
> -       struct ksmbd_conn               *conn;
>         unsigned int                    sequence_number;
>         unsigned int                    flags;
>
> @@ -59,7 +61,6 @@ struct ksmbd_session {
>         __u8                            smb3signingkey[SMB3_SIGN_KEY_SIZE];
>
>         struct ksmbd_file_table         file_table;
> -       atomic_t                        refcnt;
>  };
>
>  static inline int test_session_flag(struct ksmbd_session *sess, int bit)
> @@ -100,6 +101,4 @@ void ksmbd_release_tree_conn_id(struct ksmbd_session *sess, int id);
>  int ksmbd_session_rpc_open(struct ksmbd_session *sess, char *rpc_name);
>  void ksmbd_session_rpc_close(struct ksmbd_session *sess, int id);
>  int ksmbd_session_rpc_method(struct ksmbd_session *sess, int id);
> -int get_session(struct ksmbd_session *sess);
> -void put_session(struct ksmbd_session *sess);
>  #endif /* __USER_SESSION_MANAGEMENT_H__ */
> diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
> index 9bb4fb8b80de..ce4c15c3554d 100644
> --- a/fs/ksmbd/oplock.c
> +++ b/fs/ksmbd/oplock.c
> @@ -30,6 +30,7 @@ static DEFINE_RWLOCK(lease_list_lock);
>  static struct oplock_info *alloc_opinfo(struct ksmbd_work *work,
>                                         u64 id, __u16 Tid)
>  {
> +       struct ksmbd_conn *conn = work->conn;
>         struct ksmbd_session *sess = work->sess;
>         struct oplock_info *opinfo;
>
> @@ -38,7 +39,7 @@ static struct oplock_info *alloc_opinfo(struct ksmbd_work *work,
>                 return NULL;
>
>         opinfo->sess = sess;
> -       opinfo->conn = sess->conn;
> +       opinfo->conn = conn;
>         opinfo->level = SMB2_OPLOCK_LEVEL_NONE;
>         opinfo->op_state = OPLOCK_STATE_NONE;
>         opinfo->pending_break = 0;
> @@ -971,7 +972,7 @@ int find_same_lease_key(struct ksmbd_session *sess, struct ksmbd_inode *ci,
>         }
>
>         list_for_each_entry(lb, &lease_table_list, l_entry) {
> -               if (!memcmp(lb->client_guid, sess->conn->ClientGUID,
> +               if (!memcmp(lb->client_guid, sess->ClientGUID,
>                             SMB2_CLIENT_GUID_SIZE))
>                         goto found;
>         }
> @@ -987,7 +988,7 @@ int find_same_lease_key(struct ksmbd_session *sess, struct ksmbd_inode *ci,
>                 rcu_read_unlock();
>                 if (opinfo->o_fp->f_ci == ci)
>                         goto op_next;
> -               err = compare_guid_key(opinfo, sess->conn->ClientGUID,
> +               err = compare_guid_key(opinfo, sess->ClientGUID,
>                                        lctx->lease_key);
>                 if (err) {
>                         err = -EINVAL;
> @@ -1121,7 +1122,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
>                 struct oplock_info *m_opinfo;
>
>                 /* is lease already granted ? */
> -               m_opinfo = same_client_has_lease(ci, sess->conn->ClientGUID,
> +               m_opinfo = same_client_has_lease(ci, sess->ClientGUID,
>                                                  lctx);
>                 if (m_opinfo) {
>                         copy_lease(m_opinfo, opinfo);
> @@ -1239,7 +1240,7 @@ void smb_break_all_levII_oplock(struct ksmbd_work *work, struct ksmbd_file *fp,
>  {
>         struct oplock_info *op, *brk_op;
>         struct ksmbd_inode *ci;
> -       struct ksmbd_conn *conn = work->sess->conn;
> +       struct ksmbd_conn *conn = work->conn;
>
>         if (!test_share_config_flag(work->tcon->share_conf,
>                                     KSMBD_SHARE_FLAG_OPLOCKS))
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index ae5677a66cb2..1f4f2d5217a6 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -603,12 +603,9 @@ static void destroy_previous_session(struct ksmbd_conn *conn,
>         if (!prev_user ||
>             strcmp(user->name, prev_user->name) ||
>             user->passkey_sz != prev_user->passkey_sz ||
> -           memcmp(user->passkey, prev_user->passkey, user->passkey_sz)) {
> -               put_session(prev_sess);
> +           memcmp(user->passkey, prev_user->passkey, user->passkey_sz))
>                 return;
> -       }
>
> -       put_session(prev_sess);
>         prev_sess->state = SMB2_SESSION_EXPIRED;
>         write_lock(&prev_sess->chann_lock);
>         list_for_each_entry(chann, &prev_sess->ksmbd_chann_list, chann_list)
> @@ -1499,7 +1496,7 @@ static int ntlm_authenticate(struct ksmbd_work *work)
>
>         if (smb3_encryption_negotiated(conn) &&
>                         !(req->Flags & SMB2_SESSION_REQ_FLAG_BINDING)) {
> -               rc = conn->ops->generate_encryptionkey(sess);
> +               rc = conn->ops->generate_encryptionkey(conn, sess);
>                 if (rc) {
>                         ksmbd_debug(SMB,
>                                         "SMB3 encryption key generation failed\n");
> @@ -1590,7 +1587,7 @@ static int krb5_authenticate(struct ksmbd_work *work)
>                 sess->sign = true;
>
>         if (smb3_encryption_negotiated(conn)) {
> -               retval = conn->ops->generate_encryptionkey(sess);
> +               retval = conn->ops->generate_encryptionkey(conn, sess);
>                 if (retval) {
>                         ksmbd_debug(SMB,
>                                     "SMB3 encryption key generation failed\n");
> @@ -1678,7 +1675,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
>                         goto out_err;
>                 }
>
> -               if (conn->dialect != sess->conn->dialect) {
> +               if (conn->dialect != sess->dialect) {
>                         rc = -EINVAL;
>                         goto out_err;
>                 }
> @@ -1688,7 +1685,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
>                         goto out_err;
>                 }
>
> -               if (strncmp(conn->ClientGUID, sess->conn->ClientGUID,
> +               if (strncmp(conn->ClientGUID, sess->ClientGUID,
>                             SMB2_CLIENT_GUID_SIZE)) {
>                         rc = -ENOENT;
>                         goto out_err;
> @@ -1890,7 +1887,7 @@ int smb2_tree_connect(struct ksmbd_work *work)
>         ksmbd_debug(SMB, "tree connect request for tree %s treename %s\n",
>                     name, treename);
>
> -       status = ksmbd_tree_conn_connect(sess, name);
> +       status = ksmbd_tree_conn_connect(conn, sess, name);
>         if (status.ret == KSMBD_TREE_CONN_STATUS_OK)
>                 rsp->hdr.Id.SyncId.TreeId = cpu_to_le32(status.tree_conn->id);
>         else
> @@ -4875,7 +4872,7 @@ static int smb2_get_info_filesystem(struct ksmbd_work *work,
>                                     struct smb2_query_info_rsp *rsp)
>  {
>         struct ksmbd_session *sess = work->sess;
> -       struct ksmbd_conn *conn = sess->conn;
> +       struct ksmbd_conn *conn = work->conn;
>         struct ksmbd_share_config *share = work->tcon->share_conf;
>         int fsinfoclass = 0;
>         struct kstatfs stfs;
> @@ -5793,7 +5790,7 @@ static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp,
>         }
>  next:
>         return smb2_rename(work, fp, user_ns, rename_info,
> -                          work->sess->conn->local_nls);
> +                          work->conn->local_nls);
>  }
>
>  static int set_file_disposition_info(struct ksmbd_file *fp,
> @@ -5925,7 +5922,7 @@ static int smb2_set_info_file(struct ksmbd_work *work, struct ksmbd_file *fp,
>                 return smb2_create_link(work, work->tcon->share_conf,
>                                         (struct smb2_file_link_info *)req->Buffer,
>                                         buf_len, fp->filp,
> -                                       work->sess->conn->local_nls);
> +                                       work->conn->local_nls);
>         }
>         case FILE_DISPOSITION_INFORMATION:
>         {
> diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h
> index e1369b4345a9..318c16fa81da 100644
> --- a/fs/ksmbd/smb_common.h
> +++ b/fs/ksmbd/smb_common.h
> @@ -421,7 +421,7 @@ struct smb_version_ops {
>         int (*check_sign_req)(struct ksmbd_work *work);
>         void (*set_sign_rsp)(struct ksmbd_work *work);
>         int (*generate_signingkey)(struct ksmbd_session *sess, struct ksmbd_conn *conn);
> -       int (*generate_encryptionkey)(struct ksmbd_session *sess);
> +       int (*generate_encryptionkey)(struct ksmbd_conn *conn, struct ksmbd_session *sess);
>         bool (*is_transform_hdr)(void *buf);
>         int (*decrypt_req)(struct ksmbd_work *work);
>         int (*encrypt_resp)(struct ksmbd_work *work);
> diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c
> index 5d185564aef6..4f75b1436eab 100644
> --- a/fs/ksmbd/vfs.c
> +++ b/fs/ksmbd/vfs.c
> @@ -481,12 +481,11 @@ int ksmbd_vfs_write(struct ksmbd_work *work, struct ksmbd_file *fp,
>                     char *buf, size_t count, loff_t *pos, bool sync,
>                     ssize_t *written)
>  {
> -       struct ksmbd_session *sess = work->sess;
>         struct file *filp;
>         loff_t  offset = *pos;
>         int err = 0;
>
> -       if (sess->conn->connection_type) {
> +       if (work->conn->connection_type) {
>                 if (!(fp->daccess & FILE_WRITE_DATA_LE)) {
>                         pr_err("no right to write(%pd)\n",
>                                fp->filp->f_path.dentry);
> diff --git a/fs/ksmbd/vfs_cache.c b/fs/ksmbd/vfs_cache.c
> index c4d59d2735f0..da9163b00350 100644
> --- a/fs/ksmbd/vfs_cache.c
> +++ b/fs/ksmbd/vfs_cache.c
> @@ -569,7 +569,7 @@ struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp)
>         atomic_set(&fp->refcount, 1);
>
>         fp->filp                = filp;
> -       fp->conn                = work->sess->conn;
> +       fp->conn                = work->conn;
>         fp->tcon                = work->tcon;
>         fp->volatile_id         = KSMBD_NO_FID;
>         fp->persistent_id       = KSMBD_NO_FID;
> --
> 2.25.1
>


-- 
Thanks,
Hyunchul

Re: [PATCH 5/5] ksmbd: fix racy issue while destroying session on multichannel

[Namjae Jeon]

2022-07-25 9:56 GMT+09:00, Hyunchul Lee <hyc.lee@gmail.com>:
> 2022년 7월 22일 (금) 오후 12:04, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>>
>> After multi-channel connection with windows, Several channels of
>> session are connected. Among them, if there is a problem in one channel,
>> Windows connects again after disconnecting the channel. In this process,
>> the session is released and a kernel oop can occurs while processing
>> requests to other channels. When the channel is disconnected, if other
>> channels still exist in the session after deleting the channel from
>> the channel list in the session, the session should not be released.
>> Finally, the session will be released after all channels are
>> disconnected.
>>
>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>> ---
>>  fs/ksmbd/auth.c              | 56 ++++++++++++++++-------------
>>  fs/ksmbd/auth.h              | 11 +++---
>>  fs/ksmbd/connection.h        |  7 ----
>>  fs/ksmbd/mgmt/tree_connect.c |  5 +--
>>  fs/ksmbd/mgmt/tree_connect.h |  4 ++-
>>  fs/ksmbd/mgmt/user_session.c | 68 ++++++++++++++++++++++++------------
>>  fs/ksmbd/mgmt/user_session.h |  7 ++--
>>  fs/ksmbd/oplock.c            | 11 +++---
>>  fs/ksmbd/smb2pdu.c           | 21 +++++------
>>  fs/ksmbd/smb_common.h        |  2 +-
>>  fs/ksmbd/vfs.c               |  3 +-
>>  fs/ksmbd/vfs_cache.c         |  2 +-
>>  12 files changed, 111 insertions(+), 86 deletions(-)
>>
>> diff --git a/fs/ksmbd/auth.c b/fs/ksmbd/auth.c
>> index 911444d21267..c5a5c7b90d72 100644
>> --- a/fs/ksmbd/auth.c
>> +++ b/fs/ksmbd/auth.c
>> @@ -121,8 +121,8 @@ static int ksmbd_gen_sess_key(struct ksmbd_session
>> *sess, char *hash,
>>         return rc;
>>  }
>>
>> -static int calc_ntlmv2_hash(struct ksmbd_session *sess, char
>> *ntlmv2_hash,
>> -                           char *dname)
>> +static int calc_ntlmv2_hash(struct ksmbd_conn *conn, struct ksmbd_session
>> *sess,
>> +                           char *ntlmv2_hash, char *dname)
>>  {
>>         int ret, len, conv_len;
>>         wchar_t *domain = NULL;
>> @@ -158,7 +158,7 @@ static int calc_ntlmv2_hash(struct ksmbd_session
>> *sess, char *ntlmv2_hash,
>>         }
>>
>>         conv_len = smb_strtoUTF16(uniname, user_name(sess->user), len,
>> -                                 sess->conn->local_nls);
>> +                                 conn->local_nls);
>>         if (conv_len < 0 || conv_len > len) {
>>                 ret = -EINVAL;
>>                 goto out;
>> @@ -182,7 +182,7 @@ static int calc_ntlmv2_hash(struct ksmbd_session
>> *sess, char *ntlmv2_hash,
>>         }
>>
>>         conv_len = smb_strtoUTF16((__le16 *)domain, dname, len,
>> -                                 sess->conn->local_nls);
>> +                                 conn->local_nls);
>>         if (conv_len < 0 || conv_len > len) {
>>                 ret = -EINVAL;
>>                 goto out;
>> @@ -215,8 +215,9 @@ static int calc_ntlmv2_hash(struct ksmbd_session
>> *sess, char *ntlmv2_hash,
>>   *
>>   * Return:     0 on success, error number on error
>>   */
>> -int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp
>> *ntlmv2,
>> -                     int blen, char *domain_name, char *cryptkey)
>> +int ksmbd_auth_ntlmv2(struct ksmbd_conn *conn, struct ksmbd_session
>> *sess,
>> +                     struct ntlmv2_resp *ntlmv2, int blen, char
>> *domain_name,
>> +                     char *cryptkey)
>>  {
>>         char ntlmv2_hash[CIFS_ENCPWD_SIZE];
>>         char ntlmv2_rsp[CIFS_HMAC_MD5_HASH_SIZE];
>> @@ -230,7 +231,7 @@ int ksmbd_auth_ntlmv2(struct ksmbd_session *sess,
>> struct ntlmv2_resp *ntlmv2,
>>                 return -ENOMEM;
>>         }
>>
>> -       rc = calc_ntlmv2_hash(sess, ntlmv2_hash, domain_name);
>> +       rc = calc_ntlmv2_hash(conn, sess, ntlmv2_hash, domain_name);
>>         if (rc) {
>>                 ksmbd_debug(AUTH, "could not get v2 hash rc %d\n", rc);
>>                 goto out;
>> @@ -333,7 +334,8 @@ int ksmbd_decode_ntlmssp_auth_blob(struct
>> authenticate_message *authblob,
>>         /* process NTLMv2 authentication */
>>         ksmbd_debug(AUTH, "decode_ntlmssp_authenticate_blob dname%s\n",
>>                     domain_name);
>> -       ret = ksmbd_auth_ntlmv2(sess, (struct ntlmv2_resp *)((char
>> *)authblob + nt_off),
>> +       ret = ksmbd_auth_ntlmv2(conn, sess,
>> +                               (struct ntlmv2_resp *)((char *)authblob +
>> nt_off),
>>                                 nt_len - CIFS_ENCPWD_SIZE,
>>                                 domain_name, conn->ntlmssp.cryptkey);
>>         kfree(domain_name);
>> @@ -659,8 +661,9 @@ struct derivation {
>>         bool binding;
>>  };
>>
>> -static int generate_key(struct ksmbd_session *sess, struct kvec label,
>> -                       struct kvec context, __u8 *key, unsigned int
>> key_size)
>> +static int generate_key(struct ksmbd_conn *conn, struct ksmbd_session
>> *sess,
>> +                       struct kvec label, struct kvec context, __u8
>> *key,
>> +                       unsigned int key_size)
>>  {
>>         unsigned char zero = 0x0;
>>         __u8 i[4] = {0, 0, 0, 1};
>> @@ -720,8 +723,8 @@ static int generate_key(struct ksmbd_session *sess,
>> struct kvec label,
>>                 goto smb3signkey_ret;
>>         }
>>
>> -       if (sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
>> -           sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM)
>> +       if (conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
>> +           conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM)
>>                 rc = crypto_shash_update(CRYPTO_HMACSHA256(ctx), L256,
>> 4);
>>         else
>>                 rc = crypto_shash_update(CRYPTO_HMACSHA256(ctx), L128,
>> 4);
>> @@ -756,17 +759,17 @@ static int generate_smb3signingkey(struct
>> ksmbd_session *sess,
>>         if (!chann)
>>                 return 0;
>>
>> -       if (sess->conn->dialect >= SMB30_PROT_ID && signing->binding)
>> +       if (conn->dialect >= SMB30_PROT_ID && signing->binding)
>>                 key = chann->smb3signingkey;
>>         else
>>                 key = sess->smb3signingkey;
>>
>> -       rc = generate_key(sess, signing->label, signing->context, key,
>> +       rc = generate_key(conn, sess, signing->label, signing->context,
>> key,
>>                           SMB3_SIGN_KEY_SIZE);
>>         if (rc)
>>                 return rc;
>>
>> -       if (!(sess->conn->dialect >= SMB30_PROT_ID && signing->binding))
>> +       if (!(conn->dialect >= SMB30_PROT_ID && signing->binding))
>>                 memcpy(chann->smb3signingkey, key, SMB3_SIGN_KEY_SIZE);
>>
>>         ksmbd_debug(AUTH, "dumping generated AES signing keys\n");
>> @@ -820,30 +823,31 @@ struct derivation_twin {
>>         struct derivation decryption;
>>  };
>>
>> -static int generate_smb3encryptionkey(struct ksmbd_session *sess,
>> +static int generate_smb3encryptionkey(struct ksmbd_conn *conn,
>> +                                     struct ksmbd_session *sess,
>>                                       const struct derivation_twin
>> *ptwin)
>>  {
>>         int rc;
>>
>> -       rc = generate_key(sess, ptwin->encryption.label,
>> +       rc = generate_key(conn, sess, ptwin->encryption.label,
>>                           ptwin->encryption.context,
>> sess->smb3encryptionkey,
>>                           SMB3_ENC_DEC_KEY_SIZE);
>>         if (rc)
>>                 return rc;
>>
>> -       rc = generate_key(sess, ptwin->decryption.label,
>> +       rc = generate_key(conn, sess, ptwin->decryption.label,
>>                           ptwin->decryption.context,
>>                           sess->smb3decryptionkey,
>> SMB3_ENC_DEC_KEY_SIZE);
>>         if (rc)
>>                 return rc;
>>
>>         ksmbd_debug(AUTH, "dumping generated AES encryption keys\n");
>> -       ksmbd_debug(AUTH, "Cipher type   %d\n", sess->conn->cipher_type);
>> +       ksmbd_debug(AUTH, "Cipher type   %d\n", conn->cipher_type);
>>         ksmbd_debug(AUTH, "Session Id    %llu\n", sess->id);
>>         ksmbd_debug(AUTH, "Session Key   %*ph\n",
>>                     SMB2_NTLMV2_SESSKEY_SIZE, sess->sess_key);
>> -       if (sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
>> -           sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM) {
>> +       if (conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM ||
>> +           conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM) {
>>                 ksmbd_debug(AUTH, "ServerIn Key  %*ph\n",
>>                             SMB3_GCM256_CRYPTKEY_SIZE,
>> sess->smb3encryptionkey);
>>                 ksmbd_debug(AUTH, "ServerOut Key %*ph\n",
>> @@ -857,7 +861,8 @@ static int generate_smb3encryptionkey(struct
>> ksmbd_session *sess,
>>         return 0;
>>  }
>>
>> -int ksmbd_gen_smb30_encryptionkey(struct ksmbd_session *sess)
>> +int ksmbd_gen_smb30_encryptionkey(struct ksmbd_conn *conn,
>> +                                 struct ksmbd_session *sess)
>>  {
>>         struct derivation_twin twin;
>>         struct derivation *d;
>> @@ -874,10 +879,11 @@ int ksmbd_gen_smb30_encryptionkey(struct
>> ksmbd_session *sess)
>>         d->context.iov_base = "ServerIn ";
>>         d->context.iov_len = 10;
>>
>> -       return generate_smb3encryptionkey(sess, &twin);
>> +       return generate_smb3encryptionkey(conn, sess, &twin);
>>  }
>>
>> -int ksmbd_gen_smb311_encryptionkey(struct ksmbd_session *sess)
>> +int ksmbd_gen_smb311_encryptionkey(struct ksmbd_conn *conn,
>> +                                  struct ksmbd_session *sess)
>>  {
>>         struct derivation_twin twin;
>>         struct derivation *d;
>> @@ -894,7 +900,7 @@ int ksmbd_gen_smb311_encryptionkey(struct
>> ksmbd_session *sess)
>>         d->context.iov_base = sess->Preauth_HashValue;
>>         d->context.iov_len = 64;
>>
>> -       return generate_smb3encryptionkey(sess, &twin);
>> +       return generate_smb3encryptionkey(conn, sess, &twin);
>>  }
>>
>>  int ksmbd_gen_preauth_integrity_hash(struct ksmbd_conn *conn, char *buf,
>> diff --git a/fs/ksmbd/auth.h b/fs/ksmbd/auth.h
>> index 95629651cf26..25b772653de0 100644
>> --- a/fs/ksmbd/auth.h
>> +++ b/fs/ksmbd/auth.h
>> @@ -38,8 +38,9 @@ struct kvec;
>>  int ksmbd_crypt_message(struct ksmbd_conn *conn, struct kvec *iov,
>>                         unsigned int nvec, int enc);
>>  void ksmbd_copy_gss_neg_header(void *buf);
>> -int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp
>> *ntlmv2,
>> -                     int blen, char *domain_name, char *cryptkey);
>> +int ksmbd_auth_ntlmv2(struct ksmbd_conn *conn, struct ksmbd_session
>> *sess,
>> +                     struct ntlmv2_resp *ntlmv2, int blen, char
>> *domain_name,
>> +                     char *cryptkey);
>>  int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message
>> *authblob,
>>                                    int blob_len, struct ksmbd_conn *conn,
>>                                    struct ksmbd_session *sess);
>> @@ -58,8 +59,10 @@ int ksmbd_gen_smb30_signingkey(struct ksmbd_session
>> *sess,
>>                                struct ksmbd_conn *conn);
>>  int ksmbd_gen_smb311_signingkey(struct ksmbd_session *sess,
>>                                 struct ksmbd_conn *conn);
>> -int ksmbd_gen_smb30_encryptionkey(struct ksmbd_session *sess);
>> -int ksmbd_gen_smb311_encryptionkey(struct ksmbd_session *sess);
>> +int ksmbd_gen_smb30_encryptionkey(struct ksmbd_conn *conn,
>> +                                 struct ksmbd_session *sess);
>> +int ksmbd_gen_smb311_encryptionkey(struct ksmbd_conn *conn,
>> +                                  struct ksmbd_session *sess);
>>  int ksmbd_gen_preauth_integrity_hash(struct ksmbd_conn *conn, char *buf,
>>                                      __u8 *pi_hash);
>>  int ksmbd_gen_sd_hash(struct ksmbd_conn *conn, char *sd_buf, int len,
>> diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h
>> index 2e4730457c92..e7f7d5707951 100644
>> --- a/fs/ksmbd/connection.h
>> +++ b/fs/ksmbd/connection.h
>> @@ -20,13 +20,6 @@
>>
>>  #define KSMBD_SOCKET_BACKLOG           16
>>
>> -/*
>> - * WARNING
>> - *
>> - * This is nothing but a HACK. Session status should move to channel
>> - * or to session. As of now we have 1 tcp_conn : 1 ksmbd_session, but
>> - * we need to change it to 1 tcp_conn : N ksmbd_sessions.
>> - */
>>  enum {
>>         KSMBD_SESS_NEW = 0,
>>         KSMBD_SESS_GOOD,
>> diff --git a/fs/ksmbd/mgmt/tree_connect.c b/fs/ksmbd/mgmt/tree_connect.c
>> index 0d28e723a28c..b35ea6a6abc5 100644
>> --- a/fs/ksmbd/mgmt/tree_connect.c
>> +++ b/fs/ksmbd/mgmt/tree_connect.c
>> @@ -16,7 +16,8 @@
>>  #include "user_session.h"
>>
>>  struct ksmbd_tree_conn_status
>> -ksmbd_tree_conn_connect(struct ksmbd_session *sess, char *share_name)
>> +ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session
>> *sess,
>> +                       char *share_name)
>>  {
>>         struct ksmbd_tree_conn_status status = {-EINVAL, NULL};
>>         struct ksmbd_tree_connect_response *resp = NULL;
>> @@ -41,7 +42,7 @@ ksmbd_tree_conn_connect(struct ksmbd_session *sess, char
>> *share_name)
>>                 goto out_error;
>>         }
>>
>> -       peer_addr = KSMBD_TCP_PEER_SOCKADDR(sess->conn);
>> +       peer_addr = KSMBD_TCP_PEER_SOCKADDR(conn);
>>         resp = ksmbd_ipc_tree_connect_request(sess,
>>                                               sc,
>>                                               tree_conn,
>> diff --git a/fs/ksmbd/mgmt/tree_connect.h b/fs/ksmbd/mgmt/tree_connect.h
>> index 18e2a996e0aa..71e50271dccf 100644
>> --- a/fs/ksmbd/mgmt/tree_connect.h
>> +++ b/fs/ksmbd/mgmt/tree_connect.h
>> @@ -12,6 +12,7 @@
>>
>>  struct ksmbd_share_config;
>>  struct ksmbd_user;
>> +struct ksmbd_conn;
>>
>>  struct ksmbd_tree_connect {
>>         int                             id;
>> @@ -40,7 +41,8 @@ static inline int test_tree_conn_flag(struct
>> ksmbd_tree_connect *tree_conn,
>>  struct ksmbd_session;
>>
>>  struct ksmbd_tree_conn_status
>> -ksmbd_tree_conn_connect(struct ksmbd_session *sess, char *share_name);
>> +ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session
>> *sess,
>> +                       char *share_name);
>>
>>  int ksmbd_tree_conn_disconnect(struct ksmbd_session *sess,
>>                                struct ksmbd_tree_connect *tree_conn);
>> diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c
>> index b9acb6770b03..ce32fdd66807 100644
>> --- a/fs/ksmbd/mgmt/user_session.c
>> +++ b/fs/ksmbd/mgmt/user_session.c
>> @@ -151,9 +151,6 @@ void ksmbd_session_destroy(struct ksmbd_session
>> *sess)
>>         if (!sess)
>>                 return;
>>
>> -       if (!atomic_dec_and_test(&sess->refcnt))
>> -               return;
>> -
>>         down_write(&sessions_table_lock);
>>         hash_del(&sess->hlist);
>>         up_write(&sessions_table_lock);
>> @@ -184,16 +181,59 @@ static struct ksmbd_session
>> *__session_lookup(unsigned long long id)
>>  int ksmbd_session_register(struct ksmbd_conn *conn,
>>                            struct ksmbd_session *sess)
>>  {
>> -       sess->conn = conn;
>> +       sess->dialect = conn->dialect;
>> +       memcpy(sess->ClientGUID, conn->ClientGUID,
>> SMB2_CLIENT_GUID_SIZE);
>>         return xa_err(xa_store(&conn->sessions, sess->id, sess,
>> GFP_KERNEL));
>>  }
>>
>> +static int ksmbd_chann_del(struct ksmbd_conn *conn, struct ksmbd_session
>> *sess)
>> +{
>> +       struct channel *chann, *tmp;
>> +
>> +       write_lock(&sess->chann_lock);
>> +       list_for_each_entry_safe(chann, tmp, &sess->ksmbd_chann_list,
>> +                                chann_list) {
>> +               if (chann->conn == conn) {
>> +                       list_del(&chann->chann_list);
>> +                       kfree(chann);
>> +                       write_unlock(&sess->chann_lock);
>> +                       return 0;
>> +               }
>> +       }
>> +       write_unlock(&sess->chann_lock);
>> +
>> +       return -ENOENT;
>> +}
>> +
>
> Can we delete free_channel_list() because ksmbd_sessions_deregister()
> frees all channels by this function.
If smb2 session setup fail, session should be freed by ksmbd_session_destroy().
and free_channel_list will delete chann entries in ksmbd_session_destroy().
>
>>  void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
>>  {
>>         struct ksmbd_session *sess;
>> -       unsigned long id;
>>
>> -       xa_for_each(&conn->sessions, id, sess) {
>> +       if (conn->binding) {
>> +               struct hlist_node *tmp;
>> +               int bkt;
>> +
>> +               down_write(&sessions_table_lock);
>> +               hash_for_each_safe(sessions_table, bkt, tmp, sess, hlist)
>> {
>
> I think hash_for_each is enough instead of hash_for_each_safe.
My initial version patch delete session in this loop. Indeed,
hash_for_each() can be used on this version.

Thanks.
>
>> +                       if (!ksmbd_chann_del(conn, sess)) {
>> +                               up_write(&sessions_table_lock);
>> +                               goto sess_destroy;
>> +                       }
>> +               }
>> +               up_write(&sessions_table_lock);
>> +       } else {
>> +               unsigned long id;
>> +
>> +               xa_for_each(&conn->sessions, id, sess) {
>> +                       if (!ksmbd_chann_del(conn, sess))
>> +                               goto sess_destroy;
>> +               }
>> +       }
>> +
>> +       return;
>> +
>> +sess_destroy:
>> +       if (list_empty(&sess->ksmbd_chann_list)) {
>>                 xa_erase(&conn->sessions, sess->id);
>>                 ksmbd_session_destroy(sess);
>>         }
>> @@ -205,27 +245,12 @@ struct ksmbd_session *ksmbd_session_lookup(struct
>> ksmbd_conn *conn,
>>         return xa_load(&conn->sessions, id);
>>  }
>>
>> -int get_session(struct ksmbd_session *sess)
>> -{
>> -       return atomic_inc_not_zero(&sess->refcnt);
>> -}
>> -
>> -void put_session(struct ksmbd_session *sess)
>> -{
>> -       if (atomic_dec_and_test(&sess->refcnt))
>> -               pr_err("get/%s seems to be mismatched.", __func__);
>> -}
>> -
>>  struct ksmbd_session *ksmbd_session_lookup_slowpath(unsigned long long
>> id)
>>  {
>>         struct ksmbd_session *sess;
>>
>>         down_read(&sessions_table_lock);
>>         sess = __session_lookup(id);
>> -       if (sess) {
>> -               if (!get_session(sess))
>> -                       sess = NULL;
>> -       }
>>         up_read(&sessions_table_lock);
>>
>>         return sess;
>> @@ -306,7 +331,6 @@ static struct ksmbd_session *__session_create(int
>> protocol)
>>         INIT_LIST_HEAD(&sess->ksmbd_chann_list);
>>         INIT_LIST_HEAD(&sess->rpc_handle_list);
>>         sess->sequence_number = 1;
>> -       atomic_set(&sess->refcnt, 1);
>>         rwlock_init(&sess->chann_lock);
>>
>>         switch (protocol) {
>> diff --git a/fs/ksmbd/mgmt/user_session.h b/fs/ksmbd/mgmt/user_session.h
>> index 1ec659f0151b..8934b8ee275b 100644
>> --- a/fs/ksmbd/mgmt/user_session.h
>> +++ b/fs/ksmbd/mgmt/user_session.h
>> @@ -33,8 +33,10 @@ struct preauth_session {
>>  struct ksmbd_session {
>>         u64                             id;
>>
>> +       __u16                           dialect;
>> +       char
>> ClientGUID[SMB2_CLIENT_GUID_SIZE];
>> +
>>         struct ksmbd_user               *user;
>> -       struct ksmbd_conn               *conn;
>>         unsigned int                    sequence_number;
>>         unsigned int                    flags;
>>
>> @@ -59,7 +61,6 @@ struct ksmbd_session {
>>         __u8
>> smb3signingkey[SMB3_SIGN_KEY_SIZE];
>>
>>         struct ksmbd_file_table         file_table;
>> -       atomic_t                        refcnt;
>>  };
>>
>>  static inline int test_session_flag(struct ksmbd_session *sess, int bit)
>> @@ -100,6 +101,4 @@ void ksmbd_release_tree_conn_id(struct ksmbd_session
>> *sess, int id);
>>  int ksmbd_session_rpc_open(struct ksmbd_session *sess, char *rpc_name);
>>  void ksmbd_session_rpc_close(struct ksmbd_session *sess, int id);
>>  int ksmbd_session_rpc_method(struct ksmbd_session *sess, int id);
>> -int get_session(struct ksmbd_session *sess);
>> -void put_session(struct ksmbd_session *sess);
>>  #endif /* __USER_SESSION_MANAGEMENT_H__ */
>> diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c
>> index 9bb4fb8b80de..ce4c15c3554d 100644
>> --- a/fs/ksmbd/oplock.c
>> +++ b/fs/ksmbd/oplock.c
>> @@ -30,6 +30,7 @@ static DEFINE_RWLOCK(lease_list_lock);
>>  static struct oplock_info *alloc_opinfo(struct ksmbd_work *work,
>>                                         u64 id, __u16 Tid)
>>  {
>> +       struct ksmbd_conn *conn = work->conn;
>>         struct ksmbd_session *sess = work->sess;
>>         struct oplock_info *opinfo;
>>
>> @@ -38,7 +39,7 @@ static struct oplock_info *alloc_opinfo(struct
>> ksmbd_work *work,
>>                 return NULL;
>>
>>         opinfo->sess = sess;
>> -       opinfo->conn = sess->conn;
>> +       opinfo->conn = conn;
>>         opinfo->level = SMB2_OPLOCK_LEVEL_NONE;
>>         opinfo->op_state = OPLOCK_STATE_NONE;
>>         opinfo->pending_break = 0;
>> @@ -971,7 +972,7 @@ int find_same_lease_key(struct ksmbd_session *sess,
>> struct ksmbd_inode *ci,
>>         }
>>
>>         list_for_each_entry(lb, &lease_table_list, l_entry) {
>> -               if (!memcmp(lb->client_guid, sess->conn->ClientGUID,
>> +               if (!memcmp(lb->client_guid, sess->ClientGUID,
>>                             SMB2_CLIENT_GUID_SIZE))
>>                         goto found;
>>         }
>> @@ -987,7 +988,7 @@ int find_same_lease_key(struct ksmbd_session *sess,
>> struct ksmbd_inode *ci,
>>                 rcu_read_unlock();
>>                 if (opinfo->o_fp->f_ci == ci)
>>                         goto op_next;
>> -               err = compare_guid_key(opinfo, sess->conn->ClientGUID,
>> +               err = compare_guid_key(opinfo, sess->ClientGUID,
>>                                        lctx->lease_key);
>>                 if (err) {
>>                         err = -EINVAL;
>> @@ -1121,7 +1122,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int
>> req_op_level, u64 pid,
>>                 struct oplock_info *m_opinfo;
>>
>>                 /* is lease already granted ? */
>> -               m_opinfo = same_client_has_lease(ci,
>> sess->conn->ClientGUID,
>> +               m_opinfo = same_client_has_lease(ci, sess->ClientGUID,
>>                                                  lctx);
>>                 if (m_opinfo) {
>>                         copy_lease(m_opinfo, opinfo);
>> @@ -1239,7 +1240,7 @@ void smb_break_all_levII_oplock(struct ksmbd_work
>> *work, struct ksmbd_file *fp,
>>  {
>>         struct oplock_info *op, *brk_op;
>>         struct ksmbd_inode *ci;
>> -       struct ksmbd_conn *conn = work->sess->conn;
>> +       struct ksmbd_conn *conn = work->conn;
>>
>>         if (!test_share_config_flag(work->tcon->share_conf,
>>                                     KSMBD_SHARE_FLAG_OPLOCKS))
>> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
>> index ae5677a66cb2..1f4f2d5217a6 100644
>> --- a/fs/ksmbd/smb2pdu.c
>> +++ b/fs/ksmbd/smb2pdu.c
>> @@ -603,12 +603,9 @@ static void destroy_previous_session(struct
>> ksmbd_conn *conn,
>>         if (!prev_user ||
>>             strcmp(user->name, prev_user->name) ||
>>             user->passkey_sz != prev_user->passkey_sz ||
>> -           memcmp(user->passkey, prev_user->passkey, user->passkey_sz))
>> {
>> -               put_session(prev_sess);
>> +           memcmp(user->passkey, prev_user->passkey, user->passkey_sz))
>>                 return;
>> -       }
>>
>> -       put_session(prev_sess);
>>         prev_sess->state = SMB2_SESSION_EXPIRED;
>>         write_lock(&prev_sess->chann_lock);
>>         list_for_each_entry(chann, &prev_sess->ksmbd_chann_list,
>> chann_list)
>> @@ -1499,7 +1496,7 @@ static int ntlm_authenticate(struct ksmbd_work
>> *work)
>>
>>         if (smb3_encryption_negotiated(conn) &&
>>                         !(req->Flags & SMB2_SESSION_REQ_FLAG_BINDING)) {
>> -               rc = conn->ops->generate_encryptionkey(sess);
>> +               rc = conn->ops->generate_encryptionkey(conn, sess);
>>                 if (rc) {
>>                         ksmbd_debug(SMB,
>>                                         "SMB3 encryption key generation
>> failed\n");
>> @@ -1590,7 +1587,7 @@ static int krb5_authenticate(struct ksmbd_work
>> *work)
>>                 sess->sign = true;
>>
>>         if (smb3_encryption_negotiated(conn)) {
>> -               retval = conn->ops->generate_encryptionkey(sess);
>> +               retval = conn->ops->generate_encryptionkey(conn, sess);
>>                 if (retval) {
>>                         ksmbd_debug(SMB,
>>                                     "SMB3 encryption key generation
>> failed\n");
>> @@ -1678,7 +1675,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
>>                         goto out_err;
>>                 }
>>
>> -               if (conn->dialect != sess->conn->dialect) {
>> +               if (conn->dialect != sess->dialect) {
>>                         rc = -EINVAL;
>>                         goto out_err;
>>                 }
>> @@ -1688,7 +1685,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
>>                         goto out_err;
>>                 }
>>
>> -               if (strncmp(conn->ClientGUID, sess->conn->ClientGUID,
>> +               if (strncmp(conn->ClientGUID, sess->ClientGUID,
>>                             SMB2_CLIENT_GUID_SIZE)) {
>>                         rc = -ENOENT;
>>                         goto out_err;
>> @@ -1890,7 +1887,7 @@ int smb2_tree_connect(struct ksmbd_work *work)
>>         ksmbd_debug(SMB, "tree connect request for tree %s treename
>> %s\n",
>>                     name, treename);
>>
>> -       status = ksmbd_tree_conn_connect(sess, name);
>> +       status = ksmbd_tree_conn_connect(conn, sess, name);
>>         if (status.ret == KSMBD_TREE_CONN_STATUS_OK)
>>                 rsp->hdr.Id.SyncId.TreeId =
>> cpu_to_le32(status.tree_conn->id);
>>         else
>> @@ -4875,7 +4872,7 @@ static int smb2_get_info_filesystem(struct
>> ksmbd_work *work,
>>                                     struct smb2_query_info_rsp *rsp)
>>  {
>>         struct ksmbd_session *sess = work->sess;
>> -       struct ksmbd_conn *conn = sess->conn;
>> +       struct ksmbd_conn *conn = work->conn;
>>         struct ksmbd_share_config *share = work->tcon->share_conf;
>>         int fsinfoclass = 0;
>>         struct kstatfs stfs;
>> @@ -5793,7 +5790,7 @@ static int set_rename_info(struct ksmbd_work *work,
>> struct ksmbd_file *fp,
>>         }
>>  next:
>>         return smb2_rename(work, fp, user_ns, rename_info,
>> -                          work->sess->conn->local_nls);
>> +                          work->conn->local_nls);
>>  }
>>
>>  static int set_file_disposition_info(struct ksmbd_file *fp,
>> @@ -5925,7 +5922,7 @@ static int smb2_set_info_file(struct ksmbd_work
>> *work, struct ksmbd_file *fp,
>>                 return smb2_create_link(work, work->tcon->share_conf,
>>                                         (struct smb2_file_link_info
>> *)req->Buffer,
>>                                         buf_len, fp->filp,
>> -                                       work->sess->conn->local_nls);
>> +                                       work->conn->local_nls);
>>         }
>>         case FILE_DISPOSITION_INFORMATION:
>>         {
>> diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h
>> index e1369b4345a9..318c16fa81da 100644
>> --- a/fs/ksmbd/smb_common.h
>> +++ b/fs/ksmbd/smb_common.h
>> @@ -421,7 +421,7 @@ struct smb_version_ops {
>>         int (*check_sign_req)(struct ksmbd_work *work);
>>         void (*set_sign_rsp)(struct ksmbd_work *work);
>>         int (*generate_signingkey)(struct ksmbd_session *sess, struct
>> ksmbd_conn *conn);
>> -       int (*generate_encryptionkey)(struct ksmbd_session *sess);
>> +       int (*generate_encryptionkey)(struct ksmbd_conn *conn, struct
>> ksmbd_session *sess);
>>         bool (*is_transform_hdr)(void *buf);
>>         int (*decrypt_req)(struct ksmbd_work *work);
>>         int (*encrypt_resp)(struct ksmbd_work *work);
>> diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c
>> index 5d185564aef6..4f75b1436eab 100644
>> --- a/fs/ksmbd/vfs.c
>> +++ b/fs/ksmbd/vfs.c
>> @@ -481,12 +481,11 @@ int ksmbd_vfs_write(struct ksmbd_work *work, struct
>> ksmbd_file *fp,
>>                     char *buf, size_t count, loff_t *pos, bool sync,
>>                     ssize_t *written)
>>  {
>> -       struct ksmbd_session *sess = work->sess;
>>         struct file *filp;
>>         loff_t  offset = *pos;
>>         int err = 0;
>>
>> -       if (sess->conn->connection_type) {
>> +       if (work->conn->connection_type) {
>>                 if (!(fp->daccess & FILE_WRITE_DATA_LE)) {
>>                         pr_err("no right to write(%pd)\n",
>>                                fp->filp->f_path.dentry);
>> diff --git a/fs/ksmbd/vfs_cache.c b/fs/ksmbd/vfs_cache.c
>> index c4d59d2735f0..da9163b00350 100644
>> --- a/fs/ksmbd/vfs_cache.c
>> +++ b/fs/ksmbd/vfs_cache.c
>> @@ -569,7 +569,7 @@ struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work
>> *work, struct file *filp)
>>         atomic_set(&fp->refcount, 1);
>>
>>         fp->filp                = filp;
>> -       fp->conn                = work->sess->conn;
>> +       fp->conn                = work->conn;
>>         fp->tcon                = work->tcon;
>>         fp->volatile_id         = KSMBD_NO_FID;
>>         fp->persistent_id       = KSMBD_NO_FID;
>> --
>> 2.25.1
>>
>
>
> --
> Thanks,
> Hyunchul
>

Re: [PATCH 1/5] ksmbd: replace sessions list in connection with xarray

[Hyunchul Lee]

Hi Namjae,

2022년 7월 22일 (금) 오후 12:04, Namjae Jeon <linkinjeon@kernel.org>님이 작성:
>
> Replace sessions list in connection with xarray.
>
> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>

Looks good to me.
Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

> ---
>  fs/ksmbd/connection.c        |  3 ++-
>  fs/ksmbd/connection.h        |  2 +-
>  fs/ksmbd/mgmt/user_session.c | 31 +++++++------------------------
>  fs/ksmbd/mgmt/user_session.h |  5 ++---
>  fs/ksmbd/smb2pdu.c           | 13 +++++++++----
>  5 files changed, 21 insertions(+), 33 deletions(-)
>
> diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c
> index e8f476c5f189..ce23cc89046e 100644
> --- a/fs/ksmbd/connection.c
> +++ b/fs/ksmbd/connection.c
> @@ -36,6 +36,7 @@ void ksmbd_conn_free(struct ksmbd_conn *conn)
>         list_del(&conn->conns_list);
>         write_unlock(&conn_list_lock);
>
> +       xa_destroy(&conn->sessions);
>         kvfree(conn->request_buf);
>         kfree(conn->preauth_info);
>         kfree(conn);
> @@ -66,12 +67,12 @@ struct ksmbd_conn *ksmbd_conn_alloc(void)
>
>         init_waitqueue_head(&conn->req_running_q);
>         INIT_LIST_HEAD(&conn->conns_list);
> -       INIT_LIST_HEAD(&conn->sessions);
>         INIT_LIST_HEAD(&conn->requests);
>         INIT_LIST_HEAD(&conn->async_requests);
>         spin_lock_init(&conn->request_lock);
>         spin_lock_init(&conn->credits_lock);
>         ida_init(&conn->async_ida);
> +       xa_init(&conn->sessions);
>
>         spin_lock_init(&conn->llist_lock);
>         INIT_LIST_HEAD(&conn->lock_list);
> diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h
> index 98c1cbe45ec9..5b39f0bdeff8 100644
> --- a/fs/ksmbd/connection.h
> +++ b/fs/ksmbd/connection.h
> @@ -55,7 +55,7 @@ struct ksmbd_conn {
>         struct nls_table                *local_nls;
>         struct list_head                conns_list;
>         /* smb session 1 per user */
> -       struct list_head                sessions;
> +       struct xarray                   sessions;
>         unsigned long                   last_active;
>         /* How many request are running currently */
>         atomic_t                        req_running;
> diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c
> index 8d8ffd8c6f19..3a44e66456fc 100644
> --- a/fs/ksmbd/mgmt/user_session.c
> +++ b/fs/ksmbd/mgmt/user_session.c
> @@ -152,8 +152,6 @@ void ksmbd_session_destroy(struct ksmbd_session *sess)
>         if (!atomic_dec_and_test(&sess->refcnt))
>                 return;
>
> -       list_del(&sess->sessions_entry);
> -
>         down_write(&sessions_table_lock);
>         hash_del(&sess->hlist);
>         up_write(&sessions_table_lock);
> @@ -181,42 +179,28 @@ static struct ksmbd_session *__session_lookup(unsigned long long id)
>         return NULL;
>  }
>
> -void ksmbd_session_register(struct ksmbd_conn *conn,
> -                           struct ksmbd_session *sess)
> +int ksmbd_session_register(struct ksmbd_conn *conn,
> +                          struct ksmbd_session *sess)
>  {
>         sess->conn = conn;
> -       list_add(&sess->sessions_entry, &conn->sessions);
> +       return xa_err(xa_store(&conn->sessions, sess->id, sess, GFP_KERNEL));
>  }
>
>  void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
>  {
>         struct ksmbd_session *sess;
> +       unsigned long id;
>
> -       while (!list_empty(&conn->sessions)) {
> -               sess = list_entry(conn->sessions.next,
> -                                 struct ksmbd_session,
> -                                 sessions_entry);
> -
> +       xa_for_each(&conn->sessions, id, sess) {
> +               xa_erase(&conn->sessions, sess->id);
>                 ksmbd_session_destroy(sess);
>         }
>  }
>
> -static bool ksmbd_session_id_match(struct ksmbd_session *sess,
> -                                  unsigned long long id)
> -{
> -       return sess->id == id;
> -}
> -
>  struct ksmbd_session *ksmbd_session_lookup(struct ksmbd_conn *conn,
>                                            unsigned long long id)
>  {
> -       struct ksmbd_session *sess = NULL;
> -
> -       list_for_each_entry(sess, &conn->sessions, sessions_entry) {
> -               if (ksmbd_session_id_match(sess, id))
> -                       return sess;
> -       }
> -       return NULL;
> +       return xa_load(&conn->sessions, id);
>  }
>
>  int get_session(struct ksmbd_session *sess)
> @@ -314,7 +298,6 @@ static struct ksmbd_session *__session_create(int protocol)
>                 goto error;
>
>         set_session_flag(sess, protocol);
> -       INIT_LIST_HEAD(&sess->sessions_entry);
>         xa_init(&sess->tree_conns);
>         INIT_LIST_HEAD(&sess->ksmbd_chann_list);
>         INIT_LIST_HEAD(&sess->rpc_handle_list);
> diff --git a/fs/ksmbd/mgmt/user_session.h b/fs/ksmbd/mgmt/user_session.h
> index e241f16a3851..8b08189be3fc 100644
> --- a/fs/ksmbd/mgmt/user_session.h
> +++ b/fs/ksmbd/mgmt/user_session.h
> @@ -57,7 +57,6 @@ struct ksmbd_session {
>         __u8                            smb3decryptionkey[SMB3_ENC_DEC_KEY_SIZE];
>         __u8                            smb3signingkey[SMB3_SIGN_KEY_SIZE];
>
> -       struct list_head                sessions_entry;
>         struct ksmbd_file_table         file_table;
>         atomic_t                        refcnt;
>  };
> @@ -84,8 +83,8 @@ void ksmbd_session_destroy(struct ksmbd_session *sess);
>  struct ksmbd_session *ksmbd_session_lookup_slowpath(unsigned long long id);
>  struct ksmbd_session *ksmbd_session_lookup(struct ksmbd_conn *conn,
>                                            unsigned long long id);
> -void ksmbd_session_register(struct ksmbd_conn *conn,
> -                           struct ksmbd_session *sess);
> +int ksmbd_session_register(struct ksmbd_conn *conn,
> +                          struct ksmbd_session *sess);
>  void ksmbd_sessions_deregister(struct ksmbd_conn *conn);
>  struct ksmbd_session *ksmbd_session_lookup_all(struct ksmbd_conn *conn,
>                                                unsigned long long id);
> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
> index 94ab1dcd80e7..04d20a2e6dee 100644
> --- a/fs/ksmbd/smb2pdu.c
> +++ b/fs/ksmbd/smb2pdu.c
> @@ -588,7 +588,8 @@ int smb2_check_user_session(struct ksmbd_work *work)
>         return -EINVAL;
>  }
>
> -static void destroy_previous_session(struct ksmbd_user *user, u64 id)
> +static void destroy_previous_session(struct ksmbd_conn *conn,
> +                                    struct ksmbd_user *user, u64 id)
>  {
>         struct ksmbd_session *prev_sess = ksmbd_session_lookup_slowpath(id);
>         struct ksmbd_user *prev_user;
> @@ -607,6 +608,7 @@ static void destroy_previous_session(struct ksmbd_user *user, u64 id)
>         }
>
>         put_session(prev_sess);
> +       xa_erase(&conn->sessions, prev_sess->id);
>         ksmbd_session_destroy(prev_sess);
>  }
>
> @@ -1439,7 +1441,7 @@ static int ntlm_authenticate(struct ksmbd_work *work)
>         /* Check for previous session */
>         prev_id = le64_to_cpu(req->PreviousSessionId);
>         if (prev_id && prev_id != sess->id)
> -               destroy_previous_session(user, prev_id);
> +               destroy_previous_session(conn, user, prev_id);
>
>         if (sess->state == SMB2_SESSION_VALID) {
>                 /*
> @@ -1561,7 +1563,7 @@ static int krb5_authenticate(struct ksmbd_work *work)
>         /* Check previous session */
>         prev_sess_id = le64_to_cpu(req->PreviousSessionId);
>         if (prev_sess_id && prev_sess_id != sess->id)
> -               destroy_previous_session(sess->user, prev_sess_id);
> +               destroy_previous_session(conn, sess->user, prev_sess_id);
>
>         if (sess->state == SMB2_SESSION_VALID)
>                 ksmbd_free_user(sess->user);
> @@ -1650,7 +1652,9 @@ int smb2_sess_setup(struct ksmbd_work *work)
>                         goto out_err;
>                 }
>                 rsp->hdr.SessionId = cpu_to_le64(sess->id);
> -               ksmbd_session_register(conn, sess);
> +               rc = ksmbd_session_register(conn, sess);
> +               if (rc)
> +                       goto out_err;
>         } else if (conn->dialect >= SMB30_PROT_ID &&
>                    (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB3_MULTICHANNEL) &&
>                    req->Flags & SMB2_SESSION_REQ_FLAG_BINDING) {
> @@ -1828,6 +1832,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
>                         if (sess->user && sess->user->flags & KSMBD_USER_FLAG_DELAY_SESSION)
>                                 try_delay = true;
>
> +                       xa_erase(&conn->sessions, sess->id);
>                         ksmbd_session_destroy(sess);
>                         work->sess = NULL;
>                         if (try_delay)
> --
> 2.25.1
>


-- 
Thanks,
Hyunchul