Commit 36991c1c authored by Sean Heelan's avatar Sean Heelan Committed by Steve French
Browse files

ksmbd: Fix UAF in __close_file_table_ids 



A use-after-free is possible if one thread destroys the file
via __ksmbd_close_fd while another thread holds a reference to
it. The existing checks on fp->refcount are not sufficient to
prevent this.

The fix takes ft->lock around the section which removes the
file from the file table. This prevents two threads acquiring the
same file pointer via __close_file_table_ids, as well as the other
functions which retrieve a file from the IDR and which already use
this same lock.

Cc: stable@vger.kernel.org
Signed-off-by: default avatarSean Heelan <seanheelan@gmail.com>
Acked-by: default avatarNamjae Jeon <linkinjeon@kernel.org>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 0ca6df4f

fs/smb/server/vfs_cache.c

+26 −7
Original line number Diff line number Diff line
@@ -661,21 +661,40 @@ __close_file_table_ids(struct ksmbd_file_table *ft, 
		       bool (*skip)(struct ksmbd_tree_connect *tcon,

				    struct ksmbd_file *fp))

{

	unsigned int			id;

	struct ksmbd_file *fp;

	unsigned int id = 0;

	int num = 0;



	idr_for_each_entry(ft->idr, fp, id) {

		if (skip(tcon, fp))

	while (1) {

		write_lock(&ft->lock);

		fp = idr_get_next(ft->idr, &id);

		if (!fp) {

			write_unlock(&ft->lock);

			break;

		}



		if (skip(tcon, fp) ||

		    !atomic_dec_and_test(&fp->refcount)) {

			id++;

			write_unlock(&ft->lock);

			continue;

		}



		set_close_state_blocked_works(fp);

		idr_remove(ft->idr, fp->volatile_id);

		fp->volatile_id = KSMBD_NO_FID;

		write_unlock(&ft->lock);



		down_write(&fp->f_ci->m_lock);

		list_del_init(&fp->node);

		up_write(&fp->f_ci->m_lock);



		if (!atomic_dec_and_test(&fp->refcount))

			continue;

		__ksmbd_close_fd(ft, fp);



		num++;

		id++;

	}



	return num;

}