[PATCH] smbd: Fix snapshot query on shares with DFS enabled
Christof Schmitt
cs at samba.org
Wed Aug 17 20:48:38 UTC 2016
On Wed, Aug 17, 2016 at 12:15:40PM -0700, Jeremy Allison wrote:
> 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 !
Reviewed-by: Christof Schmitt <cs at samba.org>
>
> Jeremy.
> 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