[PATCH] Two dependent bugfixes for VSS bugs 13455 and 13688
Jeremy Allison
jra at samba.org
Fri Nov 23 20:45:47 UTC 2018
Ralph, I thin [PATCH 09/10] s3:smbd: GMT token handling on paths: prefix, don't append
is incorrect.
We used to do this, but found it breaks pathnames with a MS-DFS
prefix (such as \\server\share\path).
If you haven't already, please check this code against a
server set up with MS-DFS pathnames.
If you don't have time before then, let's talk when I'm back in at
work on Monday.
Jeremy.
On Fri, Nov 23, 2018 at 08:51:38PM +0100, Ralph Böhme wrote:
>
> From e3350909ff001ee80d6b50157d7fb8245a1ac868 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 21 Nov 2018 17:13:42 +0100
> Subject: [PATCH 09/10] s3:smbd: GMT token handling on paths: prefix, don't
> append
>
> Appending the GMT token leads to a path parsing error when the path
> contains a stream suffix in check_path_syntax_internal():
>
> while (*s) {
> if (stream_started) {
> switch (*s) {
> case '/':
> case '\\':
> return NT_STATUS_OBJECT_NAME_INVALID;
> case ':':
> if (s[1] == '\0') {
> return NT_STATUS_OBJECT_NAME_INVALID;
> }
> if (strchr_m(&s[1], ':')) {
> return NT_STATUS_OBJECT_NAME_INVALID;
> }
> break;
> }
> }
>
> stream_started is a bool that gets set to true when the ':' stream
> separator is hit. From that point onward any backslash '\' in the
> remaining path is treated as invalid.
>
> Example:
>
> foo:stream\@GMT-2018.11.21-16.18.59
>
> The ':' after "foor" sets stream_started to true, then the '\' after
> "stream" triggers the error.
>
> Fix: prepend the GMT token to the path, that's what we're later doing in
> canonicalize_snapshot_path() anyway. For the above example that results
> in:
>
> @GMT-2018.11.21-16.18.59\foo:stream
>
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13455
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> source3/smbd/smb2_create.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/source3/smbd/smb2_create.c b/source3/smbd/smb2_create.c
> index 7f80f6f8138..21bdcaf44e9 100644
> --- a/source3/smbd/smb2_create.c
> +++ b/source3/smbd/smb2_create.c
> @@ -1193,14 +1193,14 @@ static void smbd_smb2_create_before_exec(struct tevent_req *req)
>
> state->fname = talloc_asprintf(
> state,
> - "%s\\@GMT-%04u.%02u.%02u-%02u.%02u.%02u",
> - state->fname,
> + "@GMT-%04u.%02u.%02u-%02u.%02u.%02u\\%s",
> tm->tm_year + 1900,
> tm->tm_mon + 1,
> tm->tm_mday,
> tm->tm_hour,
> tm->tm_min,
> - tm->tm_sec);
> + tm->tm_sec,
> + state->fname);
> if (tevent_req_nomem(state->fname, req)) {
> return;
> }
> --
> 2.17.2
>
>
> From 199baa9f4d8148c9231b957e7031baf07fb92c44 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 21 Nov 2018 17:20:30 +0100
> Subject: [PATCH 10/10] vfs_shadow_copy2: in fstat also convert fsp->fsp_name
> and fsp->base_fsp->fsp_name
>
> Stacked VFS modules might use the file name, not the file
> handle. Looking at you, vfs_fruit...
>
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=13455
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> selftest/knownfail.d/samba3.blackbox | 1 -
> source3/modules/vfs_shadow_copy2.c | 58 ++++++++++++++++++++++++----
> 2 files changed, 50 insertions(+), 9 deletions(-)
> delete mode 100644 selftest/knownfail.d/samba3.blackbox
>
> diff --git a/selftest/knownfail.d/samba3.blackbox b/selftest/knownfail.d/samba3.blackbox
> deleted file mode 100644
> index a15359e6420..00000000000
> --- a/selftest/knownfail.d/samba3.blackbox
> +++ /dev/null
> @@ -1 +0,0 @@
> -^samba3.blackbox.shadow_copy_torture.reading stream of a shadow copy of a file\(fileserver\)
> diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
> index e105a813e23..0ddc01737bb 100644
> --- a/source3/modules/vfs_shadow_copy2.c
> +++ b/source3/modules/vfs_shadow_copy2.c
> @@ -1354,21 +1354,63 @@ static int shadow_copy2_fstat(vfs_handle_struct *handle, files_struct *fsp,
> SMB_STRUCT_STAT *sbuf)
> {
> time_t timestamp = 0;
> + struct smb_filename *orig_smb_fname = NULL;
> + struct smb_filename vss_smb_fname;
> + struct smb_filename *orig_base_smb_fname = NULL;
> + struct smb_filename vss_base_smb_fname;
> + char *stripped = NULL;
> + int saved_errno = 0;
> + bool ok;
> int ret;
>
> + ok = shadow_copy2_strip_snapshot(talloc_tos(), handle,
> + fsp->fsp_name->base_name,
> + ×tamp, &stripped);
> + if (!ok) {
> + return -1;
> + }
> +
> + if (timestamp == 0) {
> + TALLOC_FREE(stripped);
> + return SMB_VFS_NEXT_FSTAT(handle, fsp, sbuf);
> + }
> +
> + vss_smb_fname = *fsp->fsp_name;
> + vss_smb_fname.base_name = shadow_copy2_convert(talloc_tos(),
> + handle,
> + stripped,
> + timestamp);
> + TALLOC_FREE(stripped);
> + if (vss_smb_fname.base_name == NULL) {
> + return -1;
> + }
> +
> + orig_smb_fname = fsp->fsp_name;
> + fsp->fsp_name = &vss_smb_fname;
> +
> + if (fsp->base_fsp != NULL) {
> + vss_base_smb_fname = *fsp->base_fsp->fsp_name;
> + vss_base_smb_fname.base_name = vss_smb_fname.base_name;
> + orig_base_smb_fname = fsp->base_fsp->fsp_name;
> + fsp->base_fsp->fsp_name = &vss_base_smb_fname;
> + }
> +
> ret = SMB_VFS_NEXT_FSTAT(handle, fsp, sbuf);
> - if (ret == -1) {
> - return ret;
> + fsp->fsp_name = orig_smb_fname;
> + if (fsp->base_fsp != NULL) {
> + fsp->base_fsp->fsp_name = orig_base_smb_fname;
> }
> - if (!shadow_copy2_strip_snapshot(talloc_tos(), handle,
> - fsp->fsp_name->base_name,
> - ×tamp, NULL)) {
> - return 0;
> + if (ret == -1) {
> + saved_errno = errno;
> }
> - if (timestamp != 0) {
> +
> + if (ret == 0) {
> convert_sbuf(handle, fsp->fsp_name->base_name, sbuf);
> }
> - return 0;
> + if (saved_errno != 0) {
> + errno = saved_errno;
> + }
> + return ret;
> }
>
> static int shadow_copy2_open(vfs_handle_struct *handle,
> --
> 2.17.2
>
More information about the samba-technical
mailing list