vfs_shadow_copy2: ensure we call convert_sbuf() in shadow_copy2_*stat() on already...

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 <slow@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

diff --git a/selftest/knownfail.d/samba3.blackbox.shadow_copy_torture b/selftest/knownfail.d/samba3.blackbox.shadow_copy_torture
deleted file mode 100644 (file)
index aa68c3f..0000000
--- 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 31709a31a1a380f2a149664787142703f298e121..22950e4b6bfb3ef4940d4fd1595de723e0b2000c 100644 (file)
--- 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,
-                                        &timestamp, &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,
+                                                  &timestamp,
+                                                  &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,
-                                        &timestamp, &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,
+                                                  &timestamp,
+                                                  &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,
-                                        &timestamp, &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,
+                                                  &timestamp,
+                                                  &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;
 }