Commit 5a7ee91d authored by Namjae Jeon's avatar Namjae Jeon Committed by Steve French
Browse files

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: default avatarluosili <rootlab@huawei.com>
Signed-off-by: default avatarNamjae Jeon <linkinjeon@kernel.org>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 53ff5cf8

fs/smb/server/smb2pdu.c

+3 −1
Original line number Diff line number Diff line
@@ -3370,8 +3370,10 @@ int smb2_open(struct ksmbd_work *work) 
	}

	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;

fs/smb/server/vfs_cache.c

+20 −3
Original line number Diff line number Diff line
@@ -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 (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 @@ struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp) 
	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,

fs/smb/server/vfs_cache.h

+9 −0
Original line number Diff line number Diff line
@@ -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