Subject: [PATCH 20/35] s3: VFS: shadow_copy2: Fix usage of saved_errno to only set errno on error.

From: Arvid Requate <[email-masked]>
Date: Fri, 17 Mar 2017 18:43:16 +0100

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: Arvid Requate <[email-masked]>
---
 source3/modules/vfs_shadow_copy2.c | 244 ++++++++++++++++++++++++++-----------
 1 file changed, 175 insertions(+), 69 deletions(-)

diff --git a/source3/modules/vfs_shadow_copy2.c b/source3/modules/vfs_shadow_copy2.c
index d7cc449..d7e32daa 100644
--- a/source3/modules/vfs_shadow_copy2.c
+++ b/source3/modules/vfs_shadow_copy2.c
@@ -619,7 +619,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;
 	size_t in_share_offset = 0;
@@ -802,12 +803,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;
 }
 
@@ -866,7 +871,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;
 
 	if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname,
@@ -882,9 +887,13 @@ static DIR *shadow_copy2_opendir(vfs_handle_struct *handle,
 		return NULL;
 	}
 	ret = SMB_VFS_NEXT_OPENDIR(handle, conv, mask, attr);
-	saved_errno = errno;
+	if (ret == NULL) {
+		saved_errno = errno;
+	}
 	TALLOC_FREE(conv);
-	errno = saved_errno;
+	if (saved_errno != 0) {
+		errno = saved_errno;
+	}
 	return ret;
 }
 
@@ -964,7 +973,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,
@@ -986,7 +996,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;
@@ -994,7 +1006,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;
 }
 
@@ -1004,7 +1018,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,
@@ -1026,7 +1041,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;
@@ -1034,7 +1051,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;
 }
 
@@ -1066,7 +1085,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,
@@ -1088,12 +1108,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;
 }
 
@@ -1102,7 +1126,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,
@@ -1125,9 +1150,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;
 }
 
@@ -1136,7 +1165,8 @@ static int shadow_copy2_chmod(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,
@@ -1152,9 +1182,13 @@ static int shadow_copy2_chmod(vfs_handle_struct *handle, const char *fname,
 		return -1;
 	}
 	ret = SMB_VFS_NEXT_CHMOD(handle, conv, mode);
-	saved_errno = errno;
+	if (ret == -1) {
+		saved_errno = errno;
+	}
 	TALLOC_FREE(conv);
-	errno = saved_errno;
+	if (saved_errno != 0) {
+		errno = saved_errno;
+	}
 	return ret;
 }
 
@@ -1163,7 +1197,8 @@ static int shadow_copy2_chown(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,
@@ -1179,9 +1214,13 @@ static int shadow_copy2_chown(vfs_handle_struct *handle, const char *fname,
 		return -1;
 	}
 	ret = SMB_VFS_NEXT_CHOWN(handle, conv, uid, gid);
-	saved_errno = errno;
+	if (ret == -1) {
+		saved_errno = errno;
+	}
 	TALLOC_FREE(conv);
-	errno = saved_errno;
+	if (saved_errno != 0) {
+		errno = saved_errno;
+	}
 	return ret;
 }
 
@@ -1273,7 +1312,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,
@@ -1296,9 +1336,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;
 }
 
@@ -1307,7 +1351,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,
@@ -1323,9 +1368,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;
 }
 
@@ -1334,7 +1383,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,
@@ -1350,9 +1400,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;
 }
 
@@ -1363,7 +1417,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)) {
@@ -1381,10 +1435,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;
 }
 
@@ -1753,7 +1811,8 @@ static int shadow_copy2_mkdir(vfs_handle_struct *handle,
 {
 	time_t timestamp = 0;
 	char *stripped = NULL;
-	int ret, saved_errno;
+	int ret;
+	int saved_errno = 0;
 	char *conv;
 
 	if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname,
@@ -1769,9 +1828,13 @@ static int shadow_copy2_mkdir(vfs_handle_struct *handle,
 		return -1;
 	}
 	ret = SMB_VFS_NEXT_MKDIR(handle, conv, mode);
-	saved_errno = errno;
+	if (ret == -1) {
+		saved_errno = errno;
+	}
 	TALLOC_FREE(conv);
-	errno = saved_errno;
+	if (saved_errno != 0) {
+		errno = saved_errno;
+	}
 	return ret;
 }
 
@@ -1779,7 +1842,8 @@ static int shadow_copy2_rmdir(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,
@@ -1795,9 +1859,13 @@ static int shadow_copy2_rmdir(vfs_handle_struct *handle, const char *fname)
 		return -1;
 	}
 	ret = SMB_VFS_NEXT_RMDIR(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;
 }
 
@@ -1806,7 +1874,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,
@@ -1822,9 +1891,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;
 }
 
@@ -1835,7 +1908,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,
@@ -1852,9 +1925,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;
 }
 
@@ -1865,7 +1942,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,
@@ -1881,9 +1958,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;
 }
 
@@ -1892,7 +1973,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,
@@ -1908,9 +1990,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;
 }
 
@@ -1922,7 +2008,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,
@@ -1939,9 +2025,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;
 }
 
@@ -1951,7 +2041,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;
 
 	if (!shadow_copy2_strip_snapshot(talloc_tos(), handle, fname,
@@ -1967,9 +2057,13 @@ static int shadow_copy2_chmod_acl(vfs_handle_struct *handle,
 		return -1;
 	}
 	ret = SMB_VFS_NEXT_CHMOD_ACL(handle, conv, mode);
-	saved_errno = errno;
+	if (ret == -1) {
+		saved_errno = errno;
+	}
 	TALLOC_FREE(conv);
-	errno = saved_errno;
+	if (saved_errno != 0) {
+		errno = saved_errno;
+	}
 	return ret;
 }
 
@@ -1982,7 +2076,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], "
@@ -2008,10 +2102,14 @@ static int shadow_copy2_get_real_filename(struct vfs_handle_struct *handle,
 		   "name=[%s]\n", conv, name));
 	ret = SMB_VFS_NEXT_GET_REAL_FILENAME(handle, conv, name,
 					     mem_ctx, found_name);
+	if (ret == -1) {
+		saved_errno = errno;
+	}
 	DEBUG(10, ("NEXT_REAL_FILE_NAME returned %d\n", (int)ret));
-	saved_errno = errno;
 	TALLOC_FREE(conv);
-	errno = saved_errno;
+	if (saved_errno != 0) {
+		errno = saved_errno;
+	}
 	return ret;
 }
 
@@ -2023,7 +2121,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_config *config = NULL;
 
@@ -2098,11 +2196,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;
 }
 
@@ -2113,7 +2215,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,
@@ -2133,9 +2235,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;
 }
-- 
2.1.4