[PATCH] Fix smbd panic if we chdir() to an unreadable directory.

[Jeremy Allison]

On Sun, Oct 08, 2017 at 06:51:25PM +0200, Ralph Böhme wrote:
> Howdy!
> 
> On Fri, Oct 06, 2017 at 09:56:50PM +0000, Jeremy Allison wrote:
> > Please review and push if happy.
> 
> generally looks good, but, while at it, can we use an early return and reduce the
> indentation level please?

Yes of course. You know I was looking at ways to do that, but didn't
spot the obvious early return from the CHDIR(). Doh!

Thanks :-).

Jeremy.

> From cae435c8a948c1a39ab750aba24fe749e79601a2 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Sun, 8 Oct 2017 18:48:23 +0200
> Subject: [PATCH] FIXUP: early return from failing SMB_VFS_CHDIR, reducing
>  indentation level
> 
> ---
>  source3/smbd/vfs.c | 92 +++++++++++++++++++++++++++++-------------------------
>  1 file changed, 49 insertions(+), 43 deletions(-)
> 
> diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
> index 817e69b484b..d4fccb7eda9 100644
> --- a/source3/smbd/vfs.c
> +++ b/source3/smbd/vfs.c
> @@ -888,57 +888,63 @@ int vfs_ChDir(connection_struct *conn, const struct smb_filename *smb_fname)
>  	DEBUG(4,("vfs_ChDir to %s\n", smb_fname->base_name));
>  
>  	ret = SMB_VFS_CHDIR(conn, smb_fname);
> -	if (ret == 0) {
> +	if (ret != 0) {
> +		saved_errno = errno;
> +		TALLOC_FREE(saved_cwd);
> +		errno = saved_errno;
> +		return -1;
> +	}
> +
> +	/*
> +	 * Always replace conn->cwd_fname. We
> +	 * don't know if it's been modified by
> +	 * VFS modules in the stack.
> +	 */
> +
> +	/* conn cache. */
> +	TALLOC_FREE(conn->cwd_fname);
> +	conn->cwd_fname = vfs_GetWd(conn, conn);
> +	if (conn->cwd_fname == NULL) {
>  		/*
> -		 * Always replace conn->cwd_fname. We
> -		 * don't know if it's been modified by
> -		 * VFS modules in the stack.
> +		 * vfs_GetWd() failed.
> +		 * We must be able to read cwd.
> +		 * Return to original directory
> +		 * and return -1.
>  		 */
> -		/* conn cache. */
> -		TALLOC_FREE(conn->cwd_fname);
> -		conn->cwd_fname = vfs_GetWd(conn, conn);
> -		if (conn->cwd_fname == NULL) {
> +		saved_errno = errno;
> +
> +		if (saved_cwd == NULL) {
>  			/*
> -			 * vfs_GetWd() failed.
> -			 * We must be able to read cwd.
> -			 * Return to original directory
> -			 * and return -1.
> +			 * Failed on the very first chdir()+getwd()
> +			 * for this connection. We can't
> +			 * continue.
>  			 */
> -			saved_errno = errno;
> -
> -			if (saved_cwd == NULL) {
> -				/*
> -				 * Failed on the very first chdir()+getwd()
> -				 * for this connection. We can't
> -				 * continue.
> -				 */
> -				smb_panic("conn->cwd getwd failed\n");
> -				/* NOTREACHED */
> -				return -1;
> -			}
> -
> -			/* Return to the previous $cwd. */
> -			ret = SMB_VFS_CHDIR(conn, saved_cwd);
> -			if (ret != 0) {
> -				smb_panic("conn->cwd getwd failed\n");
> -				/* NOTREACHED */
> -				return -1;
> -			}
> -			/* Restore original conn->cwd_fname. */
> -			conn->cwd_fname = saved_cwd;
> -			errno = saved_errno;
> -			/* And fail the chdir(). */
> +			smb_panic("conn->cwd getwd failed\n");
> +			/* NOTREACHED */
>  			return -1;
>  		}
> -		/* vfs_GetWd() succeeded. */
> -		/* Replace global cache. */
> -		SAFE_FREE(LastDir);
> -		LastDir = SMB_STRDUP(smb_fname->base_name);
>  
> -		DEBUG(4,("vfs_ChDir got %s\n", conn->cwd_fname->base_name));
> -	} else {
> -		saved_errno = errno;
> +		/* Return to the previous $cwd. */
> +		ret = SMB_VFS_CHDIR(conn, saved_cwd);
> +		if (ret != 0) {
> +			smb_panic("conn->cwd getwd failed\n");
> +			/* NOTREACHED */
> +			return -1;
> +		}
> +		/* Restore original conn->cwd_fname. */
> +		conn->cwd_fname = saved_cwd;
> +		errno = saved_errno;
> +		/* And fail the chdir(). */
> +		return -1;
>  	}
> +
> +	/* vfs_GetWd() succeeded. */
> +	/* Replace global cache. */
> +	SAFE_FREE(LastDir);
> +	LastDir = SMB_STRDUP(smb_fname->base_name);
> +
> +	DEBUG(4,("vfs_ChDir got %s\n", conn->cwd_fname->base_name));
> +
>  	TALLOC_FREE(saved_cwd);
>  	if (saved_errno != 0) {
>  		errno = saved_errno;
> -- 
> 2.13.5
>