smb: client: prevent new fids from being removed by laundromat

Check if @cfid->time is set in laundromat so we guarantee that only
fully cached fids will be selected for removal.  While we're at it,
add missing locks to protect access of @cfid fields in order to avoid
races with open_cached_dir() and cfids_laundromat_worker(),
respectively.

Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
Reviewed-by: Shyam Prasad N <sprasad@microsoft.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 a9e5d3b..fe1bf5b 100644 (file)
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -170,15 +170,18 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
                return -ENOENT;
        }
        /*
-        * At this point we either have a lease already and we can just
-        * return it. If not we are guaranteed to be the only thread accessing
-        * this cfid.
+        * Return cached fid if it has a lease.  Otherwise, it is either a new
+        * entry or laundromat worker removed it from @cfids->entries.  Caller
+        * will put last reference if the latter.
         */
+       spin_lock(&cfids->cfid_list_lock);
        if (cfid->has_lease) {
+               spin_unlock(&cfids->cfid_list_lock);
                *ret_cfid = cfid;
                kfree(utf16_path);
                return 0;
        }
+       spin_unlock(&cfids->cfid_list_lock);
 
        /*
         * Skip any prefix paths in @path as lookup_positive_unlocked() ends up
@@ -295,9 +298,11 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
                        goto oshr_free;
                }
        }
+       spin_lock(&cfids->cfid_list_lock);
        cfid->dentry = dentry;
        cfid->time = jiffies;
        cfid->has_lease = true;
+       spin_unlock(&cfids->cfid_list_lock);
 
 oshr_free:
        kfree(utf16_path);
@@ -306,24 +311,28 @@ oshr_free:
        free_rsp_buf(resp_buftype[0], rsp_iov[0].iov_base);
        free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base);
        spin_lock(&cfids->cfid_list_lock);
-       if (rc && !cfid->has_lease) {
-               if (cfid->on_list) {
-                       list_del(&cfid->entry);
-                       cfid->on_list = false;
-                       cfids->num_entries--;
+       if (!cfid->has_lease) {
+               if (rc) {
+                       if (cfid->on_list) {
+                               list_del(&cfid->entry);
+                               cfid->on_list = false;
+                               cfids->num_entries--;
+                       }
+                       rc = -ENOENT;
+               } else {
+                       /*
+                        * We are guaranteed to have two references at this
+                        * point. One for the caller and one for a potential
+                        * lease. Release the Lease-ref so that the directory
+                        * will be closed when the caller closes the cached
+                        * handle.
+                        */
+                       spin_unlock(&cfids->cfid_list_lock);
+                       kref_put(&cfid->refcount, smb2_close_cached_fid);
+                       goto out;
                }
-               rc = -ENOENT;
        }
        spin_unlock(&cfids->cfid_list_lock);
-       if (!rc && !cfid->has_lease) {
-               /*
-                * We are guaranteed to have two references at this point.
-                * One for the caller and one for a potential lease.
-                * Release the Lease-ref so that the directory will be closed
-                * when the caller closes the cached handle.
-                */
-               kref_put(&cfid->refcount, smb2_close_cached_fid);
-       }
        if (rc) {
                if (cfid->is_open)
                        SMB2_close(0, cfid->tcon, cfid->fid.persistent_fid,
@@ -331,7 +340,7 @@ oshr_free:
                free_cached_dir(cfid);
                cfid = NULL;
        }
-
+out:
        if (rc == 0) {
                *ret_cfid = cfid;
                atomic_inc(&tcon->num_remote_opens);
@@ -583,15 +592,18 @@ static void cfids_laundromat_worker(struct work_struct *work)
 
        spin_lock(&cfids->cfid_list_lock);
        list_for_each_entry_safe(cfid, q, &cfids->entries, entry) {
-               if (time_after(jiffies, cfid->time + HZ * dir_cache_timeout)) {
+               if (cfid->time &&
+                   time_after(jiffies, cfid->time + HZ * dir_cache_timeout)) {
+                       cfid->on_list = false;
                        list_move(&cfid->entry, &entry);
                        cfids->num_entries--;
+                       /* To prevent race with smb2_cached_lease_break() */
+                       kref_get(&cfid->refcount);
                }
        }
        spin_unlock(&cfids->cfid_list_lock);
 
        list_for_each_entry_safe(cfid, q, &entry, entry) {
-               cfid->on_list = false;
                list_del(&cfid->entry);
                /*
                 * Cancel and wait for the work to finish in case we are racing
@@ -608,6 +620,8 @@ static void cfids_laundromat_worker(struct work_struct *work)
                        spin_unlock(&cfids->cfid_list_lock);
                        kref_put(&cfid->refcount, smb2_close_cached_fid);
                }
+               /* Drop the extra reference opened above */
+               kref_put(&cfid->refcount, smb2_close_cached_fid);
        }
        queue_delayed_work(cifsiod_wq, &cfids->laundromat_work,
                           dir_cache_timeout * HZ);