[PATCH] Fix failure to update dirpath

[Jeremy Allison]

On Mon, Aug 20, 2018 at 05:17:54PM +0530, Anoop C S via samba-technical wrote:
> On Sat, 2018-08-18 at 17:02 +0530, Anoop C S via samba-technical wrote:
> > Hi,
> > 
> > Please find the attached patch which updates dirpath where it failed to do so in
> > check_parent_exists().
> 
> Attached is a modified version with a change to where goto label was placed in the previous version
> of the patch so as to update "start" component too. This should be it.
> 
> > Tested with GlusterFS as backend file system.
> > 
> > Reviews are appreciated.
> 
> Ok.. so a quick re-look at the patch made me think it being too generic to update "start" and
> "dirpath" in 3 cases rather than specific to the case where last_component and smb_fname are equal. 
> 
> Hmm.. actually it should not cause any harm to further processing down the line. Just noting it down
> here to help whoever wants to review the change.

OK Anoop I'm reviewing this, and I'd like to understand
exactly what is the bug you're fixing here.

Can you explain what passed in pathnames are not being
correctly updated ?

As far as I can see this is an optimization path which
isn't required for correctness, so what am I missing ?

Thanks,

	Jeremy.

> From 06e95bfcc0f61cc32f191bb71bd8f593aad458c6 Mon Sep 17 00:00:00 2001
> From: Anoop C S <anoopcs at redhat.com>
> Date: Sat, 18 Aug 2018 14:29:05 +0530
> Subject: [PATCH 1/2] s3/smbd: Fix a typo
> 
> Signed-off-by: Anoop C S <anoopcs at redhat.com>
> ---
>  source3/smbd/filename.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
> index 9e15af1916d..662c6a95fc1 100644
> --- a/source3/smbd/filename.c
> +++ b/source3/smbd/filename.c
> @@ -183,7 +183,7 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
>  
>  	/*
>  	 * If there was no parent component in
> -	 * smb_fname->base_name of the parent name
> +	 * smb_fname->base_name or the parent name
>  	 * contained a wildcard then don't do this
>  	 * optimization.
>  	 */
> -- 
> 2.17.1
> 
> 
> From 9d1cadd230841ffd18ab5629e3a3c84c86d3ac34 Mon Sep 17 00:00:00 2001
> From: Anoop C S <anoopcs at redhat.com>
> Date: Sat, 18 Aug 2018 15:19:15 +0530
> Subject: [PATCH 2/2] s3/smbd: Update dirpath in missing places inside
>  check_parent_exists()
> 
> We failed to update *pp_dirpath in the following situations:
> * if smb_fname does not contain a '/'.
>   - which essentially implies a file/directory under root of the share
>     while we are at it. dir_path is expected to be '.'
> * if stat of parent_fname fails
> 
> Signed-off-by: Anoop C S <anoopcs at redhat.com>
> ---
>  source3/smbd/filename.c | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c
> index 662c6a95fc1..6a8177cbb49 100644
> --- a/source3/smbd/filename.c
> +++ b/source3/smbd/filename.c
> @@ -174,7 +174,8 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
>  	if (!parent_dirname(ctx, smb_fname->base_name,
>  				&parent_fname.base_name,
>  				&last_component)) {
> -		return NT_STATUS_NO_MEMORY;
> +		status = NT_STATUS_NO_MEMORY;
> +		goto err;
>  	}
>  
>  	if (!posix_pathnames) {
> @@ -189,7 +190,8 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
>  	 */
>  	if ((smb_fname->base_name == last_component) ||
>  			parent_fname_has_wild) {
> -		return NT_STATUS_OK;
> +		status = NT_STATUS_OK;
> +		goto done;
>  	}
>  
>  	if (posix_pathnames) {
> @@ -202,14 +204,16 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
>  	   with the normal tree walk. */
>  
>  	if (ret == -1) {
> -		return NT_STATUS_OK;
> +		status = NT_STATUS_OK;
> +		goto done;
>  	}
>  
>  	status = check_for_dot_component(&parent_fname);
>  	if (!NT_STATUS_IS_OK(status)) {
> -		return status;
> +		goto err;
>  	}
>  
> +done:
>  	/* Parent exists - set "start" to be the
>  	 * last component to shorten the tree walk. */
>  
> @@ -224,7 +228,8 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
>  	TALLOC_FREE(*pp_dirpath);
>  	*pp_dirpath = talloc_strdup(ctx, parent_fname.base_name);
>  	if (!*pp_dirpath) {
> -		return NT_STATUS_NO_MEMORY;
> +		status = NT_STATUS_NO_MEMORY;
> +		goto err;
>  	}
>  
>  	DEBUG(5,("check_parent_exists: name "
> @@ -233,8 +238,8 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx,
>  		smb_fname->base_name,
>  		*pp_dirpath,
>  		*pp_start));
> -
> -	return NT_STATUS_OK;
> +err:
> +	return status;
>  }
>  
>  /*
> -- 
> 2.17.1
>