Make handling of -oallow_root easier to understand

-oallow_root is handled in userspace, and requires passing -oallow_other
 to the kernel. This patch should make the code easier to understand and
 avoid the confusion that gave rise to issue #86.

5 files changed, 25 insertions, 37 deletions

diff --git a/ChangeLog.rst b/ChangeLog.rst
index 999f431..ed9ca06 100644
--- a/ChangeLog.rst
+++ b/ChangeLog.rst
@@ -30,6 +30,11 @@ UNRELEASED CHANGES
   changed. Previously, this was handled by the kernel but subject to
   race conditions.
 
+* The ``allow_other`` and ``allow_root`` mount options (accepted by
+  `fuse_session_new()`) may now be specified together. In this case,
+  ``allow_root`` takes precedence.
+
+
 FUSE 3.0.0-rc2 (2016-11-06)
 ===========================
 
diff --git a/lib/fuse_i.h b/lib/fuse_i.h
index 1e99c6f..54466f6 100644
--- a/lib/fuse_i.h
+++ b/lib/fuse_i.h
@@ -47,7 +47,7 @@ struct fuse_session {
 	int fd;
 	struct mount_opts *mo;
 	int debug;
-	int allow_root;
+	int deny_others;
 	struct fuse_lowlevel_ops op;
 	int got_init;
 	struct cuse_data *cuse_data;
diff --git a/lib/fuse_lowlevel.c b/lib/fuse_lowlevel.c
index 9386f1a..4855961 100644
--- a/lib/fuse_lowlevel.c
+++ b/lib/fuse_lowlevel.c
@@ -2491,7 +2491,8 @@ void fuse_session_process_buf_int(struct fuse_session *se,
 		goto reply_err;
 
 	err = EACCES;
-	if (se->allow_root && in->uid != se->owner && in->uid != 0 &&
+	/* Implement -o allow_root */
+	if (se->deny_others && in->uid != se->owner && in->uid != 0 &&
 		 in->opcode != FUSE_INIT && in->opcode != FUSE_READ &&
 		 in->opcode != FUSE_WRITE && in->opcode != FUSE_FSYNC &&
 		 in->opcode != FUSE_RELEASE && in->opcode != FUSE_READDIR &&
@@ -2562,7 +2563,7 @@ static const struct fuse_opt fuse_ll_opts[] = {
 	LL_OPTION("debug", debug, 1),
 	LL_OPTION("-d", debug, 1),
 	LL_OPTION("--debug", debug, 1),
-	LL_OPTION("allow_root", allow_root, 1),
+	LL_OPTION("allow_root", deny_others, 1),
 	FUSE_OPT_END
 };
 
@@ -2578,8 +2579,8 @@ void fuse_lowlevel_help(void)
 	/* These are not all options, but the ones that are
 	   potentially of interest to an end-user */
 	printf(
-"    -o allow_other         allow access to other users\n"
-"    -o allow_root          allow access to root\n"
+"    -o allow_other         allow access by all users\n"
+"    -o allow_root          allow access by root\n"
 "    -o auto_unmount        auto unmount on process termination\n");
 }
 
@@ -2792,10 +2793,20 @@ struct fuse_session *fuse_session_new(struct fuse_args *args,
 	se->conn.max_readahead = UINT_MAX;
 
 	/* Parse options */
+	if(fuse_opt_parse(args, se, fuse_ll_opts, NULL) == -1)
+		goto out2;
+	if(se->deny_others) {
+		/* Allowing access only by root is done by instructing
+		 * kernel to allow access by everyone, and then restricting
+		 * access to root and mountpoint owner in libfuse.
+		 */
+		// We may be adding the option a second time, but
+		// that doesn't hurt.
+		if(fuse_opt_add_arg(args, "-oallow_other") == -1)
+			goto out2;
+	}
 	mo = parse_mount_opts(args);
 	if (mo == NULL)
-		goto out2;
-	if(fuse_opt_parse(args, se, fuse_ll_opts, NULL) == -1)
 		goto out3;
 
 	if(args->argc == 1 &&
diff --git a/lib/mount.c b/lib/mount.c
index 5bd2858..8fbebbf 100644
--- a/lib/mount.c
+++ b/lib/mount.c
@@ -57,13 +57,12 @@ enum {
 	KEY_FUSERMOUNT_OPT,
 	KEY_SUBTYPE_OPT,
 	KEY_MTAB_OPT,
-	KEY_ALLOW_ROOT,
+	KEY_ALLOW_OTHER,
 	KEY_RO,
 };
 
 struct mount_opts {
 	int allow_other;
-	int allow_root;
 	int flags;
 	int auto_unmount;
 	int blkdev;
@@ -80,14 +79,12 @@ struct mount_opts {
 
 static const struct fuse_opt fuse_mount_opts[] = {
 	FUSE_MOUNT_OPT("allow_other",		allow_other),
-	FUSE_MOUNT_OPT("allow_root",		allow_root),
 	FUSE_MOUNT_OPT("blkdev",		blkdev),
 	FUSE_MOUNT_OPT("auto_unmount",		auto_unmount),
 	FUSE_MOUNT_OPT("fsname=%s",		fsname),
 	FUSE_MOUNT_OPT("max_read=%u",		max_read),
 	FUSE_MOUNT_OPT("subtype=%s",		subtype),
 	FUSE_OPT_KEY("allow_other",		KEY_KERN_OPT),
-	FUSE_OPT_KEY("allow_root",		KEY_ALLOW_ROOT),
 	FUSE_OPT_KEY("auto_unmount",		KEY_FUSERMOUNT_OPT),
 	FUSE_OPT_KEY("blkdev",			KEY_FUSERMOUNT_OPT),
 	FUSE_OPT_KEY("fsname=",			KEY_FUSERMOUNT_OPT),
@@ -185,15 +182,10 @@ static void set_mount_flag(const char *s, int *flags)
 static int fuse_mount_opt_proc(void *data, const char *arg, int key,
 			       struct fuse_args *outargs)
 {
+	(void) outargs;
 	struct mount_opts *mo = data;
 
 	switch (key) {
-	case KEY_ALLOW_ROOT:
-		if (fuse_opt_add_opt(&mo->kernel_opts, "allow_other") == -1 ||
-		    fuse_opt_add_arg(outargs, "-oallow_root") == -1)
-			return -1;
-		return 0;
-
 	case KEY_RO:
 		arg = "ro";
 		/* fall through */
@@ -548,11 +540,6 @@ struct mount_opts *parse_mount_opts(struct fuse_args *args)
 	    fuse_opt_parse(args, mo, fuse_mount_opts, fuse_mount_opt_proc) == -1)
 		goto err_out;
 
-	if (mo->allow_other && mo->allow_root) {
-		fprintf(stderr, "fuse: 'allow_other' and 'allow_root' options are mutually exclusive\n");
-		goto err_out;
-	}
-
 	return mo;
 
 err_out:
diff --git a/lib/mount_bsd.c b/lib/mount_bsd.c
index 8bff2cf..f0eea6f 100644
--- a/lib/mount_bsd.c
+++ b/lib/mount_bsd.c
@@ -31,14 +31,12 @@
 #define FUSE_DEV_TRUNK		"/dev/fuse"
 
 enum {
-	KEY_ALLOW_ROOT,
 	KEY_RO,
 	KEY_KERN
 };
 
 struct mount_opts {
 	int allow_other;
-	int allow_root;
 	char *kernel_opts;
 	unsigned max_read;
 };
@@ -48,9 +46,7 @@ struct mount_opts {
 
 static const struct fuse_opt fuse_mount_opts[] = {
 	{ "allow_other", offsetof(struct mount_opts, allow_other), 1 },
-	{ "allow_root", offsetof(struct mount_opts, allow_root), 1 },
 	{ "max_read=%u", offsetof(struct mount_opts, max_read), 1 },
-	FUSE_OPT_KEY("allow_root",		KEY_ALLOW_ROOT),
 	FUSE_OPT_KEY("-r",			KEY_RO),
 	/* standard FreeBSD mount options */
 	FUSE_DUAL_OPT_KEY("dev",		KEY_KERN),
@@ -111,12 +107,6 @@ static int fuse_mount_opt_proc(void *data, const char *arg, int key,
 	struct mount_opts *mo = data;
 
 	switch (key) {
-	case KEY_ALLOW_ROOT:
-		if (fuse_opt_add_opt(&mo->kernel_opts, "allow_other") == -1 ||
-		    fuse_opt_add_arg(outargs, "-oallow_root") == -1)
-			return -1;
-		return 0;
-
 	case KEY_RO:
 		arg = "ro";
 		/* fall through */
@@ -311,11 +301,6 @@ struct mount_opts *parse_mount_opts(struct fuse_args *args)
 	    fuse_opt_parse(args, mo, fuse_mount_opts, fuse_mount_opt_proc) == -1)
 		goto err_out;
 
-	if (mo->allow_other && mo->allow_root) {
-		fprintf(stderr, "fuse: 'allow_other' and 'allow_root' options are mutually exclusive\n");
-		goto err_out;
-	}
-
 	return mo;
 
 err_out: