Bug 752807 - Do not return -1 from STACK_WIND targets ever
Summary: Do not return -1 from STACK_WIND targets ever
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: hekafs
Version: 16
Hardware: All
OS: Linux
high
medium
Target Milestone: ---
Assignee: Kaleb KEITHLEY
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-11-10 13:24 UTC by Kaleb KEITHLEY
Modified: 2011-11-23 00:59 UTC (History)
3 users (show)

Fixed In Version: hekafs-0.7-18.fc16
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-11-23 00:59:24 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Kaleb KEITHLEY 2011-11-10 13:24:21 UTC
Description of problem:

My hekafs server was misconfigured and some functions failed in places
where they normally would not, and that uncovered a problem: whenever
that happened, the client would hang. This happens because we cannot
return errors like normal people do in the kernel, by throwing an
error code. Functions called through STACK_WIND must return zero and
report the error through the callback.

Not sure what errno to set in this case. Generally I just want to
indicate that "something is busted". In the past I would return EDOM.
The ENOSPC seems ridiculous enough here.

Version-Release number of selected component (if applicable):

All

How reproducible:

set uid or gid ranges 0, e.g. uidmin/uidmax: 10000/10000.

Steps to Reproduce:

N/A

Actual results:

N/A

Expected results:

N/A

Additional info:

diff --git a/xlators/features/uidmap/src/uidmap.c b/xlators/features/uidmap/src/uidmap.c
index 91104fd..191fb57 100644
--- a/xlators/features/uidmap/src/uidmap.c
+++ b/xlators/features/uidmap/src/uidmap.c
@@ -34,6 +34,9 @@
 #include "common-utils.h"
 #include "uidmap.h"
 
+/* ugly #includes below */
+#include <errno.h>
+
 #define HFS_UID_LOW_DEFAULT 10000
 #define HFS_UID_HIGH_DEFAULT 19999
 #define HFS_GID_LOW_DEFAULT 10000
@@ -1350,7 +1353,8 @@ uidmap_entrylk(call_frame_t *frame, xlator_t *this,
 		   ((type == ENTRYLK_RDLCK) ? "ENTRYLK_RDLCK" : "ENTRYLK_WRLCK"));
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(entrylk, frame, -1, ENOSPC);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_entrylk_cbk,
@@ -1373,7 +1377,8 @@ uidmap_fentrylk(call_frame_t *frame, xlator_t *this,
 		   ((type == ENTRYLK_RDLCK) ? "ENTRYLK_RDLCK" : "ENTRYLK_WRLCK"));
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(entrylk, frame, -1, ENOSPC);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_fentrylk_cbk,
@@ -1389,7 +1394,8 @@ uidmap_inodelk(call_frame_t *frame, xlator_t *this, const char *volume,
 		  loc_t *loc, int32_t cmd, struct gf_flock *flock)
 {
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(inodelk, frame, -1, ENOSPC);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_inodelk_cbk,
@@ -1409,7 +1415,8 @@ uidmap_finodelk(call_frame_t *frame, xlator_t *this, const char *volume,
 		   frame->root->unique, volume, fd, cmd);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(finodelk, frame, -1, ENOSPC);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_finodelk_cbk,
@@ -1429,7 +1436,8 @@ uidmap_xattrop(call_frame_t *frame, xlator_t *this, loc_t *loc,
 		   frame->root->unique, loc->path, loc->inode->ino, flags);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(xattrop, frame, -1, ENOSPC, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_xattrop_cbk,
@@ -1450,7 +1458,8 @@ uidmap_fxattrop(call_frame_t *frame, xlator_t *this, fd_t *fd,
 		   frame->root->unique, fd, flags);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(fxattrop, frame, -1, ENOSPC, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_fxattrop_cbk,
@@ -1473,7 +1482,9 @@ uidmap_lookup(call_frame_t *frame, xlator_t *this,
 		   loc->inode->ino);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(lookup, frame, -1, ENOSPC,
+					NULL, NULL, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_lookup_cbk,
@@ -1493,7 +1504,8 @@ uidmap_stat(call_frame_t *frame, xlator_t *this, loc_t *loc)
 		   frame->root->unique, loc->path, loc->inode->ino);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(stat, frame, -1, ENOSPC, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_stat_cbk,
@@ -1514,7 +1526,8 @@ uidmap_rchecksum(call_frame_t *frame, xlator_t *this, fd_t *fd,
 		   frame->root->unique, fd->inode->ino);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(rchecksum, frame, -1, ENOSPC, 0, 0);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_rchecksum_cbk,
@@ -1535,7 +1548,8 @@ uidmap_getspec(call_frame_t *frame, xlator_t *this, const char *key,
 		   frame->root->unique, key, flag);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(getspec, frame, -1, ENOSPC, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_getspec_cbk,
@@ -1555,7 +1569,8 @@ uidmap_readlink(call_frame_t *frame, xlator_t *this, loc_t *loc, size_t size)
 		   frame->root->unique, loc->path, loc->inode->ino, size);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(readlink, frame, -1, ENOSPC, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_readlink_cbk,
@@ -1576,7 +1591,9 @@ uidmap_mknod(call_frame_t *frame, xlator_t *this, loc_t *loc,
 		   frame->root->unique, loc->path, loc->inode->ino, mode, dev);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(mknod, frame, -1, ENOSPC, NULL, NULL,
+					NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_mknod_cbk,
@@ -1598,7 +1615,9 @@ uidmap_mkdir(call_frame_t *frame, xlator_t *this, loc_t *loc, mode_t mode,
 		   ((loc->inode)? loc->inode->ino : 0), mode);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(mkdir, frame, -1, ENOSPC,
+					NULL, NULL, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_mkdir_cbk,
@@ -1617,7 +1636,8 @@ uidmap_unlink(call_frame_t *frame, xlator_t *this, loc_t *loc)
 		   frame->root->unique, loc->path, loc->inode->ino);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(unlink, frame, -1, ENOSPC, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_unlink_cbk,
@@ -1636,7 +1656,8 @@ uidmap_rmdir(call_frame_t *frame, xlator_t *this, loc_t *loc, int flags)
 		   frame->root->unique, loc->path, loc->inode->ino, flags);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(rmdir, frame, -1, ENOSPC, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_rmdir_cbk,
@@ -1658,7 +1679,9 @@ uidmap_symlink(call_frame_t *frame, xlator_t *this, const char *linkpath,
 		   ((loc->inode)? loc->inode->ino : 0));
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(symlink, frame, -1, ENOSPC,
+					NULL, NULL, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_symlink_cbk,
@@ -1680,7 +1703,9 @@ uidmap_rename(call_frame_t *frame, xlator_t *this, loc_t *oldloc, loc_t *newloc)
 		   newloc->path, newloc->ino);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(rename, frame, -1, ENOSPC,
+					NULL, NULL, NULL, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_rename_cbk,
@@ -1702,7 +1727,9 @@ uidmap_link(call_frame_t *frame, xlator_t *this, loc_t *oldloc, loc_t *newloc)
 		   newloc->path, newloc->inode->ino);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(link, frame, -1, ENOSPC,
+				NULL, NULL, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_link_cbk,
@@ -1727,7 +1754,9 @@ uidmap_setattr(call_frame_t *frame, xlator_t *this, loc_t *loc,
 		scratch_cs.gid = stbuf->ia_gid;
 		if (((*uidmap_map)(frame->root, this->name) == -1) ||
 		    ((*uidmap_map)(&scratch_cs, this->name) == -1)) {
-			return -1;
+			STACK_UNWIND_STRICT(setattr, frame, -1, ENOSPC,
+						NULL, NULL);
+			return 0;
 		}
 		stbuf->ia_uid = scratch_cs.uid;
 		stbuf->ia_gid = scratch_cs.gid;
@@ -1756,7 +1785,9 @@ uidmap_fsetattr(call_frame_t *frame, xlator_t *this, fd_t *fd,
 		scratch_cs.gid = stbuf->ia_gid;
 		if (((*uidmap_map)(frame->root, this->name) == -1) ||
 		    ((*uidmap_map)(&scratch_cs, this->name) == -1)) {
-			return -1;
+			STACK_UNWIND_STRICT(fsetattr, frame, -1, ENOSPC,
+						NULL, NULL);
+			return 0;
 		}
 		stbuf->ia_uid = scratch_cs.uid;
 		stbuf->ia_gid = scratch_cs.gid;
@@ -1780,7 +1811,8 @@ uidmap_truncate(call_frame_t *frame, xlator_t *this, loc_t *loc,
 		   frame->root->unique, loc->path, loc->inode->ino, offset);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(truncate, frame, -1, ENOSPC, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_truncate_cbk,
@@ -1803,7 +1835,8 @@ uidmap_open(call_frame_t *frame, xlator_t *this, loc_t *loc,
 		   fd, wbflags);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(open, frame, -1, ENOSPC, 0);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_open_cbk,
@@ -1823,7 +1856,9 @@ uidmap_create(call_frame_t *frame, xlator_t *this, loc_t *loc,
 		   frame->root->unique, loc->path, loc->inode->ino, flags, mode);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(create, frame, -1, ENOSPC,
+					0, NULL, NULL, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_create_cbk,
@@ -1843,7 +1878,9 @@ uidmap_readv(call_frame_t *frame, xlator_t *this, fd_t *fd,
 		   frame->root->unique, fd, size, offset);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(readv, frame, -1, ENOSPC,
+					NULL, 0, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_readv_cbk,
@@ -1864,7 +1901,8 @@ uidmap_writev(call_frame_t *frame, xlator_t *this, fd_t *fd,
 		   frame->root->unique, fd, vector, count, offset);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(writev, frame, -1, ENOSPC, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_writev_cbk,
@@ -1884,7 +1922,8 @@ uidmap_statfs(call_frame_t *frame, xlator_t *this, loc_t *loc)
 		   ((loc->inode)? loc->inode->ino : 0));
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(statfs, frame, -1, ENOSPC, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_statfs_cbk,
@@ -1902,7 +1941,8 @@ uidmap_flush(call_frame_t *frame, xlator_t *this, fd_t *fd)
 		   "%"PRId64": (*fd=%p)", frame->root->unique, fd);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(flush, frame, -1, ENOSPC);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_flush_cbk,
@@ -1920,7 +1960,8 @@ uidmap_fsync(call_frame_t *frame, xlator_t *this, fd_t *fd, int32_t flags)
 		   "%"PRId64": (flags=%d, *fd=%p)", frame->root->unique, flags, fd);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(fsync, frame, -1, ENOSPC, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_fsync_cbk,
@@ -1941,7 +1982,8 @@ uidmap_setxattr(call_frame_t *frame, xlator_t *this,
 		   ((loc->inode)? loc->inode->ino : 0), dict, flags);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(setxattr, frame, -1, ENOSPC);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_setxattr_cbk,
@@ -1962,7 +2004,8 @@ uidmap_getxattr(call_frame_t *frame, xlator_t *this,
 		   ((loc->inode)? loc->inode->ino : 0), name);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(getxattr, frame, -1, ENOSPC, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_getxattr_cbk,
@@ -1983,7 +2026,8 @@ uidmap_fsetxattr(call_frame_t *frame, xlator_t *this,
 		   ((fd->inode)? fd->inode->ino : 0), dict, flags);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(fsetxattr, frame, -1, ENOSPC);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_fsetxattr_cbk,
@@ -2004,7 +2048,8 @@ uidmap_fgetxattr(call_frame_t *frame, xlator_t *this,
 		   ((fd->inode)? fd->inode->ino : 0), name);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(fgetxattr, frame, -1, ENOSPC, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_fgetxattr_cbk,
@@ -2025,7 +2070,8 @@ uidmap_removexattr(call_frame_t *frame, xlator_t *this,
 		   ((loc->inode)? loc->inode->ino : 0), name);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(removexattr, frame, -1, ENOSPC);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_removexattr_cbk,
@@ -2045,7 +2091,8 @@ uidmap_opendir(call_frame_t *frame, xlator_t *this, loc_t *loc, fd_t *fd)
 		   frame->root->unique, loc->path, loc->inode->ino, fd);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(opendir, frame, -1, ENOSPC, 0);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_opendir_cbk,
@@ -2064,7 +2111,8 @@ uidmap_readdirp(call_frame_t *frame, xlator_t *this, fd_t *fd, size_t size,
 		   frame->root->unique, fd, size, offset);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(readdirp, frame, -1, ENOSPC, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_readdirp_cbk,
@@ -2085,7 +2133,8 @@ uidmap_readdir(call_frame_t *frame, xlator_t *this, fd_t *fd,
 		   frame->root->unique, fd, size, offset);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(readdir, frame, -1, ENOSPC, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_readdir_cbk,
@@ -2106,7 +2155,8 @@ uidmap_fsyncdir(call_frame_t *frame, xlator_t *this,
 		   frame->root->unique, datasync, fd);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(fsyncdir, frame, -1, ENOSPC);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_fsyncdir_cbk,
@@ -2126,7 +2176,8 @@ uidmap_access(call_frame_t *frame, xlator_t *this, loc_t *loc, int32_t mask)
 		   ((loc->inode)? loc->inode->ino : 0), mask);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(access, frame, -1, ENOSPC);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_access_cbk,
@@ -2146,7 +2197,8 @@ uidmap_ftruncate(call_frame_t *frame, xlator_t *this,
 		   frame->root->unique, offset, fd);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(ftruncate, frame, -1, ENOSPC, NULL, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_ftruncate_cbk,
@@ -2165,7 +2217,8 @@ uidmap_fstat(call_frame_t *frame, xlator_t *this, fd_t *fd)
 		   "%"PRId64": (*fd=%p)", frame->root->unique, fd);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(fstat, frame, -1, ENOSPC, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_fstat_cbk,
@@ -2187,7 +2240,8 @@ uidmap_lk(call_frame_t *frame, xlator_t *this, fd_t *fd,
 		   lock->l_start, lock->l_len, lock->l_pid);
 
 	if ((*uidmap_map)(frame->root, this->name) == -1) {
-		return -1;
+		STACK_UNWIND_STRICT(lk, frame, -1, ENOSPC, NULL);
+		return 0;
 	}
 
 	STACK_WIND(frame, uidmap_lk_cbk,

Comment 1 Kaleb KEITHLEY 2011-11-10 14:51:30 UTC
Seems to me that EPERM (operation not permitted) or EINVAL (invalid argument) would be better?

EREMOTEIO (remote I/O error), or even just EIO?

Comment 2 Pete Zaitcev 2011-11-10 16:14:55 UTC
EIO sounds sensible. I didn't want EINVAL because that implies an error
by the client, whereas it's just an internal server issue.

Comment 3 Fedora Update System 2011-11-14 16:38:10 UTC
hekafs-0.7-18.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/hekafs-0.7-18.fc16

Comment 4 Fedora Update System 2011-11-14 22:24:33 UTC
Package hekafs-0.7-18.fc16:
* should fix your issue,
* was pushed to the Fedora 16 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing hekafs-0.7-18.fc16'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2011-15882
then log in and leave karma (feedback).

Comment 5 Fedora Update System 2011-11-23 00:59:24 UTC
hekafs-0.7-18.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.