Commit 734e9962 authored by Henrique Carvalho's avatar Henrique Carvalho Committed by Steve French
Browse files

smb: client: fix potential UAF in smb2_close_cached_fid() 



find_or_create_cached_dir() could grab a new reference after kref_put()
had seen the refcount drop to zero but before cfid_list_lock is acquired
in smb2_close_cached_fid(), leading to use-after-free.

Switch to kref_put_lock() so cfid_release() is called with
cfid_list_lock held, closing that gap.

Fixes: ebe98f14 ("cifs: enable caching of directories for which a lease is held")
Cc: stable@vger.kernel.org
Reported-by: default avatarJay Shin <jaeshin@redhat.com>
Reviewed-by: default avatarPaulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: default avatarHenrique Carvalho <henrique.carvalho@suse.com>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 6146a0f1

fs/smb/client/cached_dir.c

+9 −7
Original line number Diff line number Diff line
@@ -388,11 +388,11 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, 
			 * lease. Release one here, and the second below.

			 */

			cfid->has_lease = false;

			kref_put(&cfid->refcount, smb2_close_cached_fid);

			close_cached_dir(cfid);

		}

		spin_unlock(&cfids->cfid_list_lock);



		kref_put(&cfid->refcount, smb2_close_cached_fid);

		close_cached_dir(cfid);

	} else {

		*ret_cfid = cfid;

		atomic_inc(&tcon->num_remote_opens);
@@ -438,12 +438,14 @@ int open_cached_dir_by_dentry(struct cifs_tcon *tcon, 


static void

smb2_close_cached_fid(struct kref *ref)

__releases(&cfid->cfids->cfid_list_lock)

{

	struct cached_fid *cfid = container_of(ref, struct cached_fid,

					       refcount);

	int rc;



	spin_lock(&cfid->cfids->cfid_list_lock);

	lockdep_assert_held(&cfid->cfids->cfid_list_lock);



	if (cfid->on_list) {

		list_del(&cfid->entry);

		cfid->on_list = false;
@@ -478,7 +480,7 @@ void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon, 
	spin_lock(&cfid->cfids->cfid_list_lock);

	if (cfid->has_lease) {

		cfid->has_lease = false;

		kref_put(&cfid->refcount, smb2_close_cached_fid);

		close_cached_dir(cfid);

	}

	spin_unlock(&cfid->cfids->cfid_list_lock);

	close_cached_dir(cfid);
@@ -487,7 +489,7 @@ void drop_cached_dir_by_name(const unsigned int xid, struct cifs_tcon *tcon, 


void close_cached_dir(struct cached_fid *cfid)

{

	kref_put(&cfid->refcount, smb2_close_cached_fid);

	kref_put_lock(&cfid->refcount, smb2_close_cached_fid, &cfid->cfids->cfid_list_lock);

}



/*
@@ -596,7 +598,7 @@ cached_dir_offload_close(struct work_struct *work) 


	WARN_ON(cfid->on_list);



	kref_put(&cfid->refcount, smb2_close_cached_fid);

	close_cached_dir(cfid);

	cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_cached_close);

}
@@ -762,7 +764,7 @@ static void cfids_laundromat_worker(struct work_struct *work) 
			 * Drop the ref-count from above, either the lease-ref (if there

			 * was one) or the extra one acquired.

			 */

			kref_put(&cfid->refcount, smb2_close_cached_fid);

			close_cached_dir(cfid);

	}

	queue_delayed_work(cfid_put_wq, &cfids->laundromat_work,

			   dir_cache_timeout * HZ);