ksmbd: add chann_lock to protect ksmbd_chann_list xarray

ksmbd_chann_list xarray lacks synchronization, allowing use-after-free in
multi-channel sessions (between lookup_chann_list() and ksmbd_chann_del).

Adds rw_semaphore chann_lock to struct ksmbd_session and protects
all xa_load/xa_store/xa_erase accesses.

Cc: stable@vger.kernel.org
Reported-by: Igor Stepansky <igor.stepansky@orca.security>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/server/mgmt/user_session.c b/fs/smb/server/mgmt/user_session.c
index 68b3e0cb54d38f0d7f29b94f9b7301f5514d4d08..8c2b14ea7b0ec6ab6e8a72eb36020906ea003ca3 100644 (file)
--- a/fs/smb/server/mgmt/user_session.c
+++ b/fs/smb/server/mgmt/user_session.c
@@ -244,12 +244,14 @@ static void free_channel_list(struct ksmbd_session *sess)
        struct channel *chann;
        unsigned long index;
 
+       down_write(&sess->chann_lock);
        xa_for_each(&sess->ksmbd_chann_list, index, chann) {
                xa_erase(&sess->ksmbd_chann_list, index);
                kfree(chann);
        }
 
        xa_destroy(&sess->ksmbd_chann_list);
+       up_write(&sess->chann_lock);
 }
 
 static void __session_rpc_close(struct ksmbd_session *sess,
@@ -434,7 +436,9 @@ static int ksmbd_chann_del(struct ksmbd_conn *conn, struct ksmbd_session *sess)
 {
        struct channel *chann;
 
+       down_write(&sess->chann_lock);
        chann = xa_erase(&sess->ksmbd_chann_list, (long)conn);
+       up_write(&sess->chann_lock);
        if (!chann)
                return -ENOENT;
 
@@ -668,6 +672,7 @@ static struct ksmbd_session *__session_create(int protocol)
        rwlock_init(&sess->tree_conns_lock);
        atomic_set(&sess->refcnt, 2);
        init_rwsem(&sess->rpc_lock);
+       init_rwsem(&sess->chann_lock);
 
        ret = __init_smb2_session(sess);
        if (ret)

diff --git a/fs/smb/server/mgmt/user_session.h b/fs/smb/server/mgmt/user_session.h
index 176d800c2490657469738e0a7fc1ef461b9d20c0..d94f5e128a9b42a67b7ef4d0f92a3a7025a0a251 100644 (file)
--- a/fs/smb/server/mgmt/user_session.h
+++ b/fs/smb/server/mgmt/user_session.h
@@ -48,6 +48,7 @@ struct ksmbd_session {
        char                            sess_key[CIFS_KEY_SIZE];
 
        struct hlist_node               hlist;
+       struct rw_semaphore             chann_lock;
        struct xarray                   ksmbd_chann_list;
        struct xarray                   tree_conns;
        struct ida                      tree_conn_ida;

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 4d3154cc493eaef4c3be815ea5e8c24c3aa64e67..3efcc7da1b9f64850b90ab3a81cc5f9327065be3 100644 (file)
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -80,7 +80,13 @@ static inline bool check_session_id(struct ksmbd_conn *conn, u64 id)
 
 struct channel *lookup_chann_list(struct ksmbd_session *sess, struct ksmbd_conn *conn)
 {
-       return xa_load(&sess->ksmbd_chann_list, (long)conn);
+       struct channel *chann;
+
+       down_read(&sess->chann_lock);
+       chann = xa_load(&sess->ksmbd_chann_list, (long)conn);
+       up_read(&sess->chann_lock);
+
+       return chann;
 }
 
 /**
@@ -1559,8 +1565,10 @@ binding_session:
                                return -ENOMEM;
 
                        chann->conn = conn;
+                       down_write(&sess->chann_lock);
                        old = xa_store(&sess->ksmbd_chann_list, (long)conn, chann,
                                        KSMBD_DEFAULT_GFP);
+                       up_write(&sess->chann_lock);
                        if (xa_is_err(old)) {
                                kfree(chann);
                                return xa_err(old);
@@ -1652,8 +1660,10 @@ binding_session:
                                return -ENOMEM;
 
                        chann->conn = conn;
+                       down_write(&sess->chann_lock);
                        old = xa_store(&sess->ksmbd_chann_list, (long)conn,
                                        chann, KSMBD_DEFAULT_GFP);
+                       up_write(&sess->chann_lock);
                        if (xa_is_err(old)) {
                                kfree(chann);
                                return xa_err(old);