From d3c51f2eaaf40653317f2e24d3b243fc3b7f337e Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Wed, 11 Jan 2017 16:30:38 -0800 Subject: [PATCH 01/16] s3: smbd: Correctly canonicalize any incoming shadow copy path. Converts to: @GMT-token/path/last_component from all incoming path types. Allows shadow_copy modules to work when current directory is changed after removing last component. Ultimately when the VFS ABI is changed to add a timestamp to struct smb_filename, this is where the parsing will be done. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/smbd/filename.c | 150 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/source3/smbd/filename.c b/source3/smbd/filename.c index a8cfb55b0f0..efe52a04328 100644 --- a/source3/smbd/filename.c +++ b/source3/smbd/filename.c @@ -220,6 +220,148 @@ static NTSTATUS check_parent_exists(TALLOC_CTX *ctx, return NT_STATUS_OK; } +/* + * Re-order a known good @GMT-token path. + */ + +static NTSTATUS rearrange_snapshot_path(struct smb_filename *smb_fname, + char *startp, + char *endp) +{ + size_t endlen = 0; + size_t gmt_len = endp - startp; + char gmt_store[gmt_len + 1]; + char *parent = NULL; + const char *last_component = NULL; + char *newstr; + bool ret; + + DBG_DEBUG("|%s| -> ", smb_fname->base_name); + + /* Save off the @GMT-token. */ + memcpy(gmt_store, startp, gmt_len); + gmt_store[gmt_len] = '\0'; + + if (*endp == '/') { + /* Remove any trailing '/' */ + endp++; + } + + if (*endp == '\0') { + /* + * @GMT-token was at end of path. + * Remove any preceeding '/' + */ + if (startp > smb_fname->base_name && startp[-1] == '/') { + startp--; + } + } + + /* Remove @GMT-token from the path. */ + endlen = strlen(endp); + memmove(startp, endp, endlen + 1); + + /* Split the remaining path into components. */ + ret = parent_dirname(smb_fname, + smb_fname->base_name, + &parent, + &last_component); + if (ret == false) { + /* Must terminate debug with \n */ + DBG_DEBUG("NT_STATUS_NO_MEMORY\n"); + return NT_STATUS_NO_MEMORY; + } + + if (ISDOT(parent)) { + if (last_component[0] == '\0') { + newstr = talloc_strdup(smb_fname, + gmt_store); + } else { + newstr = talloc_asprintf(smb_fname, + "%s/%s", + gmt_store, + last_component); + } + } else { + newstr = talloc_asprintf(smb_fname, + "%s/%s/%s", + gmt_store, + parent, + last_component); + } + + TALLOC_FREE(parent); + TALLOC_FREE(smb_fname->base_name); + smb_fname->base_name = newstr; + + DBG_DEBUG("|%s|\n", newstr); + + return NT_STATUS_OK; +} + +/* + * Canonicalize any incoming pathname potentially containining + * a @GMT-token into a path that looks like: + * + * @GMT-YYYY-MM-DD-HH-MM-SS/path/name/components/last_component + * + * Leaves single path @GMT-token -component alone: + * + * @GMT-YYYY-MM-DD-HH-MM-SS -> @GMT-YYYY-MM-DD-HH-MM-SS + * + * Eventually when struct smb_filename is updated and the VFS + * ABI is changed this will remove the @GMT-YYYY-MM-DD-HH-MM-SS + * and store in the struct smb_filename as a struct timeval field + * instead. + */ + +static NTSTATUS canonicalize_snapshot_path(struct smb_filename *smb_fname) +{ + char *startp = strchr_m(smb_fname->base_name, '@'); + char *endp = NULL; + struct tm tm; + + if (startp == NULL) { + /* No @ */ + return NT_STATUS_OK; + } + + startp = strstr_m(startp, "@GMT-"); + if (startp == NULL) { + /* No @ */ + return NT_STATUS_OK; + } + + if ((startp > smb_fname->base_name) && (startp[-1] != '/')) { + /* the GMT-token does not start a path-component */ + return NT_STATUS_OK; + } + + endp = strptime(startp, GMT_FORMAT, &tm); + if (endp == NULL) { + /* Not a valid timestring. */ + return NT_STATUS_OK; + } + + if ( endp[0] == '\0') { + return rearrange_snapshot_path(smb_fname, + startp, + endp); + } + + if (endp[0] != '/') { + /* + * It is not a complete path component, i.e. the path + * component continues after the gmt-token. + */ + return NT_STATUS_OK; + } + + return rearrange_snapshot_path(smb_fname, + startp, + endp); +} + /**************************************************************************** This routine is called to convert names from the dos namespace to unix namespace. It needs to handle any case conversions, mangling, format changes, @@ -356,6 +498,14 @@ NTSTATUS unix_convert(TALLOC_CTX *ctx, goto err; } + /* Canonicalize any @GMT- paths. */ + if (posix_pathnames == false) { + status = canonicalize_snapshot_path(smb_fname); + if (!NT_STATUS_IS_OK(status)) { + goto err; + } + } + /* * Large directory fix normalization. If we're case sensitive, and * the case preserving parameters are set to "no", normalize the case of -- 2.11.0.483.g087da7b7c-goog From 509a2d22048bdd5088a43adcf4a2aea06ca17826 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 17 Jan 2017 11:33:18 -0800 Subject: [PATCH 02/16] s3: lib: Add canonicalize_absolute_path(). Resolves any invalid path components (.) (..) in an absolute POSIX path. We will be re-using this in several places. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/lib/util_path.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++ source3/lib/util_path.h | 1 + 2 files changed, 134 insertions(+) diff --git a/source3/lib/util_path.c b/source3/lib/util_path.c index 509ba5ff727..cbad2e15d48 100644 --- a/source3/lib/util_path.c +++ b/source3/lib/util_path.c @@ -93,3 +93,136 @@ char *cache_path(const char *name) { return xx_path(name, lp_cache_directory()); } + +/** + * @brief Removes any invalid path components in an absolute POSIX path. + * + * @param ctx Talloc context to return string. + * + * @param abs_path Absolute path string to process. + * + * @retval Pointer to a talloc'ed string containing the absolute full path. + **/ + +char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *abs_path) +{ + char *destname; + char *d; + const char *s = abs_path; + bool start_of_name_component = true; + + /* Allocate for strlen + '\0' + possible leading '/' */ + destname = (char *)talloc_size(ctx, strlen(abs_path) + 2); + if (destname == NULL) { + return NULL; + } + d = destname; + + *d++ = '/'; /* Always start with root. */ + + while (*s) { + if (*s == '/') { + /* Eat multiple '/' */ + while (*s == '/') { + s++; + } + if ((d > destname + 1) && (*s != '\0')) { + *d++ = '/'; + } + start_of_name_component = true; + continue; + } + + if (start_of_name_component) { + if ((s[0] == '.') && (s[1] == '.') && + (s[2] == '/' || s[2] == '\0')) { + /* Uh oh - "/../" or "/..\0" ! */ + + /* Go past the ../ or .. */ + if (s[2] == '/') { + s += 3; + } else { + s += 2; /* Go past the .. */ + } + + /* If we just added a '/' - delete it */ + if ((d > destname) && (*(d-1) == '/')) { + *(d-1) = '\0'; + d--; + } + + /* + * Are we at the start ? + * Can't go back further if so. + */ + if (d <= destname) { + *d++ = '/'; /* Can't delete root */ + continue; + } + /* Go back one level... */ + /* + * Decrement d first as d points to + * the *next* char to write into. + */ + for (d--; d > destname; d--) { + if (*d == '/') { + break; + } + } + /* + * We're still at the start of a name + * component, just the previous one. + */ + continue; + } else if ((s[0] == '.') && + ((s[1] == '\0') || s[1] == '/')) { + /* + * Component of pathname can't be "." only. + * Skip the '.' . + */ + if (s[1] == '/') { + s += 2; + } else { + s++; + } + continue; + } + } + + if (!(*s & 0x80)) { + *d++ = *s++; + } else { + size_t siz; + /* Get the size of the next MB character. */ + next_codepoint(s,&siz); + switch(siz) { + case 5: + *d++ = *s++; + /*fall through*/ + case 4: + *d++ = *s++; + /*fall through*/ + case 3: + *d++ = *s++; + /*fall through*/ + case 2: + *d++ = *s++; + /*fall through*/ + case 1: + *d++ = *s++; + break; + default: + break; + } + } + start_of_name_component = false; + } + *d = '\0'; + + /* And must not end in '/' */ + if (d > destname + 1 && (*(d-1) == '/')) { + *(d-1) = '\0'; + } + + return destname; +} diff --git a/source3/lib/util_path.h b/source3/lib/util_path.h index 118a4bed524..16e27926084 100644 --- a/source3/lib/util_path.h +++ b/source3/lib/util_path.h @@ -27,5 +27,6 @@ char *lock_path(const char *name); char *state_path(const char *name); char *cache_path(const char *name); +char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *abs_path); #endif -- 2.11.0.483.g087da7b7c-goog From fc8d47240557a2e4ed98535d7ebc53b001debd1f Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Thu, 19 Jan 2017 15:18:41 -0800 Subject: [PATCH 03/16] s3: lib: Fix old, old bug in set_conn_connectpath(), now in canonicalize_absolute_path(). Canonicalizing a path of /foo/bar/../baz would return /foo/barbaz as moving forward 3 characters would delete the / character. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/lib/util_path.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/source3/lib/util_path.c b/source3/lib/util_path.c index cbad2e15d48..7665954afbb 100644 --- a/source3/lib/util_path.c +++ b/source3/lib/util_path.c @@ -138,12 +138,8 @@ char *canonicalize_absolute_path(TALLOC_CTX *ctx, const char *abs_path) (s[2] == '/' || s[2] == '\0')) { /* Uh oh - "/../" or "/..\0" ! */ - /* Go past the ../ or .. */ - if (s[2] == '/') { - s += 3; - } else { - s += 2; /* Go past the .. */ - } + /* Go past the .. leaving us on the / or '\0' */ + s += 2; /* If we just added a '/' - delete it */ if ((d > destname) && (*(d-1) == '/')) { -- 2.11.0.483.g087da7b7c-goog From 3e1ec8258edda663a5f4667845c6a513faabacc9 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Tue, 17 Jan 2017 11:35:52 -0800 Subject: [PATCH 04/16] s3: smbd: Make set_conn_connectpath() call canonicalize_absolute_path(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/smbd/service.c | 103 ++----------------------------------------------- 1 file changed, 3 insertions(+), 100 deletions(-) diff --git a/source3/smbd/service.c b/source3/smbd/service.c index 3308e9dce97..a0ac9dfa2af 100644 --- a/source3/smbd/service.c +++ b/source3/smbd/service.c @@ -31,6 +31,7 @@ #include "lib/param/loadparm.h" #include "messages.h" #include "lib/afs/afs_funcs.h" +#include "lib/util_path.h" static bool canonicalize_connect_path(connection_struct *conn) { @@ -47,118 +48,20 @@ static bool canonicalize_connect_path(connection_struct *conn) /**************************************************************************** Ensure when setting connectpath it is a canonicalized (no ./ // or ../) absolute path stating in / and not ending in /. - Observent people will notice a similarity between this and check_path_syntax :-). ****************************************************************************/ bool set_conn_connectpath(connection_struct *conn, const char *connectpath) { char *destname; - char *d; - const char *s = connectpath; - bool start_of_name_component = true; if (connectpath == NULL || connectpath[0] == '\0') { return false; } - /* Allocate for strlen + '\0' + possible leading '/' */ - destname = (char *)talloc_size(conn, strlen(connectpath) + 2); - if (!destname) { + destname = canonicalize_absolute_path(conn, connectpath); + if (destname == NULL) { return false; } - d = destname; - - *d++ = '/'; /* Always start with root. */ - - while (*s) { - if (*s == '/') { - /* Eat multiple '/' */ - while (*s == '/') { - s++; - } - if ((d > destname + 1) && (*s != '\0')) { - *d++ = '/'; - } - start_of_name_component = True; - continue; - } - - if (start_of_name_component) { - if ((s[0] == '.') && (s[1] == '.') && (s[2] == '/' || s[2] == '\0')) { - /* Uh oh - "/../" or "/..\0" ! */ - - /* Go past the ../ or .. */ - if (s[2] == '/') { - s += 3; - } else { - s += 2; /* Go past the .. */ - } - - /* If we just added a '/' - delete it */ - if ((d > destname) && (*(d-1) == '/')) { - *(d-1) = '\0'; - d--; - } - - /* Are we at the start ? Can't go back further if so. */ - if (d <= destname) { - *d++ = '/'; /* Can't delete root */ - continue; - } - /* Go back one level... */ - /* Decrement d first as d points to the *next* char to write into. */ - for (d--; d > destname; d--) { - if (*d == '/') { - break; - } - } - /* We're still at the start of a name component, just the previous one. */ - continue; - } else if ((s[0] == '.') && ((s[1] == '\0') || s[1] == '/')) { - /* Component of pathname can't be "." only - skip the '.' . */ - if (s[1] == '/') { - s += 2; - } else { - s++; - } - continue; - } - } - - if (!(*s & 0x80)) { - *d++ = *s++; - } else { - size_t siz; - /* Get the size of the next MB character. */ - next_codepoint(s,&siz); - switch(siz) { - case 5: - *d++ = *s++; - /*fall through*/ - case 4: - *d++ = *s++; - /*fall through*/ - case 3: - *d++ = *s++; - /*fall through*/ - case 2: - *d++ = *s++; - /*fall through*/ - case 1: - *d++ = *s++; - break; - default: - break; - } - } - start_of_name_component = false; - } - *d = '\0'; - - /* And must not end in '/' */ - if (d > destname + 1 && (*(d-1) == '/')) { - *(d-1) = '\0'; - } DEBUG(10,("set_conn_connectpath: service %s, connectpath = %s\n", lp_servicename(talloc_tos(), SNUM(conn)), destname )); -- 2.11.0.483.g087da7b7c-goog From 57dcc43f66671e76bf9ec5c6488c3d8b644f5917 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 20 Jan 2017 11:42:39 -0800 Subject: [PATCH 05/16] s3: VFS: shadow_copy2: Correctly initialize timestamp and stripped variables. Allow the called functions to be fixed to not touch them on error. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/modules/vfs_shadow_copy2.c | 107 +++++++++++++++++++------------------ 1 file changed, 55 insertions(+), 52 deletions(-) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index 6a25c87b7f7..10c8e2e1c11 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -416,10 +416,10 @@ static bool shadow_copy2_strip_snapshot(TALLOC_CTX *mem_ctx, char **pstripped) { struct tm tm; - time_t timestamp; + time_t timestamp = 0; const char *p; char *q; - char *stripped; + char *stripped = NULL; size_t rest_len, dst_len; struct shadow_copy2_private *priv; const char *snapdir; @@ -893,8 +893,8 @@ static DIR *shadow_copy2_opendir(vfs_handle_struct *handle, const char *mask, uint32_t attr) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; DIR *ret; int saved_errno; char *conv; @@ -1002,8 +1002,9 @@ static int shadow_copy2_link(vfs_handle_struct *handle, static int shadow_copy2_stat(vfs_handle_struct *handle, struct smb_filename *smb_fname) { - time_t timestamp; - char *stripped, *tmp; + time_t timestamp = 0; + char *stripped = NULL; + char *tmp; int ret, saved_errno; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, @@ -1041,8 +1042,9 @@ static int shadow_copy2_stat(vfs_handle_struct *handle, static int shadow_copy2_lstat(vfs_handle_struct *handle, struct smb_filename *smb_fname) { - time_t timestamp; - char *stripped, *tmp; + time_t timestamp = 0; + char *stripped = NULL; + char *tmp; int ret, saved_errno; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, @@ -1080,7 +1082,7 @@ static int shadow_copy2_lstat(vfs_handle_struct *handle, static int shadow_copy2_fstat(vfs_handle_struct *handle, files_struct *fsp, SMB_STRUCT_STAT *sbuf) { - time_t timestamp; + time_t timestamp = 0; int ret; ret = SMB_VFS_NEXT_FSTAT(handle, fsp, sbuf); @@ -1102,8 +1104,9 @@ static int shadow_copy2_open(vfs_handle_struct *handle, struct smb_filename *smb_fname, files_struct *fsp, int flags, mode_t mode) { - time_t timestamp; - char *stripped, *tmp; + time_t timestamp = 0; + char *stripped = NULL; + char *tmp; int ret, saved_errno; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, @@ -1138,8 +1141,8 @@ static int shadow_copy2_open(vfs_handle_struct *handle, static int shadow_copy2_unlink(vfs_handle_struct *handle, const struct smb_filename *smb_fname) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; int ret, saved_errno; struct smb_filename *conv; @@ -1173,7 +1176,7 @@ static int shadow_copy2_chmod(vfs_handle_struct *handle, const struct smb_filename *smb_fname, mode_t mode) { - time_t timestamp; + time_t timestamp = 0; char *stripped = NULL; int ret, saved_errno; char *conv = NULL; @@ -1219,8 +1222,8 @@ static int shadow_copy2_chown(vfs_handle_struct *handle, uid_t uid, gid_t gid) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; int ret, saved_errno; char *conv = NULL; struct smb_filename *conv_smb_fname = NULL; @@ -1261,8 +1264,8 @@ static int shadow_copy2_chown(vfs_handle_struct *handle, static int shadow_copy2_chdir(vfs_handle_struct *handle, const char *fname) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; int ret, saved_errno; char *conv; @@ -1289,8 +1292,8 @@ static int shadow_copy2_ntimes(vfs_handle_struct *handle, const struct smb_filename *smb_fname, struct smb_file_time *ft) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; int ret, saved_errno; struct smb_filename *conv; @@ -1323,8 +1326,8 @@ static int shadow_copy2_ntimes(vfs_handle_struct *handle, static int shadow_copy2_readlink(vfs_handle_struct *handle, const char *fname, char *buf, size_t bufsiz) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; int ret, saved_errno; char *conv; @@ -1350,8 +1353,8 @@ static int shadow_copy2_readlink(vfs_handle_struct *handle, static int shadow_copy2_mknod(vfs_handle_struct *handle, const char *fname, mode_t mode, SMB_DEV_T dev) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; int ret, saved_errno; char *conv; @@ -1377,7 +1380,7 @@ static int shadow_copy2_mknod(vfs_handle_struct *handle, static char *shadow_copy2_realpath(vfs_handle_struct *handle, const char *fname) { - time_t timestamp; + time_t timestamp = 0; char *stripped = NULL; char *tmp = NULL; char *result = NULL; @@ -1805,8 +1808,8 @@ static NTSTATUS shadow_copy2_fget_nt_acl(vfs_handle_struct *handle, TALLOC_CTX *mem_ctx, struct security_descriptor **ppdesc) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; NTSTATUS status; char *conv; struct smb_filename *smb_fname = NULL; @@ -1849,8 +1852,8 @@ static NTSTATUS shadow_copy2_get_nt_acl(vfs_handle_struct *handle, TALLOC_CTX *mem_ctx, struct security_descriptor **ppdesc) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; NTSTATUS status; char *conv; struct smb_filename *conv_smb_fname = NULL; @@ -1891,8 +1894,8 @@ static int shadow_copy2_mkdir(vfs_handle_struct *handle, const struct smb_filename *smb_fname, mode_t mode) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; int ret, saved_errno; char *conv; struct smb_filename *conv_smb_fname = NULL; @@ -1932,8 +1935,8 @@ static int shadow_copy2_mkdir(vfs_handle_struct *handle, static int shadow_copy2_rmdir(vfs_handle_struct *handle, const struct smb_filename *smb_fname) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; int ret, saved_errno; char *conv; struct smb_filename *conv_smb_fname = NULL; @@ -1973,8 +1976,8 @@ static int shadow_copy2_rmdir(vfs_handle_struct *handle, static int shadow_copy2_chflags(vfs_handle_struct *handle, const char *fname, unsigned int flags) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; int ret, saved_errno; char *conv; @@ -2001,8 +2004,8 @@ static ssize_t shadow_copy2_getxattr(vfs_handle_struct *handle, const char *fname, const char *aname, void *value, size_t size) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; ssize_t ret; int saved_errno; char *conv; @@ -2031,8 +2034,8 @@ static ssize_t shadow_copy2_listxattr(struct vfs_handle_struct *handle, const char *fname, char *list, size_t size) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; ssize_t ret; int saved_errno; char *conv; @@ -2059,8 +2062,8 @@ static ssize_t shadow_copy2_listxattr(struct vfs_handle_struct *handle, static int shadow_copy2_removexattr(vfs_handle_struct *handle, const char *fname, const char *aname) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; int ret, saved_errno; char *conv; @@ -2088,8 +2091,8 @@ static int shadow_copy2_setxattr(struct vfs_handle_struct *handle, const char *aname, const void *value, size_t size, int flags) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; ssize_t ret; int saved_errno; char *conv; @@ -2118,8 +2121,8 @@ static int shadow_copy2_chmod_acl(vfs_handle_struct *handle, const struct smb_filename *smb_fname, mode_t mode) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; ssize_t ret; int saved_errno; char *conv = NULL; @@ -2164,8 +2167,8 @@ static int shadow_copy2_get_real_filename(struct vfs_handle_struct *handle, TALLOC_CTX *mem_ctx, char **found_name) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; ssize_t ret; int saved_errno; char *conv; @@ -2203,7 +2206,7 @@ static int shadow_copy2_get_real_filename(struct vfs_handle_struct *handle, static const char *shadow_copy2_connectpath(struct vfs_handle_struct *handle, const char *fname) { - time_t timestamp; + time_t timestamp = 0; char *stripped = NULL; char *tmp = NULL; char *result = NULL; @@ -2278,8 +2281,8 @@ static uint64_t shadow_copy2_disk_free(vfs_handle_struct *handle, const char *path, uint64_t *bsize, uint64_t *dfree, uint64_t *dsize) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; ssize_t ret; int saved_errno; char *conv; @@ -2312,8 +2315,8 @@ static int shadow_copy2_get_quota(vfs_handle_struct *handle, const char *path, enum SMB_QUOTA_TYPE qtype, unid_t id, SMB_DISK_QUOTA *dq) { - time_t timestamp; - char *stripped; + time_t timestamp = 0; + char *stripped = NULL; int ret; int saved_errno; char *conv; -- 2.11.0.483.g087da7b7c-goog From 89597b26c719965682b3500db89351c60a905568 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 20 Jan 2017 11:45:54 -0800 Subject: [PATCH 06/16] s3: VFS: shadow_copy2: Ensure pathnames for parameters are correctly relative and terminated. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/modules/vfs_shadow_copy2.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index 10c8e2e1c11..34b86068dae 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -2623,6 +2623,11 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle, } } + trim_string(config->mount_point, NULL, "/"); + trim_string(config->rel_connectpath, "/", "/"); + trim_string(config->snapdir, NULL, "/"); + trim_string(config->snapshot_basepath, NULL, "/"); + DEBUG(10, ("shadow_copy2_connect: configuration:\n" " share root: '%s'\n" " mountpoint: '%s'\n" -- 2.11.0.483.g087da7b7c-goog From 625480f3fcf6c5e2eca8cd655cec53d69956d97d Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 20 Jan 2017 11:48:40 -0800 Subject: [PATCH 07/16] s3: VFS: shadow_copy2: Fix length comparison to ensure we don't overstep a length. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/modules/vfs_shadow_copy2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index 34b86068dae..27cd4f72b81 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -2585,7 +2585,7 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle, } if (config->rel_connectpath == NULL && - strlen(basedir) != strlen(handle->conn->connectpath)) { + strlen(basedir) < strlen(handle->conn->connectpath)) { config->rel_connectpath = talloc_strdup(config, handle->conn->connectpath + strlen(basedir)); if (config->rel_connectpath == NULL) { -- 2.11.0.483.g087da7b7c-goog From 60ddd99e7e638fabb3955c3a71c8e908edf93785 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 20 Jan 2017 11:50:49 -0800 Subject: [PATCH 08/16] s3: VFS: shadow_copy2: Add two new variables to the private data. Not yet used. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/modules/vfs_shadow_copy2.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index 27cd4f72b81..f957d3c6cb9 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -74,6 +74,9 @@ struct shadow_copy2_snaplist_info { struct shadow_copy2_private { struct shadow_copy2_config *config; struct shadow_copy2_snaplist_info *snaps; + char *shadow_cwd; /* Absolute $cwd path. */ + /* Absolute connectpath - can vary depending on $cwd. */ + char *shadow_connectpath; }; static int shadow_copy2_get_shadow_copy_data( -- 2.11.0.483.g087da7b7c-goog From d5accc59b31f68126c324ed983e64c40da790ca0 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 20 Jan 2017 11:54:56 -0800 Subject: [PATCH 09/16] s3: VFS: shadow_copy2: Add a wrapper function to call the original shadow_copy2_strip_snapshot(). BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Allows an extra (currently unused) parameter to be added. Signed-off-by: Jeremy Allison --- source3/modules/vfs_shadow_copy2.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index f957d3c6cb9..5031ab36a6c 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -412,11 +412,12 @@ static char *shadow_copy2_snapshot_path(TALLOC_CTX *mem_ctx, * handed in via the smb layer. * Returns the parsed timestamp and the stripped filename. */ -static bool shadow_copy2_strip_snapshot(TALLOC_CTX *mem_ctx, +static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, struct vfs_handle_struct *handle, const char *name, time_t *ptimestamp, - char **pstripped) + char **pstripped, + char **psnappath) { struct tm tm; time_t timestamp = 0; @@ -598,6 +599,20 @@ no_snapshot: return true; } +static bool shadow_copy2_strip_snapshot(TALLOC_CTX *mem_ctx, + struct vfs_handle_struct *handle, + const char *orig_name, + time_t *ptimestamp, + char **pstripped) +{ + return shadow_copy2_strip_snapshot_internal(mem_ctx, + handle, + orig_name, + ptimestamp, + pstripped, + NULL); +} + static char *shadow_copy2_find_mount_point(TALLOC_CTX *mem_ctx, vfs_handle_struct *handle) { -- 2.11.0.483.g087da7b7c-goog From 1d467d7cf03bf30655e64a5247a5c39e391961ed Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 20 Jan 2017 11:56:21 -0800 Subject: [PATCH 10/16] s3: VFS: shadow_copy2: Change a parameter name. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Allows easy substitution later. Signed-off-by: Jeremy Allison --- 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 5031ab36a6c..a94a0ae5605 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -414,7 +414,7 @@ static char *shadow_copy2_snapshot_path(TALLOC_CTX *mem_ctx, */ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, struct vfs_handle_struct *handle, - const char *name, + const char *orig_name, time_t *ptimestamp, char **pstripped, char **psnappath) @@ -429,6 +429,7 @@ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, const char *snapdir; ssize_t snapdirlen; ptrdiff_t len_before_gmt; + const char *name = orig_name; SMB_VFS_HANDLE_GET_DATA(handle, priv, struct shadow_copy2_private, return false); -- 2.11.0.483.g087da7b7c-goog From eac45171e28d8e0e4a072b1d993cffe41cb4b2d8 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 20 Jan 2017 12:00:08 -0800 Subject: [PATCH 11/16] s3: VFS: shadow_copy2: Add two currently unused functions to make pathnames absolute or relative to $cwd. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/modules/vfs_shadow_copy2.c | 45 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index a94a0ae5605..e781612df88 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -35,6 +35,7 @@ #include "system/filesys.h" #include "include/ntioctl.h" #include "util_tdb.h" +#include "lib/util_path.h" struct shadow_copy2_config { char *gmt_format; @@ -407,6 +408,50 @@ static char *shadow_copy2_snapshot_path(TALLOC_CTX *mem_ctx, return result; } +static char *make_path_absolute(TALLOC_CTX *mem_ctx, + struct shadow_copy2_private *priv, + const char *name) +{ + char *newpath = NULL; + char *abs_path = NULL; + + if (name[0] != '/') { + newpath = talloc_asprintf(mem_ctx, + "%s/%s", + priv->shadow_cwd, + name); + if (newpath == NULL) { + return NULL; + } + name = newpath; + } + abs_path = canonicalize_absolute_path(mem_ctx, name); + TALLOC_FREE(newpath); + return abs_path; +} + +/* Return a $cwd-relative path. */ +static bool make_relative_path(const char *cwd, char *abs_path) +{ + size_t cwd_len = strlen(cwd); + size_t abs_len = strlen(abs_path); + + if (abs_len < cwd_len) { + return false; + } + if (memcmp(abs_path, cwd, cwd_len) != 0) { + return false; + } + if (abs_path[cwd_len] != '/' && abs_path[cwd_len] != '\0') { + return false; + } + if (abs_path[cwd_len] == '/') { + cwd_len++; + } + memmove(abs_path, &abs_path[cwd_len], abs_len + 1 - cwd_len); + return true; +} + /** * Strip a snapshot component from a filename as * handed in via the smb layer. -- 2.11.0.483.g087da7b7c-goog From e67a79eb9e400b438c4b15c6a6c511a8cd8fb125 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 20 Jan 2017 12:06:55 -0800 Subject: [PATCH 12/16] s3: VFS: shadow_copy2: Fix chdir to store off the needed private variables. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 This is not yet used, the users of this will be added later. Signed-off-by: Jeremy Allison --- source3/modules/vfs_shadow_copy2.c | 81 ++++++++++++++++++++++++++++++++------ 1 file changed, 68 insertions(+), 13 deletions(-) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index e781612df88..d7037a9e232 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -1325,30 +1325,85 @@ static int shadow_copy2_chown(vfs_handle_struct *handle, return ret; } +static void store_cwd_data(vfs_handle_struct *handle, + const char *connectpath) +{ + struct shadow_copy2_private *priv = NULL; + char *cwd = NULL; + + SMB_VFS_HANDLE_GET_DATA(handle, priv, struct shadow_copy2_private, + return); + + TALLOC_FREE(priv->shadow_cwd); + cwd = SMB_VFS_NEXT_GETWD(handle); + if (cwd == NULL) { + smb_panic("getwd failed\n"); + } + DBG_DEBUG("shadow cwd = %s\n", cwd); + priv->shadow_cwd = talloc_strdup(priv, cwd); + SAFE_FREE(cwd); + if (priv->shadow_cwd == NULL) { + smb_panic("talloc failed\n"); + } + TALLOC_FREE(priv->shadow_connectpath); + if (connectpath) { + DBG_DEBUG("shadow conectpath = %s\n", connectpath); + priv->shadow_connectpath = talloc_strdup(priv, connectpath); + if (priv->shadow_connectpath == NULL) { + smb_panic("talloc failed\n"); + } + } +} + static int shadow_copy2_chdir(vfs_handle_struct *handle, const char *fname) { time_t timestamp = 0; char *stripped = NULL; - int ret, saved_errno; - char *conv; + char *snappath = NULL; + int ret = -1; + int saved_errno = 0; + char *conv = NULL; + size_t rootpath_len = 0; - if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname, - ×tamp, &stripped)) { + if (!shadow_copy2_strip_snapshot_internal(talloc_tos(), handle, fname, + ×tamp, &stripped, &snappath)) { return -1; } - if (timestamp == 0) { - return SMB_VFS_NEXT_CHDIR(handle, fname); + if (stripped != NULL) { + conv = shadow_copy2_do_convert(talloc_tos(), + handle, + stripped, + timestamp, + &rootpath_len); + TALLOC_FREE(stripped); + if (conv == NULL) { + return -1; + } + fname = conv; } - conv = shadow_copy2_convert(talloc_tos(), handle, stripped, timestamp); - TALLOC_FREE(stripped); - if (conv == NULL) { - return -1; + + ret = SMB_VFS_NEXT_CHDIR(handle, fname); + if (ret == -1) { + saved_errno = errno; } - ret = SMB_VFS_NEXT_CHDIR(handle, conv); - saved_errno = errno; + + if (ret == 0) { + if (conv != NULL && rootpath_len != 0) { + conv[rootpath_len] = '\0'; + } else if (snappath != 0) { + TALLOC_FREE(conv); + conv = snappath; + } + store_cwd_data(handle, conv); + } + + TALLOC_FREE(stripped); TALLOC_FREE(conv); - errno = saved_errno; + + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } -- 2.11.0.483.g087da7b7c-goog From 6f488c8b41deb53fe7ac6bc2874acacd15611441 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 20 Jan 2017 12:09:08 -0800 Subject: [PATCH 13/16] s3: VFS: Allow shadow_copy2_connectpath() to return the cached path derived from $cwd. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/modules/vfs_shadow_copy2.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index d7037a9e232..8ac6971cfe4 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -2332,9 +2332,19 @@ static const char *shadow_copy2_connectpath(struct vfs_handle_struct *handle, char *parent_dir = NULL; int saved_errno; size_t rootpath_len = 0; + struct shadow_copy2_private *priv = NULL; + + SMB_VFS_HANDLE_GET_DATA(handle, priv, struct shadow_copy2_private, + return NULL); DBG_DEBUG("Calc connect path for [%s]\n", fname); + if (priv->shadow_connectpath != NULL) { + DBG_DEBUG("cached connect path is [%s]\n", + priv->shadow_connectpath); + return priv->shadow_connectpath; + } + if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname, ×tamp, &stripped)) { goto done; -- 2.11.0.483.g087da7b7c-goog From a4bc4f74821c730064d5d6ac3fc5615289a29d52 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 20 Jan 2017 12:33:23 -0800 Subject: [PATCH 14/16] s3: VFS: shadow_copy2: Fix module to work with variable current working directory. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/modules/vfs_shadow_copy2.c | 148 ++++++++++++++++++++++++++++++------- 1 file changed, 122 insertions(+), 26 deletions(-) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index 8ac6971cfe4..89a73d09b0a 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -453,9 +453,23 @@ static bool make_relative_path(const char *cwd, char *abs_path) } /** - * Strip a snapshot component from a filename as - * handed in via the smb layer. - * Returns the parsed timestamp and the stripped filename. + * This function does two things. + * + * 1). Checks if an incoming filename contains an + * SMB-layer @GMT- style timestamp. + * If so, it strips the timestamp, and returns + * both the timestamp and the stripped path + * (making it cwd-relative). + * + * 2). Checks if an incoming filename is already a + * snapshot converted pathname. + * If so, it returns the pathname truncated + * at the snapshot point which will be used + * as the connectpath, and then does an early return. + * + * FIXME. This function is an utter mess, and is in desperate + * need of a complete refactoring. A task for another day + * though. */ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, struct vfs_handle_struct *handle, @@ -475,22 +489,33 @@ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, ssize_t snapdirlen; ptrdiff_t len_before_gmt; const char *name = orig_name; + char *abs_path = NULL; + int ret = true; SMB_VFS_HANDLE_GET_DATA(handle, priv, struct shadow_copy2_private, return false); DEBUG(10, (__location__ ": enter path '%s'\n", name)); + abs_path = make_path_absolute(mem_ctx, priv, name); + if (abs_path == NULL) { + ret = false; + goto out; + } + name = abs_path; + + DEBUG(10, (__location__ ": abs path '%s'\n", name)); + p = strstr_m(name, "@GMT-"); if (p == NULL) { DEBUG(11, ("@GMT not found\n")); - goto no_snapshot; + goto out; } if ((p > name) && (p[-1] != '/')) { /* the GMT-token does not start a path-component */ DEBUG(10, ("not at start, p=%p, name=%p, p[-1]=%d\n", p, name, (int)p[-1])); - goto no_snapshot; + goto out; } /* @@ -514,19 +539,43 @@ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, if (strncmp(parent_snapdir, snapdir, snapdirlen) == 0) { DEBUG(10, ("name=%s is already converted\n", name)); - goto no_snapshot; + if (psnappath) { + /* + * Need to return up to the next path + * component after the @GMT-. + * This will be used as the connectpath. + */ + q = strchr(p, '/'); + if (q == NULL) { + /* + * No next path component. + * Just return entire string. + */ + *psnappath = talloc_strdup(mem_ctx, + name); + } else { + *psnappath = talloc_strndup(mem_ctx, + name, + q - name); + } + if (*psnappath == NULL) { + ret = false; + goto out; + } + } + goto out; } } q = strptime(p, GMT_FORMAT, &tm); if (q == NULL) { DEBUG(10, ("strptime failed\n")); - goto no_snapshot; + goto out; } tm.tm_isdst = -1; timestamp = timegm(&tm); if (timestamp == (time_t)-1) { DEBUG(10, ("timestamp==-1\n")); - goto no_snapshot; + goto out; } if (q[0] == '\0') { /* @@ -546,12 +595,24 @@ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, stripped = talloc_strndup(mem_ctx, name, len_before_gmt); if (stripped == NULL) { - return false; + ret = false; + goto out; + } + if (orig_name[0] != '/') { + if (make_relative_path(priv->shadow_cwd, + stripped) == false) { + DEBUG(10, (__location__ ": path '%s' " + "doesn't start with cwd '%s\n", + stripped, priv->shadow_cwd)); + ret = false; + errno = ENOENT; + goto out; + } } *pstripped = stripped; } *ptimestamp = timestamp; - return true; + goto out; } if (q[0] != '/') { /* @@ -559,7 +620,7 @@ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, * component continues after the gmt-token. */ DEBUG(10, ("q[0] = %d\n", (int)q[0])); - goto no_snapshot; + goto out; } q += 1; @@ -568,7 +629,7 @@ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, if (priv->config->snapdirseverywhere) { char *insert; - bool have_insert; + char *have_insert; insert = shadow_copy2_insert_string(talloc_tos(), handle, timestamp); if (insert == NULL) { @@ -580,15 +641,28 @@ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, "path '%s'.\n" "insert string '%s'\n", name, insert)); - have_insert = (strstr(name, insert+1) != NULL); - DEBUG(10, ("have_insert=%d, name=%s, insert+1=%s\n", - (int)have_insert, name, insert+1)); - if (have_insert) { + have_insert = strstr(name, insert+1); + DEBUG(10, ("have_insert=%s, name=%s, insert+1=%s\n", + have_insert ? have_insert : "", name, insert+1)); + if (have_insert != NULL) { DEBUG(10, (__location__ ": insert string '%s' found in " "path '%s' found in snapdirseverywhere mode " "==> already converted\n", insert, name)); + if (psnappath) { + /* + * This will be used as the connectpath. + */ + size_t insertlen = strlen(insert); + *psnappath = talloc_strndup(mem_ctx, + name, + have_insert - name + insertlen); + if (*psnappath == NULL) { + ret = false; + goto out; + } + } TALLOC_FREE(insert); - goto no_snapshot; + goto out; } TALLOC_FREE(insert); } else { @@ -599,8 +673,8 @@ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, handle, timestamp); if (snapshot_path == NULL) { - errno = ENOMEM; - return false; + ret = false; + goto out; } DEBUG(10, (__location__ " path: '%s'.\n" @@ -613,12 +687,22 @@ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, * so it is already a converted absolute * path. Don't process further. */ + if (psnappath) { + size_t slen = strlen(snapshot_path); + *psnappath = talloc_strndup(mem_ctx, + name, + slen); + if (*psnappath == NULL) { + ret = false; + goto out; + } + } DEBUG(10, (__location__ ": path '%s' starts with " "snapshot path '%s' (not in " "snapdirseverywhere mode) ==> " "already converted\n", name, snapshot_path)); talloc_free(snapshot_path); - goto no_snapshot; + goto out; } talloc_free(snapshot_path); } @@ -626,8 +710,8 @@ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, if (pstripped != NULL) { stripped = talloc_array(mem_ctx, char, dst_len+1); if (stripped == NULL) { - errno = ENOMEM; - return false; + ret = false; + goto out; } if (p > name) { memcpy(stripped, name, len_before_gmt); @@ -636,13 +720,25 @@ static bool shadow_copy2_strip_snapshot_internal(TALLOC_CTX *mem_ctx, memcpy(stripped + len_before_gmt, q, rest_len); } stripped[dst_len] = '\0'; + if (orig_name[0] != '/') { + if (make_relative_path(priv->shadow_cwd, + stripped) == false) { + DEBUG(10, (__location__ ": path '%s' " + "doesn't start with cwd '%s\n", + stripped, priv->shadow_cwd)); + ret = false; + errno = ENOENT; + goto out; + } + } *pstripped = stripped; } *ptimestamp = timestamp; - return true; -no_snapshot: - *ptimestamp = 0; - return true; + ret = true; + + out: + TALLOC_FREE(abs_path); + return ret; } static bool shadow_copy2_strip_snapshot(TALLOC_CTX *mem_ctx, -- 2.11.0.483.g087da7b7c-goog From e1ad93498e62bed6820f789dd767b45df7594559 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 23 Jan 2017 10:06:44 -0800 Subject: [PATCH 15/16] s3: VFS: shadow_copy2: Fix a memory leak in the connectpath function. BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/modules/vfs_shadow_copy2.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index 89a73d09b0a..c97e444ef4c 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -78,6 +78,8 @@ struct shadow_copy2_private { char *shadow_cwd; /* Absolute $cwd path. */ /* Absolute connectpath - can vary depending on $cwd. */ char *shadow_connectpath; + /* malloc'ed realpath return. */ + char *shadow_realpath; }; static int shadow_copy2_get_shadow_copy_data( @@ -2491,6 +2493,13 @@ static const char *shadow_copy2_connectpath(struct vfs_handle_struct *handle, goto done; } + /* + * SMB_VFS_NEXT_REALPATH returns a malloc'ed string. + * Don't leak memory. + */ + SAFE_FREE(priv->shadow_realpath); + priv->shadow_realpath = result; + DBG_DEBUG("connect path is [%s]\n", result); done: @@ -2569,6 +2578,12 @@ static int shadow_copy2_get_quota(vfs_handle_struct *handle, const char *path, return ret; } +static int shadow_copy2_private_destructor(struct shadow_copy2_private *priv) +{ + SAFE_FREE(priv->shadow_realpath); + return 0; +} + static int shadow_copy2_connect(struct vfs_handle_struct *handle, const char *service, const char *user) { @@ -2600,6 +2615,8 @@ static int shadow_copy2_connect(struct vfs_handle_struct *handle, return -1; } + talloc_set_destructor(priv, shadow_copy2_private_destructor); + priv->snaps = talloc_zero(priv, struct shadow_copy2_snaplist_info); if (priv->snaps == NULL) { DBG_ERR("talloc_zero() failed\n"); -- 2.11.0.483.g087da7b7c-goog From e2c39e50d7bc9f85029f7aab48bfce094158c792 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 23 Jan 2017 10:20:13 -0800 Subject: [PATCH 16/16] s3: VFS: shadow_copy2: Fix usage of saved_errno to only set errno on error. Rationale: VFS calls must act like their POSIX equivalents, and the POSIX versions *only* set errno on a failure. There is actually code in the upper smbd layers that depends on errno being correct on a fail return from a VFS call. For a compound VFS module like this, a common pattern is : SMB_VFS_CALL_X() { int ret; syscall1(); ret = syscall2(); syscall3(); return ret; } Where if *any* of the contained syscallX()'s fail, they'll set errno. However, the actual errno we should return is *only* the one returned if syscall2() fails (the others are lstat's checking for existence etc.). So what we should do to correctly return only the errno from syscall2() is: SMB_VFS_CALL_X() { int ret; int saved_errno = 0; syscall1() ret = syscall2(); if (ret == -1) { saved_errno = errno; } syscall3() if (saved_errno != 0) { errno = saved_errno; } return ret; } BUG: https://bugzilla.samba.org/show_bug.cgi?id=12531 Signed-off-by: Jeremy Allison --- source3/modules/vfs_shadow_copy2.c | 254 ++++++++++++++++++++++++++----------- 1 file changed, 182 insertions(+), 72 deletions(-) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index c97e444ef4c..09edf03e553 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -808,7 +808,8 @@ static char *shadow_copy2_do_convert(TALLOC_CTX *mem_ctx, char *insert = NULL; char *converted = NULL; size_t insertlen, connectlen = 0; - int i, saved_errno; + int saved_errno = 0; + int i; size_t min_offset; struct shadow_copy2_config *config; struct shadow_copy2_private *priv; @@ -994,12 +995,16 @@ static char *shadow_copy2_do_convert(TALLOC_CTX *mem_ctx, errno = ENOENT; } fail: - saved_errno = errno; + if (result == NULL) { + saved_errno = errno; + } TALLOC_FREE(converted); TALLOC_FREE(insert); TALLOC_FREE(slashes); TALLOC_FREE(path); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return result; } @@ -1058,7 +1063,7 @@ static DIR *shadow_copy2_opendir(vfs_handle_struct *handle, time_t timestamp = 0; char *stripped = NULL; DIR *ret; - int saved_errno; + int saved_errno = 0; char *conv; struct smb_filename *conv_smb_fname = NULL; @@ -1087,10 +1092,14 @@ static DIR *shadow_copy2_opendir(vfs_handle_struct *handle, return NULL; } ret = SMB_VFS_NEXT_OPENDIR(handle, conv_smb_fname, mask, attr); - saved_errno = errno; + if (ret == NULL) { + saved_errno = errno; + } TALLOC_FREE(conv); TALLOC_FREE(conv_smb_fname); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -1167,7 +1176,8 @@ static int shadow_copy2_stat(vfs_handle_struct *handle, time_t timestamp = 0; char *stripped = NULL; char *tmp; - int ret, saved_errno; + int saved_errno = 0; + int ret; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, smb_fname->base_name, @@ -1189,7 +1199,9 @@ static int shadow_copy2_stat(vfs_handle_struct *handle, } ret = SMB_VFS_NEXT_STAT(handle, smb_fname); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(smb_fname->base_name); smb_fname->base_name = tmp; @@ -1197,7 +1209,9 @@ static int shadow_copy2_stat(vfs_handle_struct *handle, if (ret == 0) { convert_sbuf(handle, smb_fname->base_name, &smb_fname->st); } - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -1207,7 +1221,8 @@ static int shadow_copy2_lstat(vfs_handle_struct *handle, time_t timestamp = 0; char *stripped = NULL; char *tmp; - int ret, saved_errno; + int saved_errno = 0; + int ret; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, smb_fname->base_name, @@ -1229,7 +1244,9 @@ static int shadow_copy2_lstat(vfs_handle_struct *handle, } ret = SMB_VFS_NEXT_LSTAT(handle, smb_fname); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(smb_fname->base_name); smb_fname->base_name = tmp; @@ -1237,7 +1254,9 @@ static int shadow_copy2_lstat(vfs_handle_struct *handle, if (ret == 0) { convert_sbuf(handle, smb_fname->base_name, &smb_fname->st); } - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -1269,7 +1288,8 @@ static int shadow_copy2_open(vfs_handle_struct *handle, time_t timestamp = 0; char *stripped = NULL; char *tmp; - int ret, saved_errno; + int saved_errno = 0; + int ret; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, smb_fname->base_name, @@ -1291,12 +1311,16 @@ static int shadow_copy2_open(vfs_handle_struct *handle, } ret = SMB_VFS_NEXT_OPEN(handle, smb_fname, fsp, flags, mode); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(smb_fname->base_name); smb_fname->base_name = tmp; - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -1305,7 +1329,8 @@ static int shadow_copy2_unlink(vfs_handle_struct *handle, { time_t timestamp = 0; char *stripped = NULL; - int ret, saved_errno; + int saved_errno = 0; + int ret; struct smb_filename *conv; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, @@ -1328,9 +1353,13 @@ static int shadow_copy2_unlink(vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_UNLINK(handle, conv); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -1340,7 +1369,8 @@ static int shadow_copy2_chmod(vfs_handle_struct *handle, { time_t timestamp = 0; char *stripped = NULL; - int ret, saved_errno; + int saved_errno = 0; + int ret; char *conv = NULL; struct smb_filename *conv_smb_fname; @@ -1372,10 +1402,14 @@ static int shadow_copy2_chmod(vfs_handle_struct *handle, } ret = SMB_VFS_NEXT_CHMOD(handle, conv_smb_fname, mode); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); TALLOC_FREE(conv_smb_fname); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -1386,7 +1420,8 @@ static int shadow_copy2_chown(vfs_handle_struct *handle, { time_t timestamp = 0; char *stripped = NULL; - int ret, saved_errno; + int saved_errno = 0; + int ret; char *conv = NULL; struct smb_filename *conv_smb_fname = NULL; @@ -1416,10 +1451,14 @@ static int shadow_copy2_chown(vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_CHOWN(handle, conv_smb_fname, uid, gid); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); TALLOC_FREE(conv_smb_fname); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -1511,7 +1550,8 @@ static int shadow_copy2_ntimes(vfs_handle_struct *handle, { time_t timestamp = 0; char *stripped = NULL; - int ret, saved_errno; + int saved_errno = 0; + int ret; struct smb_filename *conv; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, @@ -1534,9 +1574,13 @@ static int shadow_copy2_ntimes(vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_NTIMES(handle, conv, ft); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -1545,7 +1589,8 @@ static int shadow_copy2_readlink(vfs_handle_struct *handle, { time_t timestamp = 0; char *stripped = NULL; - int ret, saved_errno; + int saved_errno = 0; + int ret; char *conv; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname, @@ -1561,9 +1606,13 @@ static int shadow_copy2_readlink(vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_READLINK(handle, conv, buf, bufsiz); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -1572,7 +1621,8 @@ static int shadow_copy2_mknod(vfs_handle_struct *handle, { time_t timestamp = 0; char *stripped = NULL; - int ret, saved_errno; + int saved_errno = 0; + int ret; char *conv; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname, @@ -1588,9 +1638,13 @@ static int shadow_copy2_mknod(vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_MKNOD(handle, conv, mode, dev); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -1601,7 +1655,7 @@ static char *shadow_copy2_realpath(vfs_handle_struct *handle, char *stripped = NULL; char *tmp = NULL; char *result = NULL; - int saved_errno; + int saved_errno = 0; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname, ×tamp, &stripped)) { @@ -1619,10 +1673,14 @@ static char *shadow_copy2_realpath(vfs_handle_struct *handle, result = SMB_VFS_NEXT_REALPATH(handle, tmp); done: - saved_errno = errno; + if (result == NULL) { + saved_errno = errno; + } TALLOC_FREE(tmp); TALLOC_FREE(stripped); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return result; } @@ -2113,7 +2171,8 @@ static int shadow_copy2_mkdir(vfs_handle_struct *handle, { time_t timestamp = 0; char *stripped = NULL; - int ret, saved_errno; + int saved_errno = 0; + int ret; char *conv; struct smb_filename *conv_smb_fname = NULL; @@ -2142,10 +2201,14 @@ static int shadow_copy2_mkdir(vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_MKDIR(handle, conv_smb_fname, mode); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); TALLOC_FREE(conv_smb_fname); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -2154,7 +2217,8 @@ static int shadow_copy2_rmdir(vfs_handle_struct *handle, { time_t timestamp = 0; char *stripped = NULL; - int ret, saved_errno; + int saved_errno = 0; + int ret; char *conv; struct smb_filename *conv_smb_fname = NULL; @@ -2183,10 +2247,14 @@ static int shadow_copy2_rmdir(vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_RMDIR(handle, conv_smb_fname); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv_smb_fname); TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -2195,7 +2263,8 @@ static int shadow_copy2_chflags(vfs_handle_struct *handle, const char *fname, { time_t timestamp = 0; char *stripped = NULL; - int ret, saved_errno; + int saved_errno = 0; + int ret; char *conv; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname, @@ -2211,9 +2280,13 @@ static int shadow_copy2_chflags(vfs_handle_struct *handle, const char *fname, return -1; } ret = SMB_VFS_NEXT_CHFLAGS(handle, conv, flags); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -2224,7 +2297,7 @@ static ssize_t shadow_copy2_getxattr(vfs_handle_struct *handle, time_t timestamp = 0; char *stripped = NULL; ssize_t ret; - int saved_errno; + int saved_errno = 0; char *conv; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname, @@ -2241,9 +2314,13 @@ static ssize_t shadow_copy2_getxattr(vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_GETXATTR(handle, conv, aname, value, size); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -2254,7 +2331,7 @@ static ssize_t shadow_copy2_listxattr(struct vfs_handle_struct *handle, time_t timestamp = 0; char *stripped = NULL; ssize_t ret; - int saved_errno; + int saved_errno = 0; char *conv; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname, @@ -2270,9 +2347,13 @@ static ssize_t shadow_copy2_listxattr(struct vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_LISTXATTR(handle, conv, list, size); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -2281,7 +2362,8 @@ static int shadow_copy2_removexattr(vfs_handle_struct *handle, { time_t timestamp = 0; char *stripped = NULL; - int ret, saved_errno; + int saved_errno = 0; + int ret; char *conv; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname, @@ -2297,9 +2379,13 @@ static int shadow_copy2_removexattr(vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_REMOVEXATTR(handle, conv, aname); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -2311,7 +2397,7 @@ static int shadow_copy2_setxattr(struct vfs_handle_struct *handle, time_t timestamp = 0; char *stripped = NULL; ssize_t ret; - int saved_errno; + int saved_errno = 0; char *conv; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname, @@ -2328,9 +2414,13 @@ static int shadow_copy2_setxattr(struct vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_SETXATTR(handle, conv, aname, value, size, flags); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -2341,7 +2431,7 @@ static int shadow_copy2_chmod_acl(vfs_handle_struct *handle, time_t timestamp = 0; char *stripped = NULL; ssize_t ret; - int saved_errno; + int saved_errno = 0; char *conv = NULL; struct smb_filename *conv_smb_fname = NULL; @@ -2371,10 +2461,14 @@ static int shadow_copy2_chmod_acl(vfs_handle_struct *handle, return -1; } ret = SMB_VFS_NEXT_CHMOD_ACL(handle, conv_smb_fname, mode); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); TALLOC_FREE(conv_smb_fname); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -2387,7 +2481,7 @@ static int shadow_copy2_get_real_filename(struct vfs_handle_struct *handle, time_t timestamp = 0; char *stripped = NULL; ssize_t ret; - int saved_errno; + int saved_errno = 0; char *conv; DEBUG(10, ("shadow_copy2_get_real_filename called for path=[%s], " @@ -2414,9 +2508,13 @@ static int shadow_copy2_get_real_filename(struct vfs_handle_struct *handle, ret = SMB_VFS_NEXT_GET_REAL_FILENAME(handle, conv, name, mem_ctx, found_name); DEBUG(10, ("NEXT_REAL_FILE_NAME returned %d\n", (int)ret)); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -2428,7 +2526,7 @@ static const char *shadow_copy2_connectpath(struct vfs_handle_struct *handle, char *tmp = NULL; char *result = NULL; char *parent_dir = NULL; - int saved_errno; + int saved_errno = 0; size_t rootpath_len = 0; struct shadow_copy2_private *priv = NULL; @@ -2503,11 +2601,15 @@ static const char *shadow_copy2_connectpath(struct vfs_handle_struct *handle, DBG_DEBUG("connect path is [%s]\n", result); done: - saved_errno = errno; + if (result == NULL) { + saved_errno = errno; + } TALLOC_FREE(tmp); TALLOC_FREE(stripped); TALLOC_FREE(parent_dir); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return result; } @@ -2518,7 +2620,7 @@ static uint64_t shadow_copy2_disk_free(vfs_handle_struct *handle, time_t timestamp = 0; char *stripped = NULL; ssize_t ret; - int saved_errno; + int saved_errno = 0; char *conv; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, path, @@ -2538,9 +2640,13 @@ static uint64_t shadow_copy2_disk_free(vfs_handle_struct *handle, ret = SMB_VFS_NEXT_DISK_FREE(handle, conv, bsize, dfree, dsize); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } @@ -2552,7 +2658,7 @@ static int shadow_copy2_get_quota(vfs_handle_struct *handle, const char *path, time_t timestamp = 0; char *stripped = NULL; int ret; - int saved_errno; + int saved_errno = 0; char *conv; if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, path, ×tamp, @@ -2571,9 +2677,13 @@ static int shadow_copy2_get_quota(vfs_handle_struct *handle, const char *path, ret = SMB_VFS_NEXT_GET_QUOTA(handle, conv, qtype, id, dq); - saved_errno = errno; + if (ret == -1) { + saved_errno = errno; + } TALLOC_FREE(conv); - errno = saved_errno; + if (saved_errno != 0) { + errno = saved_errno; + } return ret; } -- 2.11.0.483.g087da7b7c-goog