Commit 3ca02e63 authored by Paul Aurich's avatar Paul Aurich Committed by Steve French
Browse files

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: default avatarHenrique Carvalho <henrique.carvalho@suse.com>
Signed-off-by: default avatarPaul Aurich <paul@darkrain42.org>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent d90b0237

fs/smb/client/cached_dir.c

+2 −8
Original line number Diff line number Diff line
@@ -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 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, 
	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 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, 
	 * 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;