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

diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index b8ac7b7faf611d9fa06887737cd925c71af365c6..018055fd2cdb1010541efa025be0c60d78a07524 100644 (file)
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -388,11 +388,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);
@@ -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);