[PATCH] Fix failure to update dirpath with Gluster - bug https://bugzilla.samba.org/show_bug.cgi?id=135850

[Jeremy Allison]

On Mon, Aug 20, 2018 at 05:21:20PM -0700, Jeremy Allison wrote:
> Fixes bug https://bugzilla.samba.org/show_bug.cgi?id=13585,
> as Gluster now needs a "." path passed in instead of a
> pointer to '\0', which we were using before in the non-optimization
> path return from check_parent_exists().
> 
> Makes the code clearer (expands one return path into two).
> 
> Based on code from Anoop C S <anoopcs at redhat.com> and
> confirmed fixes the problem by Anoop.
> 
> Please review and push if happy !

Withdrawing this patch, please don't review or push.

I've been looking really closely now at the *callers*
of check_parent_exists() and many of them depend on
*dirpath == '\0' in the case where smb_fname->base_name
doesn't contain a component containing '/' characters.

I think my fix, although it might work, will affect
other paths. Let me look at this more closely
tomorrow. I'm withdrawing this for now.

Jeremy.

> From 9e54cfac8c0760e3b718ced50b5ba885e01e4404 Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Mon, 20 Aug 2018 16:51:18 -0700
> Subject: [PATCH] s3: smbd: Ensure check_parent_exists() returns valid paths
>  for non-optimization return.
> 
> Needed for vfs_glusterfs, as Gluster requires ".".
> 
> Based on a fix from Anoop C S <anoopcs at redhat.com>
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=13585
> 
> Signed-off-by: Jeremy Allison <jra at samba.org>
> ---
>  source3/smbd/filename.c | 40 ++++++++++++++++++++++++++++++----------
>  1 file changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
> index 9e15af1916d..a2882632860 100644
> --- a/source3/smbd/filename.c
> +++ b/source3/smbd/filename.c
> @@ -164,13 +164,11 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
>  				char **pp_dirpath,
>  				char **pp_start)
>  {
> -	struct smb_filename parent_fname;
> +	struct smb_filename parent_fname = { 0 };
>  	const char *last_component = NULL;
>  	NTSTATUS status;
>  	int ret;
> -	bool parent_fname_has_wild = false;
>  
> -	ZERO_STRUCT(parent_fname);
>  	if (!parent_dirname(ctx, smb_fname->base_name,
>  				&parent_fname.base_name,
>  				&last_component)) {
> @@ -178,18 +176,18 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
>  	}
>  
>  	if (!posix_pathnames) {
> -		parent_fname_has_wild = ms_has_wild(parent_fname.base_name);
> +		if (ms_has_wild(parent_fname.base_name)) {
> +			goto no_optimization_out;
> +		}
>  	}
>  
>  	/*
>  	 * If there was no parent component in
> -	 * smb_fname->base_name of the parent name
> -	 * contained a wildcard then don't do this
> +	 * smb_fname->base_name then don't do this
>  	 * optimization.
>  	 */
> -	if ((smb_fname->base_name == last_component) ||
> -			parent_fname_has_wild) {
> -		return NT_STATUS_OK;
> +	if (smb_fname->base_name == last_component) {
> +		goto no_optimization_out;
>  	}
>  
>  	if (posix_pathnames) {
> @@ -202,7 +200,7 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
>  	   with the normal tree walk. */
>  
>  	if (ret == -1) {
> -		return NT_STATUS_OK;
> +		goto no_optimization_out;
>  	}
>  
>  	status = check_for_dot_component(&parent_fname);
> @@ -235,6 +233,28 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
>  		*pp_start));
>  
>  	return NT_STATUS_OK;
> +
> +  no_optimization_out:
> +
> +	/*
> +	 * We must still return an *pp_dirpath
> +	 * initialized to ".", and a *pp_start
> +	 * pointing at smb_fname->base_name.
> +	 */
> +
> +	TALLOC_FREE(parent_fname.base_name);
> +
> +	*pp_dirpath = talloc_strdup(ctx, ".");
> +	if (*pp_dirpath == NULL) {
> +		return NT_STATUS_NO_MEMORY;
> +	}
> +	/*
> +	 * Safe to use discard_const_p
> +	 * here as by convention smb_fname->base_name
> +	 * is allocated off ctx.
> +	 */
> +	*pp_start = discard_const_p(char, smb_fname->base_name);
> +	return NT_STATUS_OK;
>  }
>  
>  /*
> -- 
> 2.18.0.865.gffc8e1a3cd6-goog
>