smb: client: Avoid race in open_cached_dir with lease breaks

A pre-existing valid cfid returned from find_or_create_cached_dir might
race with a lease break, meaning open_cached_dir doesn't consider it
valid, and thinks it's newly-constructed. This leaks a dentry reference
if the allocation occurs before the queued lease break work runs.

Avoid the race by extending holding the cfid_list_lock across
find_or_create_cached_dir and when the result is checked.

Cc: stable@vger.kernel.org
Reviewed-by: Henrique Carvalho <henrique.carvalho@suse.com>
Signed-off-by: Paul Aurich <paul@darkrain42.org>
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 fe738623cf1ba91ee44e23de346c60460cbc647d..240d82c6f908063b9efc0c8726ec1d44f12aa289 100644 (file)
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -29,7 +29,6 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
 {
        struct cached_fid *cfid;
 
-       spin_lock(&cfids->cfid_list_lock);
        list_for_each_entry(cfid, &cfids->entries, entry) {
                if (!strcmp(cfid->path, path)) {
                        /*
@@ -38,25 +37,20 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
                         * being deleted due to a lease break.
                         */
                        if (!cfid->time || !cfid->has_lease) {
-                               spin_unlock(&cfids->cfid_list_lock);
                                return NULL;
                        }
                        kref_get(&cfid->refcount);
-                       spin_unlock(&cfids->cfid_list_lock);
                        return cfid;
                }
        }
        if (lookup_only) {
-               spin_unlock(&cfids->cfid_list_lock);
                return NULL;
        }
        if (cfids->num_entries >= max_cached_dirs) {
-               spin_unlock(&cfids->cfid_list_lock);
                return NULL;
        }
        cfid = init_cached_dir(path);
        if (cfid == NULL) {
-               spin_unlock(&cfids->cfid_list_lock);
                return NULL;
        }
        cfid->cfids = cfids;
@@ -74,7 +68,6 @@ static struct cached_fid *find_or_create_cached_dir(struct cached_fids *cfids,
         */
        cfid->has_lease = true;
 
-       spin_unlock(&cfids->cfid_list_lock);
        return cfid;
 }
 
@@ -187,8 +180,10 @@ replay_again:
        if (!utf16_path)
                return -ENOMEM;
 
+       spin_lock(&cfids->cfid_list_lock);
        cfid = find_or_create_cached_dir(cfids, path, lookup_only, tcon->max_cached_dirs);
        if (cfid == NULL) {
+               spin_unlock(&cfids->cfid_list_lock);
                kfree(utf16_path);
                return -ENOENT;
        }
@@ -197,7 +192,6 @@ replay_again:
         * 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 && cfid->time) {
                spin_unlock(&cfids->cfid_list_lock);
                *ret_cfid = cfid;