ksmbd: fix session use-after-free in multichannel connection

commit fa4cdb8cbca7d6cb6aa13e4d8d83d1103f6345db upstream.

There is a race condition between session setup and
ksmbd_sessions_deregister. The session can be freed before the connection
is added to channel list of session.
This patch check reference count of session before freeing it.

Cc: stable@vger.kernel.org
Reported-by: Sean Heelan <seanheelan@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/smb/server/auth.c b/fs/smb/server/auth.c
index c3baf6537fadb100136909a1b5ec778fb5e33411..5345d2417c7fc9125a73568f6dd171e2461a9625 100644 (file)
--- a/fs/smb/server/auth.c
+++ b/fs/smb/server/auth.c
@@ -1012,9 +1012,9 @@ static int ksmbd_get_encryption_key(struct ksmbd_work *work, __u64 ses_id,
 
        ses_enc_key = enc ? sess->smb3encryptionkey :
                sess->smb3decryptionkey;
-       if (enc)
-               ksmbd_user_session_get(sess);
        memcpy(key, ses_enc_key, SMB3_ENC_DEC_KEY_SIZE);
+       if (!enc)
+               ksmbd_user_session_put(sess);
 
        return 0;
 }

diff --git a/fs/smb/server/mgmt/user_session.c b/fs/smb/server/mgmt/user_session.c
index dc59ead4f6c3d6f7a56850ad15937e61b086af03..82dcc86a32c57a67bbfded3a6bdaf848a54cebfb 100644 (file)
--- a/fs/smb/server/mgmt/user_session.c
+++ b/fs/smb/server/mgmt/user_session.c
@@ -180,7 +180,7 @@ static void ksmbd_expire_session(struct ksmbd_conn *conn)
        down_write(&sessions_table_lock);
        down_write(&conn->session_lock);
        xa_for_each(&conn->sessions, id, sess) {
-               if (atomic_read(&sess->refcnt) == 0 &&
+               if (atomic_read(&sess->refcnt) <= 1 &&
                    (sess->state != SMB2_SESSION_VALID ||
                     time_after(jiffies,
                               sess->last_active + SMB2_SESSION_TIMEOUT))) {
@@ -232,7 +232,8 @@ void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
                                down_write(&conn->session_lock);
                                xa_erase(&conn->sessions, sess->id);
                                up_write(&conn->session_lock);
-                               ksmbd_session_destroy(sess);
+                               if (atomic_dec_and_test(&sess->refcnt))
+                                       ksmbd_session_destroy(sess);
                        }
                }
        }
@@ -251,7 +252,8 @@ void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
                if (xa_empty(&sess->ksmbd_chann_list)) {
                        xa_erase(&conn->sessions, sess->id);
                        hash_del(&sess->hlist);
-                       ksmbd_session_destroy(sess);
+                       if (atomic_dec_and_test(&sess->refcnt))
+                               ksmbd_session_destroy(sess);
                }
        }
        up_write(&conn->session_lock);
@@ -327,8 +329,8 @@ void ksmbd_user_session_put(struct ksmbd_session *sess)
 
        if (atomic_read(&sess->refcnt) <= 0)
                WARN_ON(1);
-       else
-               atomic_dec(&sess->refcnt);
+       else if (atomic_dec_and_test(&sess->refcnt))
+               ksmbd_session_destroy(sess);
 }
 
 struct preauth_session *ksmbd_preauth_session_alloc(struct ksmbd_conn *conn,
@@ -433,7 +435,7 @@ static struct ksmbd_session *__session_create(int protocol)
        xa_init(&sess->rpc_handle_list);
        sess->sequence_number = 1;
        rwlock_init(&sess->tree_conns_lock);
-       atomic_set(&sess->refcnt, 1);
+       atomic_set(&sess->refcnt, 2);
 
        ret = __init_smb2_session(sess);
        if (ret)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 17dfb5e7d66c8dbf60d3206b0b3a060670d720c4..8877f9e900b2fb89e23da57398b933554ccde094 100644 (file)
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -2231,13 +2231,14 @@ int smb2_session_logoff(struct ksmbd_work *work)
                return -ENOENT;
        }
 
-       ksmbd_destroy_file_table(&sess->file_table);
        down_write(&conn->session_lock);
        sess->state = SMB2_SESSION_EXPIRED;
        up_write(&conn->session_lock);
 
-       ksmbd_free_user(sess->user);
-       sess->user = NULL;
+       if (sess->user) {
+               ksmbd_free_user(sess->user);
+               sess->user = NULL;
+       }
        ksmbd_all_conn_set_status(sess_id, KSMBD_SESS_NEED_NEGOTIATE);
 
        rsp->StructureSize = cpu_to_le16(4);