Re: [PATCH] cifs: fix pointer initialization and checks in cifs_follow_symlink (try #3)

Jeff Layton <[email protected]> writes:

> This is the third respin of the patch posted yesterday to fix the error
> handling in cifs_follow_symlink. It also includes a fix for a bogus NULL
> pointer check in CIFSSMBQueryUnixSymLink that Jeff Moyer spotted.
>
> It's possible for CIFSSMBQueryUnixSymLink to return without setting
> target_path to a valid pointer. If that happens then the current value
> to which we're initializing this pointer could cause an oops when it's
> kfree'd.
>
> This patch is a little more comprehensive than the last patches. It
> reorganizes cifs_follow_link a bit for (hopefully) better readability.

> It should also eliminate the uneeded allocation of full_path on servers
> without unix extensions (assuming they can get to this point anyway, of
> which I'm not convinced).

Well, you've sacrificed your logging for this.  See below.

> index ea9d11e..737a386 100644
> --- a/fs/cifs/link.c
> +++ b/fs/cifs/link.c
> @@ -107,48 +107,48 @@ void *
>  cifs_follow_link(struct dentry *direntry, struct nameidata *nd)
>  {
>  	struct inode *inode = direntry->d_inode;
> -	int rc = -EACCES;
> +	int rc = -ENOMEM;
>  	int xid;
>  	char *full_path = NULL;
> -	char *target_path = ERR_PTR(-ENOMEM);
> -	struct cifs_sb_info *cifs_sb;
> -	struct cifsTconInfo *pTcon;
> +	char *target_path = NULL;
> +	struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb);
> +	struct cifsTconInfo *tcon = cifs_sb->tcon;
>  
>  	xid = GetXid();
>  
> -	full_path = build_path_from_dentry(direntry);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> -
> -	if (!full_path)
> -		goto out;
> -
>  	cFYI(1, ("Full path: %s inode = 0x%p", full_path, inode));
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Aside from that, the patch looks good.

Cheers,
Jeff