Re: [PATCH 5/7] cifs: Fix potential deadlock when updating vol in cifs_reconnect()

["Aurélien Aptel" <aaptel@suse.com>]

"Paulo Alcantara (SUSE)" <pc@cjr.nz> writes:

> We can't hold the vol_lock spinlock while refreshing the DFS cache
> because cifs_reconnect() may call dfs_cache_update_vol() while we are
> walking through the volume list.
>
> Create a temp list with all volumes eligible for refreshing and then
> use it without any locks held.

Commit msg should mention it makes the vol_info refcounted.

> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
> ---
>  fs/cifs/dfs_cache.c | 45 +++++++++++++++++++++++++++++++++------------
>  1 file changed, 33 insertions(+), 12 deletions(-)
>
> diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c
> index b082603c793a..5b9d7281dd67 100644
> --- a/fs/cifs/dfs_cache.c
> +++ b/fs/cifs/dfs_cache.c
> @@ -49,6 +49,8 @@ struct vol_info {
>  	struct smb_vol smb_vol;
>  	char *mntdata;
>  	struct list_head list;
> +	struct list_head rlist;
> +	int vol_count;
>  };
>  
>  static struct kmem_cache *cache_slab __read_mostly;
> @@ -516,13 +518,15 @@ static struct cache_entry *lookup_cache_entry(const char *path,
>  	return ce;
>  }
>  
> -static inline void free_vol(struct vol_info *vi)
> +static void put_vol(struct vol_info *vi)
>  {
> -	list_del(&vi->list);
> -	kfree(vi->fullpath);
> -	kfree(vi->mntdata);
> -	cifs_cleanup_volume_info_contents(&vi->smb_vol);
> -	kfree(vi);
> +	if (!--vi->vol_count) {
> +		list_del_init(&vi->list);
> +		kfree(vi->fullpath);
> +		kfree(vi->mntdata);
> +		cifs_cleanup_volume_info_contents(&vi->smb_vol);
> +		kfree(vi);
> +	}
>  }

Can we document that put_vol() assumes vol_lock is held?


-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, DE
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah HRB 247165 (AG München)