[PATCH] Fix bug 10982 - fallocate() returned values on failure

Jeremy Allison jra at samba.org
Fri Dec 5 16:43:15 MST 2014


Here is the patchset for master confirmed as
working by Jones <jones.kstw at gmail.com> for
bug #10982 - fallocate() returned values on failure.

Please review and push !

Thanks,

	Jeremy.
-------------- next part --------------
From 563efec99e11b9021d35d6ce2190161f5d13071a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 5 Dec 2014 15:31:19 -0800
Subject: [PATCH 1/3] s3: smbd: Fix *allocate* calls to follow POSIX error
 return convention.

vfswrap_fallocate() is broken in that it can call posix_fallocate()
which returns an int error (and doesn't set errno) but can also
call Linux fallocate() which returns -1 and sets errno.

Standardize on the -1,errno convention.

Reported by Jones <jones.kstw at gmail.com> who provided the
initial patch. This patch tested and confirmed working
by him as well.

https://bugzilla.samba.org/show_bug.cgi?id=10982

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_default.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
index 613101a..1c855c9 100644
--- a/source3/modules/vfs_default.c
+++ b/source3/modules/vfs_default.c
@@ -1863,15 +1863,14 @@ static int strict_allocate_ftruncate(vfs_handle_struct *handle, files_struct *fs
 	   return ENOTSUP or EINVAL in cases like that. */
 	ret = SMB_VFS_FALLOCATE(fsp, VFS_FALLOCATE_EXTEND_SIZE,
 				pst->st_ex_size, space_to_write);
-	if (ret == ENOSPC) {
-		errno = ENOSPC;
+	if (ret == -1 && errno == ENOSPC) {
 		return -1;
 	}
 	if (ret == 0) {
 		return 0;
 	}
 	DEBUG(10,("strict_allocate_ftruncate: SMB_VFS_FALLOCATE failed with "
-		"error %d. Falling back to slow manual allocation\n", ret));
+		"error %d. Falling back to slow manual allocation\n", errno));
 
 	/* available disk space is enough or not? */
 	space_avail = get_dfree_info(fsp->conn,
@@ -1887,8 +1886,7 @@ static int strict_allocate_ftruncate(vfs_handle_struct *handle, files_struct *fs
 	/* Write out the real space on disk. */
 	ret = vfs_slow_fallocate(fsp, pst->st_ex_size, space_to_write);
 	if (ret != 0) {
-		errno = ret;
-		ret = -1;
+		return -1;
 	}
 
 	return 0;
@@ -1973,6 +1971,15 @@ static int vfswrap_fallocate(vfs_handle_struct *handle,
 	START_PROFILE(syscall_fallocate);
 	if (mode == VFS_FALLOCATE_EXTEND_SIZE) {
 		result = sys_posix_fallocate(fsp->fh->fd, offset, len);
+		/*
+		 * posix_fallocate returns 0 on success, errno on error
+		 * and doesn't set errno. Make it behave like fallocate()
+		 * which returns -1, and sets errno on failure.
+		 */
+		if (result != 0) {
+			errno = result;
+			result = -1;
+		}
 	} else if (mode == VFS_FALLOCATE_KEEP_SIZE) {
 		result = sys_fallocate(fsp->fh->fd, mode, offset, len);
 	} else {
-- 
2.2.0.rc0.207.ga3a616c


From f008450b85b6e5ac37b4e4337d167833e66e32c9 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 5 Dec 2014 15:34:12 -0800
Subject: [PATCH 2/3] s3: smbd: Fix *allocate* calls to follow POSIX error
 return convention.

Fix vfs_allocate_file_space(), vfs_slow_fallocate(),
vfs_fill_sparse() to follow the -1,errno convention
for errors.

Standardize on the -1,errno convention.

Reported by Jones <jones.kstw at gmail.com> who provided the
initial patch. This patch tested and confirmed working
by him as well.

https://bugzilla.samba.org/show_bug.cgi?id=10982

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/smbd/vfs.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
index 51362c3..d10e0d6 100644
--- a/source3/smbd/vfs.c
+++ b/source3/smbd/vfs.c
@@ -586,6 +586,10 @@ int vfs_allocate_file_space(files_struct *fsp, uint64_t len)
 		return 0;
 	}
 
+	if (ret == -1 && errno == ENOSPC) {
+		return -1;
+	}
+
 	len -= fsp->fsp_name->st.st_ex_size;
 	len /= 1024; /* Len is now number of 1k blocks needed. */
 	space_avail = get_dfree_info(conn, fsp->fsp_name->base_name, false,
@@ -640,7 +644,7 @@ int vfs_set_filelen(files_struct *fsp, off_t len)
  fails. Needs to be outside of the default version of SMB_VFS_FALLOCATE
  as this is also called from the default SMB_VFS_FTRUNCATE code.
  Always extends the file size.
- Returns 0 on success, errno on failure.
+ Returns 0 on success, -1 on failure.
 ****************************************************************************/
 
 #define SPARSE_BUF_WRITE_SIZE (32*1024)
@@ -654,7 +658,7 @@ int vfs_slow_fallocate(files_struct *fsp, off_t offset, off_t len)
 		sparse_buf = SMB_CALLOC_ARRAY(char, SPARSE_BUF_WRITE_SIZE);
 		if (!sparse_buf) {
 			errno = ENOMEM;
-			return ENOMEM;
+			return -1;
 		}
 	}
 
@@ -663,10 +667,12 @@ int vfs_slow_fallocate(files_struct *fsp, off_t offset, off_t len)
 
 		pwrite_ret = SMB_VFS_PWRITE(fsp, sparse_buf, curr_write_size, offset + total);
 		if (pwrite_ret == -1) {
+			int saved_errno = errno;
 			DEBUG(10,("vfs_slow_fallocate: SMB_VFS_PWRITE for file "
 				  "%s failed with error %s\n",
-				  fsp_str_dbg(fsp), strerror(errno)));
-			return errno;
+				  fsp_str_dbg(fsp), strerror(saved_errno)));
+			errno = saved_errno;
+			return -1;
 		}
 		total += pwrite_ret;
 	}
@@ -724,9 +730,7 @@ int vfs_fill_sparse(files_struct *fsp, off_t len)
 		 * return ENOTSUP or EINVAL in cases like that. */
 		ret = SMB_VFS_FALLOCATE(fsp, VFS_FALLOCATE_EXTEND_SIZE,
 				offset, num_to_write);
-		if (ret == ENOSPC) {
-			errno = ENOSPC;
-			ret = -1;
+		if (ret == -1 && errno == ENOSPC) {
 			goto out;
 		}
 		if (ret == 0) {
@@ -737,10 +741,6 @@ int vfs_fill_sparse(files_struct *fsp, off_t len)
 	}
 
 	ret = vfs_slow_fallocate(fsp, offset, num_to_write);
-	if (ret != 0) {
-		errno = ret;
-		ret = -1;
-	}
 
  out:
 
-- 
2.2.0.rc0.207.ga3a616c


From 18d31ba922a76b492ac067b00d0be543ad8a1f42 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Fri, 5 Dec 2014 15:37:11 -0800
Subject: [PATCH 3/3] s3: modules: Fix *allocate* calls to follow POSIX error
 return convention.

Fix up the ceph, fruit and streams_xattr modules to follow
the -1,errno convention for errors.

Reported by Jones <jones.kstw at gmail.com> who provided the
initial patch. This patch tested and confirmed working
by him as well.

Signed-off-by: Jeremy Allison <jra at samba.org>
---
 source3/modules/vfs_ceph.c          | 13 +++----------
 source3/modules/vfs_fruit.c         |  5 +++--
 source3/modules/vfs_streams_xattr.c |  5 +++--
 3 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/source3/modules/vfs_ceph.c b/source3/modules/vfs_ceph.c
index e402ff1..b0a0024 100644
--- a/source3/modules/vfs_ceph.c
+++ b/source3/modules/vfs_ceph.c
@@ -795,15 +795,14 @@ static int strict_allocate_ftruncate(struct vfs_handle_struct *handle, files_str
 	   return ENOTSUP or EINVAL in cases like that. */
 	ret = SMB_VFS_FALLOCATE(fsp, VFS_FALLOCATE_EXTEND_SIZE,
 				pst->st_ex_size, space_to_write);
-	if (ret == ENOSPC) {
-		errno = ENOSPC;
+	if (ret == -1 && errno == ENOSPC) {
 		return -1;
 	}
 	if (ret == 0) {
 		return 0;
 	}
 	DEBUG(10,("[CEPH] strict_allocate_ftruncate: SMB_VFS_FALLOCATE failed with "
-		"error %d. Falling back to slow manual allocation\n", ret));
+		"error %d. Falling back to slow manual allocation\n", errno));
 
 	/* available disk space is enough or not? */
 	space_avail = get_dfree_info(fsp->conn,
@@ -817,13 +816,7 @@ static int strict_allocate_ftruncate(struct vfs_handle_struct *handle, files_str
 	}
 
 	/* Write out the real space on disk. */
-	ret = vfs_slow_fallocate(fsp, pst->st_ex_size, space_to_write);
-	if (ret != 0) {
-		errno = ret;
-		ret = -1;
-	}
-
-	return 0;
+	return vfs_slow_fallocate(fsp, pst->st_ex_size, space_to_write);
 }
 
 static int cephwrap_ftruncate(struct vfs_handle_struct *handle, files_struct *fsp, off_t len)
diff --git a/source3/modules/vfs_fruit.c b/source3/modules/vfs_fruit.c
index ebafe3a..d1b023d 100644
--- a/source3/modules/vfs_fruit.c
+++ b/source3/modules/vfs_fruit.c
@@ -3049,11 +3049,12 @@ static int fruit_fallocate(struct vfs_handle_struct *handle,
 	}
 
 	if (!fruit_fsp_recheck(ad, fsp)) {
-		return errno;
+		return -1;
 	}
 
 	/* Let the pwrite code path handle it. */
-	return ENOSYS;
+	errno = ENOSYS;
+	return -1;
 }
 
 static int fruit_ftruncate(struct vfs_handle_struct *handle,
diff --git a/source3/modules/vfs_streams_xattr.c b/source3/modules/vfs_streams_xattr.c
index f0ab732..5c5a9a1 100644
--- a/source3/modules/vfs_streams_xattr.c
+++ b/source3/modules/vfs_streams_xattr.c
@@ -1103,11 +1103,12 @@ static int streams_xattr_fallocate(struct vfs_handle_struct *handle,
 	}
 
 	if (!streams_xattr_recheck(sio)) {
-		return errno;
+		return -1;
 	}
 
 	/* Let the pwrite code path handle it. */
-	return ENOSYS;
+	errno = ENOSYS;
+	return -1;
 }
 
 
-- 
2.2.0.rc0.207.ga3a616c



More information about the samba-technical mailing list