[linux-cifs-client] [PATCH] [CIFS] Fixed race: Read freed memory when accessing cifs_sb->prepath

[Jeff Layton]

On Mon, 06 Oct 2008 19:51:08 +0400
Igor Mammedov <niallain at gmail.com> wrote:

> >From a902e94f1e603855797a56237a30726e088f3f05 Mon Sep 17 00:00:00 2001  
> From: Igor Mammedov <niallain at gmail.com>
> Date: Mon, 6 Oct 2008 18:28:21 +0400
> Subject: [PATCH] [CIFS] Fixed race: Read freed memory when accessing cifs_sb->prepath
> 
> Signed-off-by: Igor Mammedov <niallain at gmail.com>
> ---
>  fs/cifs/connect.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 4c13bcd..0665b89 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2308,13 +2308,15 @@ cifs_mount(struct super_block *sb, struct cifs_sb_info *cifs_sb,
>  	the password ptr is put in the new session structure (in which case the
>  	password will be freed at unmount time) */
>  out:
> +	if (rc)
> +		kfree(volume_info.prepath);
> +
>  	/* zero out password before freeing */
>  	if (volume_info.password != NULL) {
>  		memset(volume_info.password, 0, strlen(volume_info.password));
>  		kfree(volume_info.password);
>  	}
>  	kfree(volume_info.UNC);
> -	kfree(volume_info.prepath);
>  	FreeXid(xid);
>  	return rc;
>  }

I don't see any race here. Can you explain how this happens? The only place
we reference volume_info.prepath is here:

                /* calculate prepath */
                cifs_sb->prepath = volume_info.prepath;
                if (cifs_sb->prepath) {
                        cifs_sb->prepathlen = strlen(cifs_sb->prepath);
                        /* we can not convert the / to \ in the path
                        separators in the prefixpath yet because we do not
                        know (until reset_cifs_unix_caps is called later)
                        whether POSIX PATH CAP is available. We normalize
                        the / to \ after reset_cifs_unix_caps is called */
                        volume_info.prepath = NULL;

The pointer should be NULL after this, and that should make the kfree
harmless.

-- 
Jeff Layton <jlayton at redhat.com>