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

Jeremy Allison jra at samba.org
Wed Aug 17 19:15:40 UTC 2016


On Wed, Aug 17, 2016 at 10:54:15AM -0700, Christof Schmitt wrote:
> On Wed, Aug 17, 2016 at 10:32:38AM -0700, Jeremy Allison wrote:
> > On Wed, Aug 17, 2016 at 10:18:21AM -0700, Christof Schmitt wrote:
> > > 
> > > If cli_set_previous_version(cli, time_t t) already registers the
> > > timestamp that should be used for opens, then the cli code could create
> > > the twrp context from that for SMB2, or in the SMB1 case insert the @GMT
> > > token in the path. That way the protocol differences between SMB1 and
> > > SMB2 would be hidden from the application using the cli.
> > 
> > It's the 'insert the @GMT token in the path' for SMB1 part that is
> > hard. It would touch a *LOT* of client code :-(.
> 
> Ok, that is the issue of SMB1 doing everything based on the path. If we
> want to avoid large changes, then i am not sure how we could have a
> clean API.

So right now, the only use of the shadow_copy client code is
in smbclient where in the 'allinfo' command we get previous
versions and then try and list the meta-data on them, by
constructing an SMB1 path (prepending the @GMT-token).

I'm willing to do something "dirty" such as the dual
cli_set_previous_version(cli, time_t t)/@GMT-YYYY.. path
append inside smbclient for now - for test purposes until
we figure out what the real API should be for users such
as the GNOME VFS etc.

> > I'm working on the snapper fix right now which I'll propose
> > for review today, and then the tests.
> 
> Christof

Here is the fix for the vfs_snapper module.

I think this is the last VFS module fix to all the /@GMT-YYYY-token
append inside SMB2 to work against all supported modules.

I'll work on the test case code next.

Please review !

Jeremy.
-------------- next part --------------
From 5982e72013a1a2e29a99f3d45c8a5ab04c011259 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 17 Aug 2016 10:49:50 -0700
Subject: [PATCH 1/3] s3: vfs: shadow_copy2: Replace all uses of (p-name) with
 len_before_gmt.

p and name don't change, and we've already calculated this length.
Part of the effort to make the code inside vfs_snapper.c that does
the same thing more similar (we can't make these functions identical
due to the 'snapdir' use case).

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 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
index 4ac16d3..2a72740 100644
--- a/source3/modules/vfs_shadow_copy2.c
+++ b/source3/modules/vfs_shadow_copy2.c
@@ -514,7 +514,7 @@ static bool shadow_copy2_strip_snapshot(TALLOC_CTX *mem_ctx,
 	q += 1;
 
 	rest_len = strlen(q);
-	dst_len = (p-name) + rest_len;
+	dst_len = len_before_gmt + rest_len;
 
 	if (priv->config->snapdirseverywhere) {
 		char *insert;
@@ -580,10 +580,10 @@ static bool shadow_copy2_strip_snapshot(TALLOC_CTX *mem_ctx,
 			return false;
 		}
 		if (p > name) {
-			memcpy(stripped, name, p-name);
+			memcpy(stripped, name, len_before_gmt);
 		}
 		if (rest_len > 0) {
-			memcpy(stripped + (p-name), q, rest_len);
+			memcpy(stripped + len_before_gmt, q, rest_len);
 		}
 		stripped[dst_len] = '\0';
 		*pstripped = stripped;
-- 
2.8.0.rc3.226.g39d4020


From 4a20434b0eb591839ea944464ee71d93dbb90507 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 17 Aug 2016 10:53:08 -0700
Subject: [PATCH 2/3] s3: vfs: snapper: Add and use len_before_gmt, calculated
 as (p-name).

Make the code closer to the same functionality in shadow_copy2.c:shadow_copy2_strip_snapshot().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12150

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_snapper.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/source3/modules/vfs_snapper.c b/source3/modules/vfs_snapper.c
index 8f3c647..f5ccc15 100644
--- a/source3/modules/vfs_snapper.c
+++ b/source3/modules/vfs_snapper.c
@@ -1729,6 +1729,7 @@ static bool snapper_gmt_strip_snapshot(TALLOC_CTX *mem_ctx,
 	char *q;
 	char *stripped;
 	size_t rest_len, dst_len;
+	ptrdiff_t len_before_gmt;
 
 	p = strstr_m(name, "@GMT-");
 	if (p == NULL) {
@@ -1737,6 +1738,7 @@ static bool snapper_gmt_strip_snapshot(TALLOC_CTX *mem_ctx,
 	if ((p > name) && (p[-1] != '/')) {
 		goto no_snapshot;
 	}
+	len_before_gmt = p - name;
 	q = strptime(p, GMT_FORMAT, &tm);
 	if (q == NULL) {
 		goto no_snapshot;
@@ -1763,7 +1765,7 @@ static bool snapper_gmt_strip_snapshot(TALLOC_CTX *mem_ctx,
 	q += 1;
 
 	rest_len = strlen(q);
-	dst_len = (p-name) + rest_len;
+	dst_len = len_before_gmt + rest_len;
 
 	if (pstripped != NULL) {
 		stripped = talloc_array(mem_ctx, char, dst_len+1);
@@ -1772,10 +1774,10 @@ static bool snapper_gmt_strip_snapshot(TALLOC_CTX *mem_ctx,
 			return false;
 		}
 		if (p > name) {
-			memcpy(stripped, name, p-name);
+			memcpy(stripped, name, len_before_gmt);
 		}
 		if (rest_len > 0) {
-			memcpy(stripped + (p-name), q, rest_len);
+			memcpy(stripped + len_before_gmt, q, rest_len);
 		}
 		stripped[dst_len] = '\0';
 		*pstripped = stripped;
-- 
2.8.0.rc3.226.g39d4020


From 5161e85884f39840ca5b6275c864cb736d7d9628 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Wed, 17 Aug 2016 10:57:10 -0700
Subject: [PATCH 3/3] s3: vfs: snapper: Fix snapper_gmt_strip_snapshot()
 function to strip @GMT token identically to
 shadow_copy2.c:shadow_copy2_strip_snapshot()

BUG: https://bugzilla.samba.org/show_bug.cgi?id=12150

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_snapper.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/source3/modules/vfs_snapper.c b/source3/modules/vfs_snapper.c
index f5ccc15..5c1821d 100644
--- a/source3/modules/vfs_snapper.c
+++ b/source3/modules/vfs_snapper.c
@@ -1748,9 +1748,23 @@ static bool snapper_gmt_strip_snapshot(TALLOC_CTX *mem_ctx,
 	if (timestamp == (time_t)-1) {
 		goto no_snapshot;
 	}
-	if ((p == name) && (q[0] == '\0')) {
+	if (q[0] == '\0') {
+		/*
+		 * The name consists of only the GMT token or the GMT
+		 * token is at the end of the path. XP seems to send
+		 * @GMT- at the end under certain circumstances even
+		 * with a path prefix.
+		 */
 		if (pstripped != NULL) {
-			stripped = talloc_strdup(mem_ctx, "");
+			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) {
 				return false;
 			}
@@ -1760,6 +1774,10 @@ static bool snapper_gmt_strip_snapshot(TALLOC_CTX *mem_ctx,
 		return true;
 	}
 	if (q[0] != '/') {
+		/*
+		 * It is not a complete path component, i.e. the path
+		 * component continues after the gmt-token.
+		 */
 		goto no_snapshot;
 	}
 	q += 1;
-- 
2.8.0.rc3.226.g39d4020



More information about the samba-technical mailing list