ksmbd: fix race condition with fp

fp can used in each command. If smb2_close command is coming at the
same time, UAF issue can happen by race condition.

                           Time
                            +
Thread A                    | Thread B1 B2 .... B5
smb2_open                   | smb2_close
                            |
 __open_id                  |
   insert fp to file_table  |
                            |
                            |   atomic_dec_and_test(&fp->refcount)
                            |   if fp->refcount == 0, free fp by kfree.
 // UAF!                    |
 use fp                     |
                            +
This patch add f_state not to use freed fp is used and not to free fp in
use.

Reported-by: luosili <rootlab@huawei.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 544022d..420e769 100644 (file)
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -3370,8 +3370,10 @@ err_out:
        }
        ksmbd_revert_fsids(work);
 err_out1:
-       if (!rc)
+       if (!rc) {
+               ksmbd_update_fstate(&work->sess->file_table, fp, FP_INITED);
                rc = ksmbd_iov_pin_rsp(work, (void *)rsp, iov_len);
+       }
        if (rc) {
                if (rc == -EINVAL)
                        rsp->hdr.Status = STATUS_INVALID_PARAMETER;

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index f41f8d6..c4b80ab 100644 (file)
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -333,6 +333,9 @@ static void __ksmbd_close_fd(struct ksmbd_file_table *ft, struct ksmbd_file *fp)
 
 static struct ksmbd_file *ksmbd_fp_get(struct ksmbd_file *fp)
 {
+       if (fp->f_state != FP_INITED)
+               return NULL;
+
        if (!atomic_inc_not_zero(&fp->refcount))
                return NULL;
        return fp;
@@ -382,15 +385,20 @@ int ksmbd_close_fd(struct ksmbd_work *work, u64 id)
                return 0;
 
        ft = &work->sess->file_table;
-       read_lock(&ft->lock);
+       write_lock(&ft->lock);
        fp = idr_find(ft->idr, id);
        if (fp) {
                set_close_state_blocked_works(fp);
 
-               if (!atomic_dec_and_test(&fp->refcount))
+               if (fp->f_state != FP_INITED)
                        fp = NULL;
+               else {
+                       fp->f_state = FP_CLOSED;
+                       if (!atomic_dec_and_test(&fp->refcount))
+                               fp = NULL;
+               }
        }
-       read_unlock(&ft->lock);
+       write_unlock(&ft->lock);
 
        if (!fp)
                return -EINVAL;
@@ -570,6 +578,7 @@ struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp)
        fp->tcon                = work->tcon;
        fp->volatile_id         = KSMBD_NO_FID;
        fp->persistent_id       = KSMBD_NO_FID;
+       fp->f_state             = FP_NEW;
        fp->f_ci                = ksmbd_inode_get(fp);
 
        if (!fp->f_ci) {
@@ -591,6 +600,14 @@ err_out:
        return ERR_PTR(ret);
 }
 
+void ksmbd_update_fstate(struct ksmbd_file_table *ft, struct ksmbd_file *fp,
+                        unsigned int state)
+{
+       write_lock(&ft->lock);
+       fp->f_state = state;
+       write_unlock(&ft->lock);
+}
+
 static int
 __close_file_table_ids(struct ksmbd_file_table *ft,
                       struct ksmbd_tree_connect *tcon,

diff --git a/fs/smb/server/vfs_cache.h b/fs/smb/server/vfs_cache.h
index fcb1341..03d0bf9 100644 (file)
--- a/fs/smb/server/vfs_cache.h
+++ b/fs/smb/server/vfs_cache.h
@@ -60,6 +60,12 @@ struct ksmbd_inode {
        __le32                          m_fattr;
 };
 
+enum {
+       FP_NEW = 0,
+       FP_INITED,
+       FP_CLOSED
+};
+
 struct ksmbd_file {
        struct file                     *filp;
        u64                             persistent_id;
@@ -98,6 +104,7 @@ struct ksmbd_file {
        /* if ls is happening on directory, below is valid*/
        struct ksmbd_readdir_data       readdir_data;
        int                             dot_dotdot[2];
+       unsigned int                    f_state;
 };
 
 static inline void set_ctx_actor(struct dir_context *ctx,
@@ -142,6 +149,8 @@ int ksmbd_close_inode_fds(struct ksmbd_work *work, struct inode *inode);
 int ksmbd_init_global_file_table(void);
 void ksmbd_free_global_file_table(void);
 void ksmbd_set_fd_limit(unsigned long limit);
+void ksmbd_update_fstate(struct ksmbd_file_table *ft, struct ksmbd_file *fp,
+                        unsigned int state);
 
 /*
  * INODE hash