ksmbd: fix user-after-free from session log off

[ Upstream commit 7aa8804c0b67b3cb263a472d17f2cb50d7f1a930 ]

There is racy issue between smb2 session log off and smb2 session setup.
It will cause user-after-free from session log off.
This add session_lock when setting SMB2_SESSION_EXPIRED and referece
count to session struct not to free session while it is being used.

Cc: stable@vger.kernel.org # v5.15+
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-25282
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c
index 15f68ee0508946843983b7d773dc6837f12b3bf0..844db95e66511c872dece9cdcc985cebf371e675 100644 (file)
--- a/fs/ksmbd/mgmt/user_session.c
+++ b/fs/ksmbd/mgmt/user_session.c
@@ -176,9 +176,10 @@ static void ksmbd_expire_session(struct ksmbd_conn *conn)
 
        down_write(&conn->session_lock);
        xa_for_each(&conn->sessions, id, sess) {
-               if (sess->state != SMB2_SESSION_VALID ||
-                   time_after(jiffies,
-                              sess->last_active + SMB2_SESSION_TIMEOUT)) {
+               if (atomic_read(&sess->refcnt) == 0 &&
+                   (sess->state != SMB2_SESSION_VALID ||
+                    time_after(jiffies,
+                              sess->last_active + SMB2_SESSION_TIMEOUT))) {
                        xa_erase(&conn->sessions, sess->id);
                        hash_del(&sess->hlist);
                        ksmbd_session_destroy(sess);
@@ -268,8 +269,6 @@ struct ksmbd_session *ksmbd_session_lookup_slowpath(unsigned long long id)
 
        down_read(&sessions_table_lock);
        sess = __session_lookup(id);
-       if (sess)
-               sess->last_active = jiffies;
        up_read(&sessions_table_lock);
 
        return sess;
@@ -288,6 +287,22 @@ struct ksmbd_session *ksmbd_session_lookup_all(struct ksmbd_conn *conn,
        return sess;
 }
 
+void ksmbd_user_session_get(struct ksmbd_session *sess)
+{
+       atomic_inc(&sess->refcnt);
+}
+
+void ksmbd_user_session_put(struct ksmbd_session *sess)
+{
+       if (!sess)
+               return;
+
+       if (atomic_read(&sess->refcnt) <= 0)
+               WARN_ON(1);
+       else
+               atomic_dec(&sess->refcnt);
+}
+
 struct preauth_session *ksmbd_preauth_session_alloc(struct ksmbd_conn *conn,
                                                    u64 sess_id)
 {
@@ -356,6 +371,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);
 
        ret = __init_smb2_session(sess);
        if (ret)

diff --git a/fs/ksmbd/mgmt/user_session.h b/fs/ksmbd/mgmt/user_session.h
index 63cb08fffde84c4a5f03b936577fe02288885b6e..ce91b1d698e710d3583decc61d7ce19a9f94e843 100644 (file)
--- a/fs/ksmbd/mgmt/user_session.h
+++ b/fs/ksmbd/mgmt/user_session.h
@@ -61,6 +61,8 @@ struct ksmbd_session {
        struct ksmbd_file_table         file_table;
        unsigned long                   last_active;
        rwlock_t                        tree_conns_lock;
+
+       atomic_t                        refcnt;
 };
 
 static inline int test_session_flag(struct ksmbd_session *sess, int bit)
@@ -101,4 +103,6 @@ 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);
+void ksmbd_user_session_get(struct ksmbd_session *sess);
+void ksmbd_user_session_put(struct ksmbd_session *sess);
 #endif /* __USER_SESSION_MANAGEMENT_H__ */

diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c
index 63b01f7d97031f60a1c7842ccd5159bf918d7e80..09ebcf39d5bcb7c3fed5e7e311eff99e7016a656 100644 (file)
--- a/fs/ksmbd/server.c
+++ b/fs/ksmbd/server.c
@@ -238,6 +238,8 @@ static void __handle_ksmbd_work(struct ksmbd_work *work,
        } while (is_chained == true);
 
 send:
+       if (work->sess)
+               ksmbd_user_session_put(work->sess);
        if (work->tcon)
                ksmbd_tree_connect_put(work->tcon);
        smb3_preauth_hash_rsp(work);

diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 089dc2f51229ab9f42c7c296cdb2ad7fdb895ad9..54f7cf7a98b2b5001bed27488ae5b6960a86391c 100644 (file)
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -606,8 +606,10 @@ int smb2_check_user_session(struct ksmbd_work *work)
 
        /* Check for validity of user session */
        work->sess = ksmbd_session_lookup_all(conn, sess_id);
-       if (work->sess)
+       if (work->sess) {
+               ksmbd_user_session_get(work->sess);
                return 1;
+       }
        ksmbd_debug(SMB, "Invalid user session, Uid %llu\n", sess_id);
        return -ENOENT;
 }
@@ -1761,6 +1763,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
                }
 
                conn->binding = true;
+               ksmbd_user_session_get(sess);
        } else if ((conn->dialect < SMB30_PROT_ID ||
                    server_conf.flags & KSMBD_GLOBAL_FLAG_SMB3_MULTICHANNEL) &&
                   (req->Flags & SMB2_SESSION_REQ_FLAG_BINDING)) {
@@ -1787,6 +1790,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
                }
 
                conn->binding = false;
+               ksmbd_user_session_get(sess);
        }
        work->sess = sess;
 
@@ -2235,7 +2239,9 @@ int smb2_session_logoff(struct ksmbd_work *work)
        }
 
        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;