Andrew,
This patch removes checks for zero uid (spotted by you). These cannot
be replaced with checking for capable(CAP_SYS_ADMIN), since for mount
this capability will always be set. Better aproach seems to be to
move the checks to fusermount (the mount utility provided with the
FUSE library).
Signed-off-by: Miklos Szeredi <[email protected]>
diff -rup linux-2.6.11-rc1-mm1/fs/fuse/inode.c linux-2.6.11-rc1-mm1-fuse/fs/fuse/inode.c
--- linux-2.6.11-rc1-mm1/fs/fuse/inode.c 2005-01-14 12:30:07.000000000 +0100
+++ linux-2.6.11-rc1-mm1-fuse/fs/fuse/inode.c 2005-01-14 12:44:36.000000000 +0100
@@ -15,7 +15,6 @@
#include <linux/seq_file.h>
#include <linux/init.h>
#include <linux/module.h>
-#include <linux/moduleparam.h>
#include <linux/parser.h>
#include <linux/statfs.h>
@@ -25,15 +24,6 @@ MODULE_LICENSE("GPL");
spinlock_t fuse_lock;
static kmem_cache_t *fuse_inode_cachep;
-static int mount_count;
-
-static int user_allow_other;
-module_param(user_allow_other, int, 0644);
-MODULE_PARM_DESC(user_allow_other, "Allow non root user to specify the \"allow_other\" or \"allow_root\" mount options");
-
-static int mount_max = 1000;
-module_param(mount_max, int, 0644);
-MODULE_PARM_DESC(mount_max, "Maximum number of FUSE mounts allowed, if -1 then unlimited (default: 1000)");
#define FUSE_SUPER_MAGIC 0x65735546
@@ -199,7 +189,6 @@ static void fuse_put_super(struct super_
struct fuse_conn *fc = get_fuse_conn_super(sb);
spin_lock(&fuse_lock);
- mount_count --;
fc->sb = NULL;
fc->user_id = 0;
fc->flags = 0;
@@ -512,17 +501,6 @@ static struct super_operations fuse_supe
.show_options = fuse_show_options,
};
-static int inc_mount_count(void)
-{
- int success = 0;
- spin_lock(&fuse_lock);
- mount_count ++;
- if (mount_max == -1 || mount_count <= mount_max)
- success = 1;
- spin_unlock(&fuse_lock);
- return success;
-}
-
static int fuse_fill_super(struct super_block *sb, void *data, int silent)
{
struct fuse_conn *fc;
@@ -534,11 +512,6 @@ static int fuse_fill_super(struct super_
if (!parse_fuse_opt((char *) data, &d))
return -EINVAL;
- if (!user_allow_other &&
- (d.flags & (FUSE_ALLOW_OTHER | FUSE_ALLOW_ROOT)) &&
- current->uid != 0)
- return -EPERM;
-
sb->s_blocksize = PAGE_CACHE_SIZE;
sb->s_blocksize_bits = PAGE_CACHE_SHIFT;
sb->s_magic = FUSE_SUPER_MAGIC;
@@ -564,10 +537,6 @@ static int fuse_fill_super(struct super_
*get_fuse_conn_super_p(sb) = fc;
- err = -ENFILE;
- if (!inc_mount_count() && current->uid != 0)
- goto err;
-
err = -ENOMEM;
root = get_root_inode(sb, d.rootmode);
if (root == NULL)
@@ -583,7 +552,6 @@ static int fuse_fill_super(struct super_
err:
spin_lock(&fuse_lock);
- mount_count --;
fc->sb = NULL;
fuse_release_conn(fc);
spin_unlock(&fuse_lock);
On Fri, 14 Jan 2005, Miklos Szeredi wrote:
> This patch removes checks for zero uid (spotted by you). These cannot
> be replaced with checking for capable(CAP_SYS_ADMIN), since for mount
> this capability will always be set. Better aproach seems to be to
> move the checks to fusermount (the mount utility provided with the
> FUSE library).
>
> Signed-off-by: Miklos Szeredi <[email protected]>
> diff -rup linux-2.6.11-rc1-mm1/fs/fuse/inode.c linux-2.6.11-rc1-mm1-fuse/fs/fuse/inode.c
> --- linux-2.6.11-rc1-mm1/fs/fuse/inode.c 2005-01-14 12:30:07.000000000 +0100
> +++ linux-2.6.11-rc1-mm1-fuse/fs/fuse/inode.c 2005-01-14 12:44:36.000000000 +0100
[snip]
> @@ -534,11 +512,6 @@ static int fuse_fill_super(struct super_
> if (!parse_fuse_opt((char *) data, &d))
> return -EINVAL;
>
> - if (!user_allow_other &&
> - (d.flags & (FUSE_ALLOW_OTHER | FUSE_ALLOW_ROOT)) &&
> - current->uid != 0)
> - return -EPERM;
> -
> sb->s_blocksize = PAGE_CACHE_SIZE;
> sb->s_blocksize_bits = PAGE_CACHE_SHIFT;
> sb->s_magic = FUSE_SUPER_MAGIC;
[snip]
Are you sure you want to do this? Placing security checks inside a
userspace utility and allowing everyone to do it in the kernel means that
any user/hacker could compile their own version of fusermount without the
check and bypass your security... So if you really do not want users to
be able to do this you must do it inside the kernel.
Best regards,
Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/
> Are you sure you want to do this? Placing security checks inside a
> userspace utility and allowing everyone to do it in the kernel means that
> any user/hacker could compile their own version of fusermount without the
> check and bypass your security...
These checks were part of the mount procedure. Since currently
mount() is a privileged operation, it makes no difference if the check
is made inside the kernel or in a (secure) suid userspace app.
> So if you really do not want users to be able to do this you must do
> it inside the kernel.
I'd very much prefer a solution, where in certain situations the
privileges required for mount() could be relaxed. But this involves
more than just a few checks in the FUSE kernel module.
Thanks,
Miklos