[PATCH] smbd: Fix snapshot query on shares with DFS enabled

[Christof Schmitt]

On Tue, Aug 16, 2016 at 09:59:35AM -0700, Jeremy Allison wrote:
> On Mon, Aug 15, 2016 at 04:25:10PM -0700, Christof Schmitt wrote:
> > On Fri, Aug 12, 2016 at 04:51:35PM -0700, Jeremy Allison wrote:
> > > On Fri, Aug 12, 2016 at 03:59:27PM -0700, Christof Schmitt wrote:
> > > > From 3a33435fc3d61605c75ed3f3b87612fbe729b553 Mon Sep 17 00:00:00 2001
> > > > From: Christof Schmitt <cs at samba.org>
> > > > Date: Fri, 12 Aug 2016 14:59:07 -0700
> > > > Subject: [PATCH] smbd: Fix snapshot query on shares with DFS enabled
> > > > 
> > > > When DFS is enabled (host msdfs = yes and msdfs root = yes), then SMB
> > > > clients send create requests in the format \hostname\service\path.
> > > > Putting the GMT tag as first element breaks the DFS parsing and results
> > > > in OBJECT_NOT_FOUND for snapshotted files.  Fix this by appending the
> > > > GMT tag to the end of the path.
> > > 
> > > Great catch ! Pushed with additional comment:
> > > 
> > > BUG: https://bugzilla.samba.org/show_bug.cgi?id=12150
> > > 
> > > as we're going to need this backported.
> > 
> > I just came across one more thing: The test is on a system with the
> > shadowcopy2 module enabled. While the patches fixes the problem for
> > directories, there seems to be an issue when a SMB client is explicitly
> > asking for older versions of a file. It looks like shadowcopy leaves a
> > trailing slash from the GMT tag removal:
> > 
> > (test is a file, not  a directory)
> > 
> > [2016/08/15 16:12:36.178799, 10, pid=13125, effective(12000500, 12000513), real(12000500, 0)] ../source3/modules/vfs_shadow_copy2.c:245(shadow_copy2_strip_snapshot)
> >   ../source3/modules/vfs_shadow_copy2.c:245: enter path 'test/@GMT-2016.08.12-19.56.11'
> > [2016/08/15 16:12:36.178814, 10, pid=13125, effective(12000500, 12000513), real(12000500, 0)] ../source3/modules/vfs_shadow_copy2.c:462(shadow_copy2_do_convert)
> >  converting 'test/'
> > [2016/08/15 16:12:36.178837, 10, pid=13125, effective(12000500, 12000513), real(12000500, 0)] ../source3/modules/vfs_shadow_copy2.c:604(shadow_copy2_do_convert)
> >  Trying[snapdirseverywhere] /fs1/smbexport1/test/.snapshots/@GMT-2016.08.12-19.56.11/: -1 (Not a directory)
> > [2016/08/15 16:12:36.178860, 10, pid=13125, effective(12000500, 12000513), real(12000500, 0)] ../source3/modules/vfs_shadow_copy2.c:604(shadow_copy2_do_convert)
> >  Trying[snapdirseverywhere] /fs1/smbexport1/.snapshots/@GMT-2016.08.12-19.56.11/test/: -1 (No such file or directory)
> > [2016/08/15 16:12:36.178886, 10, pid=13125, effective(12000500, 12000513), real(12000500, 0)] ../source3/modules/vfs_shadow_copy2.c:604(shadow_copy2_do_convert)
> >  Trying[snapdirseverywhere] /fs1/.snapshots/@GMT-2016.08.12-19.56.11/smbexport1/test/: -1 (Not a directory)
> > 
> > I will try to take a closer look tomorrow, maybe that can be easily
> > handled in the shadowcopy2 module. The backport should probably wait
> > until this is resolved
> 
> I think this should fix it - can you test ?

Yes, this works.

Reviewed-by: Christof Schmitt <cs at samba.org>

> 
> I'll work on a regression test now.
> 
> Thanks,
> 
> Jeremy.

> From 3b54af0e18172e4b3aae294df0edfe97e572a480 Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Tue, 16 Aug 2016 09:41:43 -0700
> Subject: [PATCH 1/2] s3: vfs: shadow_copy2: Re-use an existing variable
>  already set to the right value (p - name).
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12150
> 
> Signed-off-by: Jeremy Allison <jra at samba.org>
> ---
>  source3/modules/vfs_shadow_copy2.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
> index bda934e..b1aae1e 100644
> --- a/source3/modules/vfs_shadow_copy2.c
> +++ b/source3/modules/vfs_shadow_copy2.c
> @@ -486,7 +486,8 @@ static bool shadow_copy2_strip_snapshot(TALLOC_CTX *mem_ctx,
>  		 * with a path prefix.
>  		 */
>  		if (pstripped != NULL) {
> -			stripped = talloc_strndup(mem_ctx, name, p - name);
> +			stripped = talloc_strndup(mem_ctx, name,
> +					len_before_gmt);
>  			if (stripped == NULL) {
>  				return false;
>  			}
> -- 
> 2.8.0.rc3.226.g39d4020
> 
> 
> From 3b14aa977d069479ceadd912ca4c109887849f20 Mon Sep 17 00:00:00 2001
> From: Jeremy Allison <jra at samba.org>
> Date: Tue, 16 Aug 2016 09:43:37 -0700
> Subject: [PATCH 2/2] s3: vfs: shadow_copy2. Remove any trailing slash when
>  stripping @GMT-YYYY... from the end of a path.
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12150
> 
> Signed-off-by: Jeremy Allison <jra at samba.org>
> ---
>  source3/modules/vfs_shadow_copy2.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
> index b1aae1e..4ac16d3 100644
> --- a/source3/modules/vfs_shadow_copy2.c
> +++ b/source3/modules/vfs_shadow_copy2.c
> @@ -486,6 +486,13 @@ static bool shadow_copy2_strip_snapshot(TALLOC_CTX *mem_ctx,
>  		 * with a path prefix.
>  		 */
>  		if (pstripped != NULL) {
> +			if (len_before_gmt > 0) {
> +				/*
> +				 * There is a slash before
> +				 * the @GMT-. Remove it.
> +				 */
> +				len_before_gmt -= 1;
> +			}
>  			stripped = talloc_strndup(mem_ctx, name,
>  					len_before_gmt);
>  			if (stripped == NULL) {
> -- 
> 2.8.0.rc3.226.g39d4020
>