[fuse-devel] [RFC] fuse: Support posix ACLs

[Seth F. <set...@ca...>]

Eric and I are working towards adding support for fuse mounts in
non-init user namespaces. Towards that end we'd like to add ACL support
to fuse as this will allow for a cleaner implementation overall. Below
is an initial patch to support this. I'd like to get some general
feedback on this patch and ask a couple of specific questions.

There are some indications that fuse supports ACLs on the userspace side
when default_permissions is not used (though I'm not seeing how that
works). Will these changes conflict with that support, and if how do we
avoid those conflicts?

Are there any places where we need to invalidate cached ACLs that aren't
covered in the patch?

Thanks,
Seth


diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 8466e122ee62..2f53b0491159 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -13,6 +13,8 @@
 #include <linux/sched.h>
 #include <linux/namei.h>
 #include <linux/slab.h>
+#include <linux/xattr.h>
+#include <linux/posix_acl_xattr.h>
 
 static bool fuse_use_readdirplus(struct inode *dir, struct dir_context *ctx)
 {
@@ -1061,6 +1063,7 @@ static int fuse_perm_getattr(struct inode *inode, int mask)
 	if (mask & MAY_NOT_BLOCK)
 		return -ECHILD;
 
+	forget_all_cached_acls(inode);
 	return fuse_do_getattr(inode, NULL, NULL);
 }
 
@@ -1719,9 +1722,8 @@ static int fuse_getattr(struct vfsmount *mnt, struct dentry *entry,
 	return fuse_update_attributes(inode, stat, NULL, NULL);
 }
 
-static int fuse_setxattr(struct dentry *unused, struct inode *inode,
-			 const char *name, const void *value,
-			 size_t size, int flags)
+static int fuse_setxattr(struct inode *inode, const char *name,
+			 const void *value, size_t size, int flags)
 {
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	FUSE_ARGS(args);
@@ -1755,8 +1757,8 @@ static int fuse_setxattr(struct dentry *unused, struct inode *inode,
 	return err;
 }
 
-static ssize_t fuse_getxattr(struct dentry *entry, struct inode *inode,
-			     const char *name, void *value, size_t size)
+static ssize_t fuse_getxattr(struct inode *inode, const char *name,
+			     void *value, size_t size)
 {
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	FUSE_ARGS(args);
@@ -1838,9 +1840,8 @@ static ssize_t fuse_listxattr(struct dentry *entry, char *list, size_t size)
 	return ret;
 }
 
-static int fuse_removexattr(struct dentry *entry, const char *name)
+static int fuse_removexattr(struct inode *inode, const char *name)
 {
-	struct inode *inode = d_inode(entry);
 	struct fuse_conn *fc = get_fuse_conn(inode);
 	FUSE_ARGS(args);
 	int err;
@@ -1865,6 +1866,138 @@ static int fuse_removexattr(struct dentry *entry, const char *name)
 	return err;
 }
 
+static int fuse_xattr_get(const struct xattr_handler *handler,
+			  struct dentry *dentry, struct inode *inode,
+			  const char *name, void *value, size_t size)
+{
+	return fuse_getxattr(inode, name, value, size);
+}
+
+static int fuse_xattr_set(const struct xattr_handler *handler,
+			  struct dentry *dentry, struct inode *inode,
+			  const char *name, const void *value, size_t size,
+			  int flags)
+{
+	if (!value)
+		return fuse_removexattr(inode, name);
+
+	return fuse_setxattr(inode, name, value, size, flags);
+}
+
+static const struct xattr_handler fuse_xattr_handler = {
+	.prefix	= "",
+	.get	= fuse_xattr_get,
+	.set	= fuse_xattr_set,
+};
+
+#ifndef CONFIG_POSIX_ACL
+static int fuse_xattr_acl_get(const struct xattr_handler *handler,
+			      struct dentry *dentry, struct inode *inode,
+			      const char *name, void *value, size_t size)
+{
+	return -EOPNOTSUPP;
+}
+
+static int fuse_xattr_acl_set(const struct xattr_handler *handler,
+				struct dentry *dentry, struct inode *inode,
+				const char *name, const void *value, size_t size,
+				int flags)
+{
+	return -EOPNOTSUPP;
+}
+
+static const struct xattr_handler fuse_xattr_acl_access_handler = {
+	.name	= XATTR_NAME_POSIX_ACL_ACCESS,
+	.get	= fuse_xattr_acl_get,
+	.set	= fuse_xattr_acl_set,
+};
+
+static const struct xattr_handler fuse_xattr_acl_default_handler = {
+	.name	= XATTR_NAME_POSIX_ACL_DEFAULT,
+	.get	= fuse_xattr_acl_get,
+	.set	= fuse_xattr_acl_set,
+};
+#endif /* CONFIG_POSIX_ACL */
+
+const struct xattr_handler *fuse_xattr_handlers[] = {
+#ifdef CONFIG_FS_POSIX_ACL
+	&posix_acl_access_xattr_handler,
+	&posix_acl_default_xattr_handler,
+#else
+	&fuse_xattr_acl_access_handler,
+	&fuse_xattr_acl_default_handler,
+#endif
+	&fuse_xattr_handler,
+};
+
+static struct posix_acl *fuse_get_acl(struct inode *inode, int type)
+{
+	int size;
+	const char *name;
+	void *value = NULL;
+	struct posix_acl *acl;
+
+	if (type == ACL_TYPE_ACCESS)
+		name = XATTR_NAME_POSIX_ACL_ACCESS;
+	else if (type == ACL_TYPE_DEFAULT)
+		name = XATTR_NAME_POSIX_ACL_DEFAULT;
+	else
+		return ERR_PTR(-EOPNOTSUPP);
+
+	size = fuse_getxattr(inode, name, NULL, 0);
+	if (size > 0) {
+		value = kzalloc(size, GFP_KERNEL);
+		if (!value)
+			return ERR_PTR(-ENOMEM);
+		size = fuse_getxattr(inode, name, value, size);
+	}
+	if (size > 0) {
+		acl = posix_acl_from_xattr(inode->i_sb->s_user_ns, value, size);
+	} else if ((size == 0) || (size == -ENODATA)) {
+		acl = NULL;
+	} else {
+		acl = ERR_PTR(size);
+	}
+	kfree(value);
+
+	return acl;
+}
+
+static int fuse_set_acl(struct inode *inode, struct posix_acl *acl, int type)
+{
+	const char *name;
+	int ret;
+
+	if (type == ACL_TYPE_ACCESS)
+		name = XATTR_NAME_POSIX_ACL_ACCESS;
+	else if (type == ACL_TYPE_DEFAULT)
+		name = XATTR_NAME_POSIX_ACL_DEFAULT;
+	else
+		return -EINVAL;
+
+	if (acl) {
+		struct user_namespace *s_user_ns = inode->i_sb->s_user_ns;
+		size_t size = posix_acl_xattr_size(acl->a_count);
+		void *value = kmalloc(size, GFP_KERNEL);
+		if (!value)
+			return -ENOMEM;
+
+		ret = posix_acl_to_xattr(s_user_ns, acl, value, size);
+		if (ret < 0) {
+			kfree(value);
+			return ret;
+		}
+
+		ret = fuse_setxattr(inode, name, value, size, 0);
+		kfree(value);
+	} else {
+		ret = fuse_removexattr(inode, name);
+	}
+	if (ret == 0)
+		set_cached_acl(inode, type, acl);
+	return ret;
+}
+
 static const struct inode_operations fuse_dir_inode_operations = {
 	.lookup		= fuse_lookup,
 	.mkdir		= fuse_mkdir,
@@ -1879,10 +2012,12 @@ static const struct inode_operations fuse_dir_inode_operations = {
 	.mknod		= fuse_mknod,
 	.permission	= fuse_permission,
 	.getattr	= fuse_getattr,
-	.setxattr	= fuse_setxattr,
-	.getxattr	= fuse_getxattr,
+	.setxattr	= generic_setxattr,
+	.getxattr	= generic_getxattr,
 	.listxattr	= fuse_listxattr,
-	.removexattr	= fuse_removexattr,
+	.removexattr	= generic_removexattr,
+	.get_acl	= fuse_get_acl,
+	.set_acl	= fuse_set_acl,
 };
 
 static const struct file_operations fuse_dir_operations = {
@@ -1900,10 +2035,12 @@ static const struct inode_operations fuse_common_inode_operations = {
 	.setattr	= fuse_setattr,
 	.permission	= fuse_permission,
 	.getattr	= fuse_getattr,
-	.setxattr	= fuse_setxattr,
-	.getxattr	= fuse_getxattr,
+	.setxattr	= generic_setxattr,
+	.getxattr	= generic_getxattr,
 	.listxattr	= fuse_listxattr,
-	.removexattr	= fuse_removexattr,
+	.removexattr	= generic_removexattr,
+	.get_acl	= fuse_get_acl,
+	.set_acl	= fuse_set_acl,
 };
 
 static const struct inode_operations fuse_symlink_inode_operations = {
@@ -1911,10 +2048,12 @@ static const struct inode_operations fuse_symlink_inode_operations = {
 	.get_link	= fuse_get_link,
 	.readlink	= generic_readlink,
 	.getattr	= fuse_getattr,
-	.setxattr	= fuse_setxattr,
-	.getxattr	= fuse_getxattr,
+	.setxattr	= generic_setxattr,
+	.getxattr	= generic_getxattr,
 	.listxattr	= fuse_listxattr,
-	.removexattr	= fuse_removexattr,
+	.removexattr	= generic_removexattr,
+	.get_acl	= fuse_get_acl,
+	.set_acl	= fuse_set_acl,
 };
 
 void fuse_init_common(struct inode *inode)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 9f4c3c82edd6..02c08796051e 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -23,6 +23,7 @@
 #include <linux/poll.h>
 #include <linux/workqueue.h>
 #include <linux/kref.h>
+#include <linux/xattr.h>
 #include <linux/pid_namespace.h>
 #include <linux/user_namespace.h>
 
@@ -693,6 +694,8 @@ extern const struct file_operations fuse_dev_operations;
 
 extern const struct dentry_operations fuse_dentry_operations;
 
+extern const struct xattr_handler *fuse_xattr_handlers[];
+
 /**
  * Inode to nodeid comparison.
  */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 254f1944ee98..9c1519c269bb 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -21,6 +21,7 @@
 #include <linux/sched.h>
 #include <linux/exportfs.h>
 #include <linux/pid_namespace.h>
+#include <linux/posix_acl.h>
 
 MODULE_AUTHOR("Miklos Szeredi <mi...@sz...>");
 MODULE_DESCRIPTION("Filesystem in Userspace");
@@ -339,6 +340,7 @@ int fuse_reverse_inval_inode(struct super_block *sb, u64 nodeid,
 		return -ENOENT;
 
 	fuse_invalidate_attr(inode);
+	forget_all_cached_acls(inode);
 	if (offset >= 0) {
 		pg_start = offset >> PAGE_SHIFT;
 		if (len <= 0)
@@ -1066,6 +1068,7 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
 	}
 	sb->s_magic = FUSE_SUPER_MAGIC;
 	sb->s_op = &fuse_super_operations;
+	sb->s_xattr = fuse_xattr_handlers;
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
 	sb->s_time_gran = 1;
 	sb->s_export_op = &fuse_export_operations;