ksmbd: fix race condition with fp

[ Upstream commit 5a7ee91d11 ]

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Namjae Jeon 2023-12-19 00:34:32 +09:00 committed by Greg Kroah-Hartman
parent c77fd3e25a
commit f99d5d1d2a
3 changed files with 32 additions and 4 deletions

View File

@ -3367,8 +3367,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;

View File

@ -332,6 +332,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;
@ -381,15 +384,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;
@ -569,6 +577,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) {
@ -590,6 +599,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,

View File

@ -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