From: Ralph Boehme Date: Sat, 3 Jul 2021 13:46:11 +0000 (+0200) Subject: vfs_shadow_copy2: ensure we call convert_sbuf() in shadow_copy2_*stat() on already... X-Git-Tag: talloc-2.3.3~72 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c7d6745858f2efdd24ed6fd353ec5ece898033fa;p=thirdparty%2Fsamba.git vfs_shadow_copy2: ensure we call convert_sbuf() in shadow_copy2_*stat() on already converted paths with absolute path shadow_copy2_strip_snapshot() will happily return without modifying the passed timestamp=0 if the path is already converted and refers to an object in a snapshot, eg (first debug line from extra debugging patch [1]): [10 2021/07/02 08:19:28.811424 pid=738290 ../../source3/modules/vfs_shadow_copy2.c:1303 shadow_copy2_fstat] shadow_copy2_fstat: fsp [test.txt {@GMT-2000.01.02-03.04.05}] [10 2021/07/02 08:19:28.811449 pid=738290 ../../source3/modules/vfs_shadow_copy2.c:607 _shadow_copy2_strip_snapshot_internal] _shadow_copy2_strip_snapshot_internal: [from shadow_copy2_fstat()] Path 'test.txt {@GMT-2000.01.02-03.04.05}' [10 2021/07/02 08:19:28.811474 pid=738290 ../../source3/modules/vfs_shadow_copy2.c:619 _shadow_copy2_strip_snapshot_internal] _shadow_copy2_strip_snapshot_internal: abs path '/gpfs0/smb_snapshots2/filesetone/.snapshots/@GMT-2000.01.02-03.04.05/test.txt' [10 2021/07/02 08:19:28.811496 pid=738290 ../../source3/modules/vfs_shadow_copy2.c:1924 shadow_copy2_snapshot_to_gmt] shadow_copy2_snapshot_to_gmt: match @GMT-%Y.%m.%d-%H.%M.%S: @GMT-2000.01.02-03.04.05 [10 2021/07/02 08:19:28.811536 pid=738290 ../../source3/modules/vfs_shadow_copy2.c:566 check_for_converted_path] check_for_converted_path: path |/gpfs0/smb_snapshots2/filesetone/.snapshots/@GMT-2000.01.02-03.04.05/test.txt| is already converted. connect path = |/gpfs0/smb_snapshots2/filesetone/.snapshots/@GMT-2000.01.02-03.04.05| As check_for_converted_path() detects an "already converted path", _shadow_copy2_strip_snapshot_internal() just returns without modifying the value of the timestamp. By using shadow_copy2_strip_snapshot_converted() instead of shadow_copy2_strip_snapshot() we can check if the path is in fact referring to a VSS object by checking the "converted" bool. An alternative way would have been directly checking fsp->fsp_name->twrp != 0, but that would be a new semantic in the module, I'll leave this excersize for the future when we clean up the usage of shadow_copy2_strip_snapshot() in the whole module. This change also switches to using the absolute paths in both place where convert_sbuf() is called. [1] @@ -1309,8 +1348,16 @@ static int shadow_copy2_fstat(vfs_handle_struct *handle, files_struct *fsp, saved_errno = errno; } + DBG_DEBUG("fsp [%s]\n", fsp_str_dbg(fsp)); RN: vfs_shadow_copy2 fixinodes not correctly updating inode numbers BUG: https://bugzilla.samba.org/show_bug.cgi?id=14756 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison --- diff --git a/selftest/knownfail.d/samba3.blackbox.shadow_copy_torture b/selftest/knownfail.d/samba3.blackbox.shadow_copy_torture deleted file mode 100644 index aa68c3f3bbe..00000000000 --- a/selftest/knownfail.d/samba3.blackbox.shadow_copy_torture +++ /dev/null @@ -1 +0,0 @@ -^samba3.blackbox.shadow_copy_torture.fix inodes with hardlink\(fileserver\) diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c index 31709a31a1a..22950e4b6bf 100644 --- a/source3/modules/vfs_shadow_copy2.c +++ b/source3/modules/vfs_shadow_copy2.c @@ -1168,19 +1168,45 @@ static int shadow_copy2_linkat(vfs_handle_struct *handle, static int shadow_copy2_stat(vfs_handle_struct *handle, struct smb_filename *smb_fname) { + struct shadow_copy2_private *priv = NULL; time_t timestamp = 0; char *stripped = NULL; + bool converted = false; + char *abspath = NULL; char *tmp; - int saved_errno = 0; - int ret; + int ret = 0; - if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, - smb_fname, - ×tamp, &stripped)) { + SMB_VFS_HANDLE_GET_DATA(handle, priv, struct shadow_copy2_private, + return -1); + + if (!shadow_copy2_strip_snapshot_converted(talloc_tos(), + handle, + smb_fname, + ×tamp, + &stripped, + &converted)) { return -1; } if (timestamp == 0) { - return SMB_VFS_NEXT_STAT(handle, smb_fname); + TALLOC_FREE(stripped); + ret = SMB_VFS_NEXT_STAT(handle, smb_fname); + if (ret != 0) { + return ret; + } + if (!converted) { + return 0; + } + + abspath = make_path_absolute(talloc_tos(), + priv, + smb_fname->base_name); + if (abspath == NULL) { + return -1; + } + + convert_sbuf(handle, abspath, &smb_fname->st); + TALLOC_FREE(abspath); + return 0; } tmp = smb_fname->base_name; @@ -1194,38 +1220,70 @@ static int shadow_copy2_stat(vfs_handle_struct *handle, } ret = SMB_VFS_NEXT_STAT(handle, smb_fname); - if (ret == -1) { - saved_errno = errno; + if (ret != 0) { + goto out; } + abspath = make_path_absolute(talloc_tos(), + priv, + smb_fname->base_name); + if (abspath == NULL) { + ret = -1; + goto out; + } + + convert_sbuf(handle, abspath, &smb_fname->st); + TALLOC_FREE(abspath); + +out: TALLOC_FREE(smb_fname->base_name); smb_fname->base_name = tmp; - if (ret == 0) { - convert_sbuf(handle, smb_fname->base_name, &smb_fname->st); - } - if (saved_errno != 0) { - errno = saved_errno; - } return ret; } static int shadow_copy2_lstat(vfs_handle_struct *handle, struct smb_filename *smb_fname) { + struct shadow_copy2_private *priv = NULL; time_t timestamp = 0; char *stripped = NULL; + bool converted = false; + char *abspath = NULL; char *tmp; - int saved_errno = 0; - int ret; + int ret = 0; - if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, - smb_fname, - ×tamp, &stripped)) { + SMB_VFS_HANDLE_GET_DATA(handle, priv, struct shadow_copy2_private, + return -1); + + if (!shadow_copy2_strip_snapshot_converted(talloc_tos(), + handle, + smb_fname, + ×tamp, + &stripped, + &converted)) { return -1; } if (timestamp == 0) { - return SMB_VFS_NEXT_LSTAT(handle, smb_fname); + TALLOC_FREE(stripped); + ret = SMB_VFS_NEXT_LSTAT(handle, smb_fname); + if (ret != 0) { + return ret; + } + if (!converted) { + return 0; + } + + abspath = make_path_absolute(talloc_tos(), + priv, + smb_fname->base_name); + if (abspath == NULL) { + return -1; + } + + convert_sbuf(handle, abspath, &smb_fname->st); + TALLOC_FREE(abspath); + return 0; } tmp = smb_fname->base_name; @@ -1239,45 +1297,76 @@ static int shadow_copy2_lstat(vfs_handle_struct *handle, } ret = SMB_VFS_NEXT_LSTAT(handle, smb_fname); - if (ret == -1) { - saved_errno = errno; + if (ret != 0) { + goto out; + } + + abspath = make_path_absolute(talloc_tos(), + priv, + smb_fname->base_name); + if (abspath == NULL) { + ret = -1; + goto out; } + convert_sbuf(handle, abspath, &smb_fname->st); + TALLOC_FREE(abspath); + +out: TALLOC_FREE(smb_fname->base_name); smb_fname->base_name = tmp; - if (ret == 0) { - convert_sbuf(handle, smb_fname->base_name, &smb_fname->st); - } - if (saved_errno != 0) { - errno = saved_errno; - } return ret; } static int shadow_copy2_fstat(vfs_handle_struct *handle, files_struct *fsp, SMB_STRUCT_STAT *sbuf) { + struct shadow_copy2_private *priv = NULL; 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; + char *abspath = NULL; + bool converted = false; bool ok; int ret; - ok = shadow_copy2_strip_snapshot(talloc_tos(), handle, - fsp->fsp_name, - ×tamp, &stripped); + SMB_VFS_HANDLE_GET_DATA(handle, priv, struct shadow_copy2_private, + return -1); + + ok = shadow_copy2_strip_snapshot_converted(talloc_tos(), + handle, + fsp->fsp_name, + ×tamp, + &stripped, + &converted); if (!ok) { return -1; } if (timestamp == 0) { TALLOC_FREE(stripped); - return SMB_VFS_NEXT_FSTAT(handle, fsp, sbuf); + ret = SMB_VFS_NEXT_FSTAT(handle, fsp, sbuf); + if (ret != 0) { + return ret; + } + if (!converted) { + return 0; + } + + abspath = make_path_absolute(talloc_tos(), + priv, + fsp->fsp_name->base_name); + if (abspath == NULL) { + return -1; + } + + convert_sbuf(handle, abspath, sbuf); + TALLOC_FREE(abspath); + return 0; } vss_smb_fname = *fsp->fsp_name; @@ -1301,20 +1390,27 @@ static int shadow_copy2_fstat(vfs_handle_struct *handle, files_struct *fsp, } ret = SMB_VFS_NEXT_FSTAT(handle, fsp, sbuf); + if (ret != 0) { + goto out; + } + + abspath = make_path_absolute(talloc_tos(), + priv, + fsp->fsp_name->base_name); + if (abspath == NULL) { + ret = -1; + goto out; + } + + convert_sbuf(handle, abspath, sbuf); + TALLOC_FREE(abspath); + +out: fsp->fsp_name = orig_smb_fname; if (fsp->base_fsp != NULL) { fsp->base_fsp->fsp_name = orig_base_smb_fname; } - if (ret == -1) { - saved_errno = errno; - } - if (ret == 0) { - convert_sbuf(handle, fsp->fsp_name->base_name, sbuf); - } - if (saved_errno != 0) { - errno = saved_errno; - } return ret; }