smb: client: fix potential UAF in smb2_close_cached_fid()

commit 734e99623c5b65bf2c03e35978a0b980ebc3c2f8 upstream.

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: ebe98f1447bb ("cifs: enable caching of directories for which a lease is held")
Cc: stable@vger.kernel.org
Reported-by: Jay Shin <jaeshin@redhat.com>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index cc857a030a778d2c3ac342f11f03f9392e264f13..1ab737ffedfeea656db2963a4771bb6ee478cef0 100644 (file)
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -389,11 +389,11 @@ out:
                         * 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);
@@ -434,12 +434,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;
@@ -474,7 +476,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);
@@ -483,7 +485,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);
 }
 
 /*
@@ -594,7 +596,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);
 }
 
@@ -771,7 +773,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);