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 <jra@samba.org>
Reviewed-by: Uri Simchoni <uri@samba.org>

diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
index 137dad8bf4fb24556409a44f0f4312ae0fafdb2d..f2a80f4b456746525dc3e74b32e272c880f38f4d 100644 (file)
--- 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;
 }
 
@@ -1170,7 +1179,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,
@@ -1192,7 +1202,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;
@@ -1200,7 +1212,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;
 }
 
@@ -1210,7 +1224,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,
@@ -1232,7 +1247,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;
@@ -1240,7 +1257,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;
 }
 
@@ -1272,7 +1291,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,
@@ -1294,12 +1314,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;
 }
 
@@ -1308,7 +1332,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,
@@ -1331,9 +1356,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;
 }
 
@@ -1343,7 +1372,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;
 
@@ -1375,10 +1405,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;
 }
 
@@ -1389,7 +1423,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;
 
@@ -1419,10 +1454,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;
 }
 
@@ -1514,7 +1553,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,
@@ -1537,9 +1577,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;
 }
 
@@ -1548,7 +1592,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,
@@ -1564,9 +1609,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;
 }
 
@@ -1575,7 +1624,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,
@@ -1591,9 +1641,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;
 }
 
@@ -1604,7 +1658,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,
                                         &timestamp, &stripped)) {
@@ -1622,10 +1676,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;
 }
 
@@ -2116,7 +2174,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;
 
@@ -2145,10 +2204,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;
 }
 
@@ -2157,7 +2220,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;
 
@@ -2186,10 +2250,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;
 }
 
@@ -2198,7 +2266,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,
@@ -2214,9 +2283,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;
 }
 
@@ -2227,7 +2300,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,
@@ -2244,9 +2317,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;
 }
 
@@ -2257,7 +2334,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,
@@ -2273,9 +2350,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;
 }
 
@@ -2284,7 +2365,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,
@@ -2300,9 +2382,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;
 }
 
@@ -2314,7 +2400,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,
@@ -2331,9 +2417,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;
 }
 
@@ -2344,7 +2434,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;
 
@@ -2374,10 +2464,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;
 }
 
@@ -2390,7 +2484,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], "
@@ -2417,9 +2511,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;
 }
 
@@ -2431,7 +2529,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;
 
@@ -2506,11 +2604,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;
 }
 
@@ -2521,7 +2623,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,
@@ -2541,9 +2643,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;
 }
@@ -2555,7 +2661,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, &timestamp,
@@ -2574,9 +2680,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;
 }