[PATCH 0/2] Support for posix ACLs in fuse

classic Classic list List threaded Threaded
26 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2] Support for posix ACLs in fuse

Seth Forshee
Hi Miklos,

Here's an updated set of patches for supporting posix ACLs in fuse. I
think I've incorporated all the feedback from the last RFC series, and
so I've dropped the RFC this time.

I also pushed to github the changes I made to libfuse for testing this.
They're a little rough and probably not 100% complete, but it is
sufficient for exercising the functionality of these patches with
fusexmp.

 https://github.com/sforshee/libfuse/tree/posix-acl

Changes since RFC v3:
 - Add terminating NULL element to fuse_xattr_handlers array.
 - Remove the FUSE_FS_POSIX_ACL config option and select FS_POSIX_ACL
   whenever FUSE_FS is enabled.
 - Use an INIT flag to negotiate ACL support with userspace, only when
   default_permissions is enabled.
 - Use a different set of xattr handlers when ACL support is negotiated,
   preserving the current behavior whenever ACLs are not being enforced
   by the kernel.
 - Use a PAGE_SIZE buffer initially in fuse_get_acl() and fall back to
   querying the xattr size only if that isn't large enough.
 - Remove code to keep ACLs and file mode in sync in the kernel. FUSE
   userspace will be responsible for this.

Thanks,
Seth

Seth Forshee (2):
  fuse: Use generic xattr ops
  fuse: Add posix ACL support

 fs/fuse/Kconfig           |   1 +
 fs/fuse/Makefile          |   2 +-
 fs/fuse/acl.c             | 100 ++++++++++++++++++++++++
 fs/fuse/dir.c             | 192 ++++++++--------------------------------------
 fs/fuse/fuse_i.h          |  23 ++++++
 fs/fuse/inode.c           |  10 +++
 fs/fuse/xattr.c           | 192 ++++++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/fuse.h |   3 +
 8 files changed, 364 insertions(+), 159 deletions(-)
 create mode 100644 fs/fuse/acl.c
 create mode 100644 fs/fuse/xattr.c


------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] fuse: Use generic xattr ops

Seth Forshee
In preparation for posix acl support, rework fuse to use xattr
handlers and the generic setxattr/getxattr/listxattr callbacks.
Split the xattr code out into it's own file, and promote symbols
to module-global scope as needed.

Functionally these changes have no impact, as fuse still uses a
single handler for all xattrs which uses the old callbacks.

Signed-off-by: Seth Forshee <[hidden email]>
---
 fs/fuse/Makefile |   2 +-
 fs/fuse/dir.c    | 167 ++++----------------------------------------------
 fs/fuse/fuse_i.h |   5 ++
 fs/fuse/inode.c  |   1 +
 fs/fuse/xattr.c  | 184 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 202 insertions(+), 157 deletions(-)
 create mode 100644 fs/fuse/xattr.c

diff --git a/fs/fuse/Makefile b/fs/fuse/Makefile
index e95eeb445e58..448aa27ada00 100644
--- a/fs/fuse/Makefile
+++ b/fs/fuse/Makefile
@@ -5,4 +5,4 @@
 obj-$(CONFIG_FUSE_FS) += fuse.o
 obj-$(CONFIG_CUSE) += cuse.o
 
-fuse-objs := dev.o dir.o file.o inode.o control.o
+fuse-objs := dev.o dir.o file.o inode.o control.o xattr.o
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index c47b7780ce37..7490db141dd9 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -13,6 +13,7 @@
 #include <linux/sched.h>
 #include <linux/namei.h>
 #include <linux/slab.h>
+#include <linux/xattr.h>
 
 static bool fuse_use_readdirplus(struct inode *dir, struct dir_context *ctx)
 {
@@ -634,7 +635,7 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
  return create_new_entry(fc, &args, dir, entry, S_IFLNK);
 }
 
-static inline void fuse_update_ctime(struct inode *inode)
+void fuse_update_ctime(struct inode *inode)
 {
  if (!IS_NOCMTIME(inode)) {
  inode->i_ctime = current_fs_time(inode->i_sb);
@@ -1724,152 +1725,6 @@ 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)
-{
- struct fuse_conn *fc = get_fuse_conn(inode);
- FUSE_ARGS(args);
- struct fuse_setxattr_in inarg;
- int err;
-
- if (fc->no_setxattr)
- return -EOPNOTSUPP;
-
- memset(&inarg, 0, sizeof(inarg));
- inarg.size = size;
- inarg.flags = flags;
- args.in.h.opcode = FUSE_SETXATTR;
- args.in.h.nodeid = get_node_id(inode);
- args.in.numargs = 3;
- args.in.args[0].size = sizeof(inarg);
- args.in.args[0].value = &inarg;
- args.in.args[1].size = strlen(name) + 1;
- args.in.args[1].value = name;
- args.in.args[2].size = size;
- args.in.args[2].value = value;
- err = fuse_simple_request(fc, &args);
- if (err == -ENOSYS) {
- fc->no_setxattr = 1;
- err = -EOPNOTSUPP;
- }
- if (!err) {
- fuse_invalidate_attr(inode);
- fuse_update_ctime(inode);
- }
- return err;
-}
-
-static ssize_t fuse_getxattr(struct dentry *entry, struct inode *inode,
-     const char *name, void *value, size_t size)
-{
- struct fuse_conn *fc = get_fuse_conn(inode);
- FUSE_ARGS(args);
- struct fuse_getxattr_in inarg;
- struct fuse_getxattr_out outarg;
- ssize_t ret;
-
- if (fc->no_getxattr)
- return -EOPNOTSUPP;
-
- memset(&inarg, 0, sizeof(inarg));
- inarg.size = size;
- args.in.h.opcode = FUSE_GETXATTR;
- args.in.h.nodeid = get_node_id(inode);
- args.in.numargs = 2;
- args.in.args[0].size = sizeof(inarg);
- args.in.args[0].value = &inarg;
- args.in.args[1].size = strlen(name) + 1;
- args.in.args[1].value = name;
- /* This is really two different operations rolled into one */
- args.out.numargs = 1;
- if (size) {
- args.out.argvar = 1;
- args.out.args[0].size = size;
- args.out.args[0].value = value;
- } else {
- args.out.args[0].size = sizeof(outarg);
- args.out.args[0].value = &outarg;
- }
- ret = fuse_simple_request(fc, &args);
- if (!ret && !size)
- ret = outarg.size;
- if (ret == -ENOSYS) {
- fc->no_getxattr = 1;
- ret = -EOPNOTSUPP;
- }
- return ret;
-}
-
-static ssize_t fuse_listxattr(struct dentry *entry, char *list, size_t size)
-{
- struct inode *inode = d_inode(entry);
- struct fuse_conn *fc = get_fuse_conn(inode);
- FUSE_ARGS(args);
- struct fuse_getxattr_in inarg;
- struct fuse_getxattr_out outarg;
- ssize_t ret;
-
- if (!fuse_allow_current_process(fc))
- return -EACCES;
-
- if (fc->no_listxattr)
- return -EOPNOTSUPP;
-
- memset(&inarg, 0, sizeof(inarg));
- inarg.size = size;
- args.in.h.opcode = FUSE_LISTXATTR;
- args.in.h.nodeid = get_node_id(inode);
- args.in.numargs = 1;
- args.in.args[0].size = sizeof(inarg);
- args.in.args[0].value = &inarg;
- /* This is really two different operations rolled into one */
- args.out.numargs = 1;
- if (size) {
- args.out.argvar = 1;
- args.out.args[0].size = size;
- args.out.args[0].value = list;
- } else {
- args.out.args[0].size = sizeof(outarg);
- args.out.args[0].value = &outarg;
- }
- ret = fuse_simple_request(fc, &args);
- if (!ret && !size)
- ret = outarg.size;
- if (ret == -ENOSYS) {
- fc->no_listxattr = 1;
- ret = -EOPNOTSUPP;
- }
- return ret;
-}
-
-static int fuse_removexattr(struct dentry *entry, const char *name)
-{
- struct inode *inode = d_inode(entry);
- struct fuse_conn *fc = get_fuse_conn(inode);
- FUSE_ARGS(args);
- int err;
-
- if (fc->no_removexattr)
- return -EOPNOTSUPP;
-
- args.in.h.opcode = FUSE_REMOVEXATTR;
- args.in.h.nodeid = get_node_id(inode);
- args.in.numargs = 1;
- args.in.args[0].size = strlen(name) + 1;
- args.in.args[0].value = name;
- err = fuse_simple_request(fc, &args);
- if (err == -ENOSYS) {
- fc->no_removexattr = 1;
- err = -EOPNOTSUPP;
- }
- if (!err) {
- fuse_invalidate_attr(inode);
- fuse_update_ctime(inode);
- }
- return err;
-}
-
 static const struct inode_operations fuse_dir_inode_operations = {
  .lookup = fuse_lookup,
  .mkdir = fuse_mkdir,
@@ -1884,10 +1739,10 @@ 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,
 };
 
 static const struct file_operations fuse_dir_operations = {
@@ -1905,10 +1760,10 @@ 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,
 };
 
 static const struct inode_operations fuse_symlink_inode_operations = {
@@ -1916,10 +1771,10 @@ 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,
 };
 
 void fuse_init_common(struct inode *inode)
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index d98d8cc84def..6db54d0bd81b 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -902,6 +902,8 @@ int fuse_allow_current_process(struct fuse_conn *fc);
 
 u64 fuse_lock_owner_id(struct fuse_conn *fc, fl_owner_t id);
 
+void fuse_update_ctime(struct inode *inode);
+
 int fuse_update_attributes(struct inode *inode, struct kstat *stat,
    struct file *file, bool *refreshed);
 
@@ -966,4 +968,7 @@ void fuse_set_initialized(struct fuse_conn *fc);
 void fuse_unlock_inode(struct inode *inode);
 void fuse_lock_inode(struct inode *inode);
 
+ssize_t fuse_listxattr(struct dentry *entry, char *list, size_t size);
+extern const struct xattr_handler *fuse_xattr_handlers[];
+
 #endif /* _FS_FUSE_I_H */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 4e05b51120f4..1e535f31fed0 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1071,6 +1071,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;
diff --git a/fs/fuse/xattr.c b/fs/fuse/xattr.c
new file mode 100644
index 000000000000..9929cbe8cf03
--- /dev/null
+++ b/fs/fuse/xattr.c
@@ -0,0 +1,184 @@
+/*
+  FUSE: Filesystem in Userspace
+  Copyright (C) 2001-2008  Miklos Szeredi <[hidden email]>
+
+  This program can be distributed under the terms of the GNU GPL.
+  See the file COPYING.
+*/
+
+#include "fuse_i.h"
+
+#include <linux/xattr.h>
+
+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);
+ struct fuse_setxattr_in inarg;
+ int err;
+
+ if (fc->no_setxattr)
+ return -EOPNOTSUPP;
+
+ memset(&inarg, 0, sizeof(inarg));
+ inarg.size = size;
+ inarg.flags = flags;
+ args.in.h.opcode = FUSE_SETXATTR;
+ args.in.h.nodeid = get_node_id(inode);
+ args.in.numargs = 3;
+ args.in.args[0].size = sizeof(inarg);
+ args.in.args[0].value = &inarg;
+ args.in.args[1].size = strlen(name) + 1;
+ args.in.args[1].value = name;
+ args.in.args[2].size = size;
+ args.in.args[2].value = value;
+ err = fuse_simple_request(fc, &args);
+ if (err == -ENOSYS) {
+ fc->no_setxattr = 1;
+ err = -EOPNOTSUPP;
+ }
+ if (!err) {
+ fuse_invalidate_attr(inode);
+ fuse_update_ctime(inode);
+ }
+ return err;
+}
+
+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);
+ struct fuse_getxattr_in inarg;
+ struct fuse_getxattr_out outarg;
+ ssize_t ret;
+
+ if (fc->no_getxattr)
+ return -EOPNOTSUPP;
+
+ memset(&inarg, 0, sizeof(inarg));
+ inarg.size = size;
+ args.in.h.opcode = FUSE_GETXATTR;
+ args.in.h.nodeid = get_node_id(inode);
+ args.in.numargs = 2;
+ args.in.args[0].size = sizeof(inarg);
+ args.in.args[0].value = &inarg;
+ args.in.args[1].size = strlen(name) + 1;
+ args.in.args[1].value = name;
+ /* This is really two different operations rolled into one */
+ args.out.numargs = 1;
+ if (size) {
+ args.out.argvar = 1;
+ args.out.args[0].size = size;
+ args.out.args[0].value = value;
+ } else {
+ args.out.args[0].size = sizeof(outarg);
+ args.out.args[0].value = &outarg;
+ }
+ ret = fuse_simple_request(fc, &args);
+ if (!ret && !size)
+ ret = outarg.size;
+ if (ret == -ENOSYS) {
+ fc->no_getxattr = 1;
+ ret = -EOPNOTSUPP;
+ }
+ return ret;
+}
+
+ssize_t fuse_listxattr(struct dentry *entry, char *list, size_t size)
+{
+ struct inode *inode = d_inode(entry);
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ FUSE_ARGS(args);
+ struct fuse_getxattr_in inarg;
+ struct fuse_getxattr_out outarg;
+ ssize_t ret;
+
+ if (!fuse_allow_current_process(fc))
+ return -EACCES;
+
+ if (fc->no_listxattr)
+ return -EOPNOTSUPP;
+
+ memset(&inarg, 0, sizeof(inarg));
+ inarg.size = size;
+ args.in.h.opcode = FUSE_LISTXATTR;
+ args.in.h.nodeid = get_node_id(inode);
+ args.in.numargs = 1;
+ args.in.args[0].size = sizeof(inarg);
+ args.in.args[0].value = &inarg;
+ /* This is really two different operations rolled into one */
+ args.out.numargs = 1;
+ if (size) {
+ args.out.argvar = 1;
+ args.out.args[0].size = size;
+ args.out.args[0].value = list;
+ } else {
+ args.out.args[0].size = sizeof(outarg);
+ args.out.args[0].value = &outarg;
+ }
+ ret = fuse_simple_request(fc, &args);
+ if (!ret && !size)
+ ret = outarg.size;
+ if (ret == -ENOSYS) {
+ fc->no_listxattr = 1;
+ ret = -EOPNOTSUPP;
+ }
+ return ret;
+}
+
+static int fuse_removexattr(struct inode *inode, const char *name)
+{
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ FUSE_ARGS(args);
+ int err;
+
+ if (fc->no_removexattr)
+ return -EOPNOTSUPP;
+
+ args.in.h.opcode = FUSE_REMOVEXATTR;
+ args.in.h.nodeid = get_node_id(inode);
+ args.in.numargs = 1;
+ args.in.args[0].size = strlen(name) + 1;
+ args.in.args[0].value = name;
+ err = fuse_simple_request(fc, &args);
+ if (err == -ENOSYS) {
+ fc->no_removexattr = 1;
+ err = -EOPNOTSUPP;
+ }
+ if (!err) {
+ fuse_invalidate_attr(inode);
+ fuse_update_ctime(inode);
+ }
+ 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,
+};
+
+const struct xattr_handler *fuse_xattr_handlers[] = {
+ &fuse_xattr_handler,
+ NULL
+};
--
2.7.4


------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] fuse: Add posix ACL support

Seth Forshee
In reply to this post by Seth Forshee
Add a new INIT flag, FUSE_POSIX_ACL, for negotiating ACL support
with userspace. When default_permissions is enabled the kernel
will set this flag in FUSE_INIT, and if it is also set in the
response ACL support will be enabled.

When ACL support is enabled, the kernel will cache and have
responsibility for enforcing ACLs. ACL xattrs will be passed to
userspace, which is responsible for updating the ACLs in the
filesystem, keeping the file mode in sync, and inheritance of
default ACLs when new filesystem nodes are created.

Signed-off-by: Seth Forshee <[hidden email]>
---
 fs/fuse/Kconfig           |   1 +
 fs/fuse/Makefile          |   2 +-
 fs/fuse/acl.c             | 100 ++++++++++++++++++++++++++++++++++++++++++++++
 fs/fuse/dir.c             |  25 +++++++++++-
 fs/fuse/fuse_i.h          |  18 +++++++++
 fs/fuse/inode.c           |   9 +++++
 fs/fuse/xattr.c           |  18 ++++++---
 include/uapi/linux/fuse.h |   3 ++
 8 files changed, 168 insertions(+), 8 deletions(-)
 create mode 100644 fs/fuse/acl.c

diff --git a/fs/fuse/Kconfig b/fs/fuse/Kconfig
index 1b2f6c2c3aaf..76f09ce7e5b2 100644
--- a/fs/fuse/Kconfig
+++ b/fs/fuse/Kconfig
@@ -1,5 +1,6 @@
 config FUSE_FS
  tristate "FUSE (Filesystem in Userspace) support"
+ select FS_POSIX_ACL
  help
   With FUSE it is possible to implement a fully functional filesystem
   in a userspace program.
diff --git a/fs/fuse/Makefile b/fs/fuse/Makefile
index 448aa27ada00..60da84a86dab 100644
--- a/fs/fuse/Makefile
+++ b/fs/fuse/Makefile
@@ -5,4 +5,4 @@
 obj-$(CONFIG_FUSE_FS) += fuse.o
 obj-$(CONFIG_CUSE) += cuse.o
 
-fuse-objs := dev.o dir.o file.o inode.o control.o xattr.o
+fuse-objs := dev.o dir.o file.o inode.o control.o xattr.o acl.o
diff --git a/fs/fuse/acl.c b/fs/fuse/acl.c
new file mode 100644
index 000000000000..87220da95e91
--- /dev/null
+++ b/fs/fuse/acl.c
@@ -0,0 +1,100 @@
+/*
+  FUSE: Filesystem in Userspace
+  Copyright (C) 2016 Canonical Ltd. <[hidden email]>
+
+  This program can be distributed under the terms of the GNU GPL.
+  See the file COPYING.
+*/
+
+#include "fuse_i.h"
+
+#include <linux/posix_acl.h>
+#include <linux/posix_acl_xattr.h>
+
+struct posix_acl *fuse_get_acl(struct inode *inode, int type)
+{
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ int size;
+ const char *name;
+ void *value = NULL;
+ struct posix_acl *acl;
+
+ if (!fc->posix_acl || fc->no_getxattr)
+ return NULL;
+
+ 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);
+
+ value = kmalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!value)
+ return ERR_PTR(-ENOMEM);
+ size = fuse_getxattr(inode, name, value, PAGE_SIZE);
+ if (size == -ERANGE) {
+ kfree(value);
+ size = fuse_getxattr(inode, name, NULL, 0);
+ value = kmalloc(size, GFP_KERNEL);
+ if (!value)
+ return ERR_PTR(-ENOMEM);
+ size = fuse_getxattr(inode, name, value, size);
+ }
+
+ if (size > 0)
+ acl = posix_acl_from_xattr(&init_user_ns, value, size);
+ else if ((size == 0) || (size == -ENODATA) ||
+ (size == -EOPNOTSUPP && fc->no_getxattr))
+ acl = NULL;
+ else
+ acl = ERR_PTR(size);
+
+ kfree(value);
+ return acl;
+}
+
+int fuse_set_acl(struct inode *inode, struct posix_acl *acl, int type)
+{
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ const char *name;
+ int ret;
+
+ if (!fc->posix_acl || fc->no_setxattr)
+ return -EOPNOTSUPP;
+
+ 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) {
+ /*
+ * Fuse userspace is responsible for updating access
+ * permissions in the inode, if needed. fuse_setxattr
+ * invalidates the inode attributes, which will force
+ * them to be refreshed the next time they are used,
+ * and it also updates i_ctime.
+ */
+ 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(&init_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;
+}
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 7490db141dd9..d25e00cfe72d 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -14,6 +14,7 @@
 #include <linux/namei.h>
 #include <linux/slab.h>
 #include <linux/xattr.h>
+#include <linux/posix_acl.h>
 
 static bool fuse_use_readdirplus(struct inode *dir, struct dir_context *ctx)
 {
@@ -244,6 +245,7 @@ static int fuse_dentry_revalidate(struct dentry *entry, unsigned int flags)
  if (ret || (outarg.attr.mode ^ inode->i_mode) & S_IFMT)
  goto invalid;
 
+ forget_all_cached_acls(inode);
  fuse_change_attributes(inode, &outarg.attr,
        entry_attr_timeout(&outarg),
        attr_version);
@@ -918,6 +920,7 @@ int fuse_update_attributes(struct inode *inode, struct kstat *stat,
 
  if (time_before64(fi->i_time, get_jiffies_64())) {
  r = true;
+ forget_all_cached_acls(inode);
  err = fuse_do_getattr(inode, stat, file);
  } else {
  r = false;
@@ -1065,6 +1068,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);
 }
 
@@ -1234,6 +1238,7 @@ retry:
  fi->nlookup++;
  spin_unlock(&fc->lock);
 
+ forget_all_cached_acls(inode);
  fuse_change_attributes(inode, &o->attr,
        entry_attr_timeout(o),
        attr_version);
@@ -1703,14 +1708,24 @@ error:
 static int fuse_setattr(struct dentry *entry, struct iattr *attr)
 {
  struct inode *inode = d_inode(entry);
+ struct fuse_conn *fc = get_fuse_conn(inode);
+ int ret;
 
  if (!fuse_allow_current_process(get_fuse_conn(inode)))
  return -EACCES;
 
  if (attr->ia_valid & ATTR_FILE)
- return fuse_do_setattr(inode, attr, attr->ia_file);
+ ret = fuse_do_setattr(inode, attr, attr->ia_file);
  else
- return fuse_do_setattr(inode, attr, NULL);
+ ret = fuse_do_setattr(inode, attr, NULL);
+
+ /*
+ * If filesystem supports acls it may have updated acl xattrs in
+ * the filesystem, so forget cached acls for the inode.
+ */
+ if (!ret && fc->posix_acl)
+ forget_all_cached_acls(inode);
+ return ret;
 }
 
 static int fuse_getattr(struct vfsmount *mnt, struct dentry *entry,
@@ -1743,6 +1758,8 @@ static const struct inode_operations fuse_dir_inode_operations = {
  .getxattr = generic_getxattr,
  .listxattr = fuse_listxattr,
  .removexattr = generic_removexattr,
+ .get_acl = fuse_get_acl,
+ .set_acl = fuse_set_acl,
 };
 
 static const struct file_operations fuse_dir_operations = {
@@ -1764,6 +1781,8 @@ static const struct inode_operations fuse_common_inode_operations = {
  .getxattr = generic_getxattr,
  .listxattr = fuse_listxattr,
  .removexattr = generic_removexattr,
+ .get_acl = fuse_get_acl,
+ .set_acl = fuse_set_acl,
 };
 
 static const struct inode_operations fuse_symlink_inode_operations = {
@@ -1775,6 +1794,8 @@ static const struct inode_operations fuse_symlink_inode_operations = {
  .getxattr = generic_getxattr,
  .listxattr = fuse_listxattr,
  .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 6db54d0bd81b..5d8a6bf6594d 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>
 
 /** Max number of pages that can be used in a single read request */
 #define FUSE_MAX_PAGES_PER_REQ 32
@@ -624,6 +625,9 @@ struct fuse_conn {
  /** Is lseek not implemented by fs? */
  unsigned no_lseek:1;
 
+ /** Does the filesystem support posix acls? */
+ unsigned posix_acl:1;
+
  /** The number of requests waiting for completion */
  atomic_t num_waiting;
 
@@ -968,7 +972,21 @@ void fuse_set_initialized(struct fuse_conn *fc);
 void fuse_unlock_inode(struct inode *inode);
 void fuse_lock_inode(struct inode *inode);
 
+int fuse_setxattr(struct inode *inode, const char *name, const void *value,
+  size_t size, int flags);
+ssize_t fuse_getxattr(struct inode *inode, const char *name, void *value,
+      size_t size);
 ssize_t fuse_listxattr(struct dentry *entry, char *list, size_t size);
+int fuse_removexattr(struct inode *inode, const char *name);
 extern const struct xattr_handler *fuse_xattr_handlers[];
+extern const struct xattr_handler *fuse_acl_xattr_handlers[];
+
+struct posix_acl;
+
+extern const struct xattr_handler fuse_xattr_acl_access_handler;
+extern const struct xattr_handler fuse_xattr_acl_default_handler;
+
+struct posix_acl *fuse_get_acl(struct inode *inode, int type);
+int fuse_set_acl(struct inode *inode, struct posix_acl *acl, int type);
 
 #endif /* _FS_FUSE_I_H */
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 1e535f31fed0..a3cfcb541243 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -20,6 +20,7 @@
 #include <linux/random.h>
 #include <linux/sched.h>
 #include <linux/exportfs.h>
+#include <linux/posix_acl.h>
 
 MODULE_AUTHOR("Miklos Szeredi <[hidden email]>");
 MODULE_DESCRIPTION("Filesystem in Userspace");
@@ -340,6 +341,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)
@@ -912,6 +914,11 @@ static void process_init_reply(struct fuse_conn *fc, struct fuse_req *req)
  fc->parallel_dirops = 1;
  if (arg->time_gran && arg->time_gran <= 1000000000)
  fc->sb->s_time_gran = arg->time_gran;
+ if ((arg->flags & FUSE_POSIX_ACL) &&
+    (fc->flags & FUSE_DEFAULT_PERMISSIONS)) {
+ fc->posix_acl = 1;
+ fc->sb->s_xattr = fuse_acl_xattr_handlers;
+ }
  } else {
  ra_pages = fc->max_read / PAGE_SIZE;
  fc->no_lock = 1;
@@ -942,6 +949,8 @@ static void fuse_send_init(struct fuse_conn *fc, struct fuse_req *req)
  FUSE_DO_READDIRPLUS | FUSE_READDIRPLUS_AUTO | FUSE_ASYNC_DIO |
  FUSE_WRITEBACK_CACHE | FUSE_NO_OPEN_SUPPORT |
  FUSE_PARALLEL_DIROPS;
+ if (fc->flags & FUSE_DEFAULT_PERMISSIONS)
+ arg->flags |= FUSE_POSIX_ACL;
  req->in.h.opcode = FUSE_INIT;
  req->in.numargs = 1;
  req->in.args[0].size = sizeof(*arg);
diff --git a/fs/fuse/xattr.c b/fs/fuse/xattr.c
index 9929cbe8cf03..855e701f8c11 100644
--- a/fs/fuse/xattr.c
+++ b/fs/fuse/xattr.c
@@ -9,9 +9,10 @@
 #include "fuse_i.h"
 
 #include <linux/xattr.h>
+#include <linux/posix_acl_xattr.h>
 
-static int fuse_setxattr(struct inode *inode, const char *name,
- const void *value, size_t size, int flags)
+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);
@@ -45,8 +46,8 @@ static int fuse_setxattr(struct inode *inode, const char *name,
  return err;
 }
 
-static ssize_t fuse_getxattr(struct inode *inode, const char *name,
-     void *value, size_t size)
+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);
@@ -128,7 +129,7 @@ ssize_t fuse_listxattr(struct dentry *entry, char *list, size_t size)
  return ret;
 }
 
-static int fuse_removexattr(struct inode *inode, const char *name)
+int fuse_removexattr(struct inode *inode, const char *name)
 {
  struct fuse_conn *fc = get_fuse_conn(inode);
  FUSE_ARGS(args);
@@ -182,3 +183,10 @@ const struct xattr_handler *fuse_xattr_handlers[] = {
  &fuse_xattr_handler,
  NULL
 };
+
+const struct xattr_handler *fuse_acl_xattr_handlers[] = {
+ &posix_acl_access_xattr_handler,
+ &posix_acl_default_xattr_handler,
+ &fuse_xattr_handler,
+ NULL
+};
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index 27e17363263a..cb48ec4546ce 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -108,6 +108,7 @@
  *
  *  7.25
  *  - add FUSE_PARALLEL_DIROPS
+ *  - add FUSE_POSIX_ACL
  */
 
 #ifndef _LINUX_FUSE_H
@@ -238,6 +239,7 @@ struct fuse_file_lock {
  * FUSE_WRITEBACK_CACHE: use writeback cache for buffered writes
  * FUSE_NO_OPEN_SUPPORT: kernel supports zero-message opens
  * FUSE_PARALLEL_DIROPS: allow parallel lookups and readdir
+ * FUSE_POSIX_ACL: filesystem supports posix acls
  */
 #define FUSE_ASYNC_READ (1 << 0)
 #define FUSE_POSIX_LOCKS (1 << 1)
@@ -258,6 +260,7 @@ struct fuse_file_lock {
 #define FUSE_WRITEBACK_CACHE (1 << 16)
 #define FUSE_NO_OPEN_SUPPORT (1 << 17)
 #define FUSE_PARALLEL_DIROPS    (1 << 18)
+#define FUSE_POSIX_ACL (1 << 19)
 
 /**
  * CUSE INIT request/reply flags
--
2.7.4


------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Nikolaus Rath
In reply to this post by Seth Forshee
Hi Seth,

On Aug 29 2016, Seth Forshee <[hidden email]> wrote:

> Hi Miklos,
>
> Here's an updated set of patches for supporting posix ACLs in fuse. I
> think I've incorporated all the feedback from the last RFC series, and
> so I've dropped the RFC this time.
>
> I also pushed to github the changes I made to libfuse for testing this.
> They're a little rough and probably not 100% complete, but it is
> sufficient for exercising the functionality of these patches with
> fusexmp.
>
>  https://github.com/sforshee/libfuse/tree/posix-acl

I think it would be great to finally have this in FUSE. Thanks for
working on this!

Would you mind submitting a pull request for the userspace changes to
libfuse as well (via GitHub)?

Best,
-Nikolaus

--
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Seth Forshee
On Tue, Sep 06, 2016 at 08:32:21PM -0700, Nikolaus Rath wrote:

> Hi Seth,
>
> On Aug 29 2016, Seth Forshee <[hidden email]> wrote:
> > Hi Miklos,
> >
> > Here's an updated set of patches for supporting posix ACLs in fuse. I
> > think I've incorporated all the feedback from the last RFC series, and
> > so I've dropped the RFC this time.
> >
> > I also pushed to github the changes I made to libfuse for testing this.
> > They're a little rough and probably not 100% complete, but it is
> > sufficient for exercising the functionality of these patches with
> > fusexmp.
> >
> >  https://github.com/sforshee/libfuse/tree/posix-acl
>
> I think it would be great to finally have this in FUSE. Thanks for
> working on this!
>
> Would you mind submitting a pull request for the userspace changes to
> libfuse as well (via GitHub)?

I can, I'll just have to find some time first to make sure I've covered
everything and to clean it all up.

Seth

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Miklos Szeredi
In reply to this post by Seth Forshee
[Adding Andreas Gruenbacher to Cc]

On Mon, Aug 29, 2016 at 3:46 PM, Seth Forshee
<[hidden email]> wrote:
> Hi Miklos,
>
> Here's an updated set of patches for supporting posix ACLs in fuse. I
> think I've incorporated all the feedback from the last RFC series, and
> so I've dropped the RFC this time.

Pushed, with minor changes, to

  git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#for-next

Please verify that I didn't break it.

> I also pushed to github the changes I made to libfuse for testing this.
> They're a little rough and probably not 100% complete, but it is
> sufficient for exercising the functionality of these patches with
> fusexmp.
>
>  https://github.com/sforshee/libfuse/tree/posix-acl

As for the libfuse part:

1) Please don't mess with fusexmp.c.  The added code is really an
anti-example.  Posix acls will will work fine in such pass-through
filesystems without doing anything.  The added complexity just makes
it brittle and racy without actually doing anything positive.

2) You define some constants and structures (POSIX_ACL_*) in
fuse_common.h that don't seem to belong there.  There's <sys/acl.h>
that contains some parts of that, but I'm not sure how much we want to
tie libfuse to libacl...  It's a difficult thing.  Generally I'd try
to keep the interface as narrow as possible.  Perhaps it's enough to
have a a function to return the equivalent mode from the xattr?

3) How will richacl's fit into this?

4) We really need a better example to check the efficiency of the new
interface, but that's hard because we need a "real" filesystem for
that and those are rare.  Ntfs-3g is one such, and it would be
interesting to "port" it to using the new API.

Jean-Pierre, how difficult would that be?

Thanks,
Miklos

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Jean-Pierre André
Hi,

Miklos Szeredi wrote:

> [Adding Andreas Gruenbacher to Cc]
>
> On Mon, Aug 29, 2016 at 3:46 PM, Seth Forshee
> <[hidden email]> wrote:
>> Hi Miklos,
>>
>> Here's an updated set of patches for supporting posix ACLs in fuse. I
>> think I've incorporated all the feedback from the last RFC series, and
>> so I've dropped the RFC this time.
>
> Pushed, with minor changes, to
>
>    git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#for-next
>
> Please verify that I didn't break it.
>
>> I also pushed to github the changes I made to libfuse for testing this.
>> They're a little rough and probably not 100% complete, but it is
>> sufficient for exercising the functionality of these patches with
>> fusexmp.
>>
>>   https://github.com/sforshee/libfuse/tree/posix-acl
>
> As for the libfuse part:
>
> 1) Please don't mess with fusexmp.c.  The added code is really an
> anti-example.  Posix acls will will work fine in such pass-through
> filesystems without doing anything.  The added complexity just makes
> it brittle and racy without actually doing anything positive.
>
> 2) You define some constants and structures (POSIX_ACL_*) in
> fuse_common.h that don't seem to belong there.  There's <sys/acl.h>
> that contains some parts of that, but I'm not sure how much we want to
> tie libfuse to libacl...  It's a difficult thing.  Generally I'd try
> to keep the interface as narrow as possible.  Perhaps it's enough to
> have a a function to return the equivalent mode from the xattr?
>
> 3) How will richacl's fit into this?
>
> 4) We really need a better example to check the efficiency of the new
> interface, but that's hard because we need a "real" filesystem for
> that and those are rare.  Ntfs-3g is one such, and it would be
> interesting to "port" it to using the new API.
>
> Jean-Pierre, how difficult would that be?

The code for Posix ACLs support within the kernel is
already present in ntfs-3g (just have to define the
macro KERNELACLS in order to disable the checks
within ntfs-3g and rely on the kernel ones).

This however relies on assumptions I made a few years
back, and might be invalid now. I will at least have
to set some flag in the init callback.

The ACLs are supposed to be set and retrieved through
the extended attributes system.posix_acl_access and
system.posix_acl_default. Recent posts about it in
this list mention "xattr handlers", do they mean
getxattr() and setxattr() on these extended attributes ?

Also the sync with the file mode is done natively,
as the ACLs and modes are translated to a single
object (an NTFS ACL). There is no need for fuse to
duplicate the mode to a ACL setting or conversely.

Now the main problem for ntfs-3g is to migrate to
libfuse3, still keeping the compatibility with non-Linux
implementations (MacOSX, OpenIndiana, and others).
I have not done anything about it yet.

Does the Posix ACL in-kernel support require libfuse3 ?

Finally, I am still using an old kernel, so I will
have to setup a test environment...

Jean-Pierre

>
> Thanks,
> Miklos
>


------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Seth Forshee
In reply to this post by Miklos Szeredi
On Wed, Sep 21, 2016 at 10:30:14AM +0200, Miklos Szeredi wrote:

> [Adding Andreas Gruenbacher to Cc]
>
> On Mon, Aug 29, 2016 at 3:46 PM, Seth Forshee
> <[hidden email]> wrote:
> > Hi Miklos,
> >
> > Here's an updated set of patches for supporting posix ACLs in fuse. I
> > think I've incorporated all the feedback from the last RFC series, and
> > so I've dropped the RFC this time.
>
> Pushed, with minor changes, to
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#for-next
>
> Please verify that I didn't break it.

I've reviewed the changes and they seem okay, still need to test.

> > I also pushed to github the changes I made to libfuse for testing this.
> > They're a little rough and probably not 100% complete, but it is
> > sufficient for exercising the functionality of these patches with
> > fusexmp.
> >
> >  https://github.com/sforshee/libfuse/tree/posix-acl
>
> As for the libfuse part:
>
> 1) Please don't mess with fusexmp.c.  The added code is really an
> anti-example.  Posix acls will will work fine in such pass-through
> filesystems without doing anything.  The added complexity just makes
> it brittle and racy without actually doing anything positive.

As you note below, it's hard to find a "real" filesystem to test it
with so fusexmp proved convenient for that. But I'll omit it when I
update the pull req.

> 2) You define some constants and structures (POSIX_ACL_*) in
> fuse_common.h that don't seem to belong there.  There's <sys/acl.h>
> that contains some parts of that, but I'm not sure how much we want to
> tie libfuse to libacl...  It's a difficult thing.  Generally I'd try
> to keep the interface as narrow as possible.  Perhaps it's enough to
> have a a function to return the equivalent mode from the xattr?

To be honest I only really meant that to serve as an example of all the
stuff that would need to happen in userspace based on the kernel
implementation. Looking now at libacl I guess it could just be expected
that filesystems will use that. It seems to provide the essentials to do
what I did with fusexmp at least, even an interface for getting the
equivalent mode (acl_equiv_mode). Not sure how well it works if e.g. a
filesystem needs to convert between the posix ACL format and some
different format native to that filesystem.

> 3) How will richacl's fit into this?

I don't know, I haven't looked at those patches closely, but in git I'm
not seeing any support for richacls in fuse yet anyhow.

Thanks,
Seth

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Miklos Szeredi
On Wed, Sep 21, 2016 at 3:41 PM, Seth Forshee
<[hidden email]> wrote:
> On Wed, Sep 21, 2016 at 10:30:14AM +0200, Miklos Szeredi wrote:

>> 2) You define some constants and structures (POSIX_ACL_*) in
>> fuse_common.h that don't seem to belong there.  There's <sys/acl.h>
>> that contains some parts of that, but I'm not sure how much we want to
>> tie libfuse to libacl...  It's a difficult thing.  Generally I'd try
>> to keep the interface as narrow as possible.  Perhaps it's enough to
>> have a a function to return the equivalent mode from the xattr?
>
> To be honest I only really meant that to serve as an example of all the
> stuff that would need to happen in userspace based on the kernel
> implementation. Looking now at libacl I guess it could just be expected
> that filesystems will use that. It seems to provide the essentials to do
> what I did with fusexmp at least, even an interface for getting the
> equivalent mode (acl_equiv_mode). Not sure how well it works if e.g. a
> filesystem needs to convert between the posix ACL format and some
> different format native to that filesystem.

The kernel structures and functions are quite good for converting to
native formats (it's simple and easy to encode/decode).

So we can use that instead of having to link to libacl, which does
seem a not a bit too abstracted.

But we should at least try to use the standard names for the constants.

Thanks,
Miklos

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Miklos Szeredi
In reply to this post by Jean-Pierre André
On Wed, Sep 21, 2016 at 2:25 PM, Jean-Pierre André
<[hidden email]> wrote:

> The code for Posix ACLs support within the kernel is
> already present in ntfs-3g (just have to define the
> macro KERNELACLS in order to disable the checks
> within ntfs-3g and rely on the kernel ones).
>
> This however relies on assumptions I made a few years
> back, and might be invalid now. I will at least have
> to set some flag in the init callback.
>
> The ACLs are supposed to be set and retrieved through
> the extended attributes system.posix_acl_access and
> system.posix_acl_default. Recent posts about it in
> this list mention "xattr handlers", do they mean
> getxattr() and setxattr() on these extended attributes ?

Right.

> Also the sync with the file mode is done natively,
> as the ACLs and modes are translated to a single
> object (an NTFS ACL). There is no need for fuse to
> duplicate the mode to a ACL setting or conversely.
>
> Now the main problem for ntfs-3g is to migrate to
> libfuse3, still keeping the compatibility with non-Linux
> implementations (MacOSX, OpenIndiana, and others).
> I have not done anything about it yet.
>
> Does the Posix ACL in-kernel support require libfuse3 ?

Not really, the most important change (and apparently the only one
that ntfs-3g cares about) will be the added capability flag, but it's
trivial to add it to libfuse2.

Thanks,
Miklos

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Eric W. Biederman
In reply to this post by Miklos Szeredi
Miklos Szeredi <[hidden email]> writes:
> 3) How will richacl's fit into this?

As best as I can read the situation richacl support has not yet been
merged into Linux yet.

Last I was following the richacl discussion there were some fundamental
features of richacls that were incompatible with the expectations of
ordinary linux applications.  The negative acls if my memory serves.
That raised some concern if richacls could ever be safely be merged.

As I recall Christoph Hellwig was a primary on raising those concerns,
and when I read those arguments they seemed persuasive to me.

That said richacls (if they happen) will communicate to the filesystem
primarily with extended attributes just like the acls, and flag will
need to be added to the protocol to enable richacl support in the kernel
if and when that exists.

Or in short richacls are the same tune but different dance partners.

Eric

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Eric W. Biederman
Andreas Grünbacher <[hidden email]> writes:

> 2016-09-21 17:40 GMT+02:00 Eric W. Biederman <[hidden email]>:
>> Miklos Szeredi <[hidden email]> writes:
>>> 3) How will richacl's fit into this?
>>
>> As best as I can read the situation richacl support has not yet been
>> merged into Linux yet.
>
> True. The patches are stuck in Al Viro's inbox since a very long time.
>
>> Last I was following the richacl discussion there were some fundamental
>> features of richacls that were incompatible with the expectations of
>> ordinary linux applications.
>
> Not true.
>
>> The negative acls if my memory serves.
>> That raised some concern if richacls could ever be safely be merged.
>>
>> As I recall Christoph Hellwig was a primary on raising those concerns,
>> and when I read those arguments they seemed persuasive to me.
>
> The whole point of Richacls is to be compatible with the POSIX file
> permission model. Entries that deny permissions are a necessary part
> of Richacls for various reasons. Christoph doesn't like them, but that
> doesn't make them any less necessary.

Negative access control permissions are extremly nasty, and cause a lot
of problems in the real world, especially where they have not existed
before.   And sometimes where they are rare enough to not have existed
before.

I am would prefer you taking a measured approach rather than ignoring
people's concerns.

I certainly don't need any kind of ACLs for any kind of files I have
ever dealt with in practice.  I deal with ACLs and make certain the
kernel supports them well to ensure there are not nasty bugs waiting
in the wings.

If my memory serves there are very funny corner cases between negative
uids ACLs and user namespaces.  The kind that can lead to security
vulnerabilities etc.

The practical problem I would envision is users using user namespaces to
enable them to change their uid or gid and that in turn would result in
negative acls failing to contain deny users access to files.

So I would be very careful with adding negative permission checks that
are going to be (at best) very error prone to use to the current unix
permission model.

Eric

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Michael Theall
In reply to this post by Miklos Szeredi
On Wed, 2016-09-21 at 16:14 +0200, Miklos Szeredi wrote:

> On Wed, Sep 21, 2016 at 2:25 PM, Jean-Pierre André
> <[hidden email]> wrote:
>
> >
> > The code for Posix ACLs support within the kernel is
> > already present in ntfs-3g (just have to define the
> > macro KERNELACLS in order to disable the checks
> > within ntfs-3g and rely on the kernel ones).
> >
> > This however relies on assumptions I made a few years
> > back, and might be invalid now. I will at least have
> > to set some flag in the init callback.
> >
> > The ACLs are supposed to be set and retrieved through
> > the extended attributes system.posix_acl_access and
> > system.posix_acl_default. Recent posts about it in
> > this list mention "xattr handlers", do they mean
> > getxattr() and setxattr() on these extended attributes ?
> Right.
>
> >
> > Also the sync with the file mode is done natively,
> > as the ACLs and modes are translated to a single
> > object (an NTFS ACL). There is no need for fuse to
> > duplicate the mode to a ACL setting or conversely.
> >
> > Now the main problem for ntfs-3g is to migrate to
> > libfuse3, still keeping the compatibility with non-Linux
> > implementations (MacOSX, OpenIndiana, and others).
> > I have not done anything about it yet.
> >
> > Does the Posix ACL in-kernel support require libfuse3 ?
> Not really, the most important change (and apparently the only one
> that ntfs-3g cares about) will be the added capability flag, but it's
> trivial to add it to libfuse2.
>
> Thanks,
> Miklos

Hi,

I'm currently trying to port my code to take advantage of this. I've
found myself in sort of a catch-22 situation: I don't know whether to
pass default_permissions or not.

Right now, I support ACLs by not using default_permissions. However,
the FUSE_POSIX_ACL capability is only given when default_permissions is
turned on. If I unconditionally set FUSE_POSIX_ACL in the "want" flags,
the capability remains disabled if default_permissions is off.

I need to support ACLs on backlevel platforms with default_permissions
off, and the coming platforms with default_permissions on. How do you
suggest I determine whether to provide it or not?

Regards,
Michael Theall


------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Jean-Pierre André
In reply to this post by Miklos Szeredi
Hi,

Miklos Szeredi wrote:
> [Adding Andreas Gruenbacher to Cc]
>
> On Mon, Aug 29, 2016 at 3:46 PM, Seth Forshee
> <[hidden email]> wrote:
>> Hi Miklos,
>>
>> Here's an updated set of patches for supporting posix ACLs in fuse. I
>> think I've incorporated all the feedback from the last RFC series, and
>> so I've dropped the RFC this time.

[...]

> 4) We really need a better example to check the efficiency of the new
> interface, but that's hard because we need a "real" filesystem for
> that and those are rare.  Ntfs-3g is one such, and it would be
> interesting to "port" it to using the new API.
>
> Jean-Pierre, how difficult would that be?
>

In order to set up a test environment, I would like to make
sure I have a correct base.

Are a kernel 4.8.0 and libfuse 2.9.7 relevant for a test ?
 From these bases, what are the needed patches ?
(note : I would like to avoid compiling a full kernel).

I will mainly run the Posix tests from
https://sourceforge.net/p/ntfs-3g/pjd-fstest/ci/master/tree/
By the way, these tests (including the ACL ones) can be used
to check other file systems.

Jean-Pierre



------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Michael Theall
On Thu, 2016-09-22 at 10:09 +0200, Jean-Pierre André wrote:

> Hi,
>
> Miklos Szeredi wrote:
> >
> > [Adding Andreas Gruenbacher to Cc]
> >
> > On Mon, Aug 29, 2016 at 3:46 PM, Seth Forshee
> > <[hidden email]> wrote:
> > >
> > > Hi Miklos,
> > >
> > > Here's an updated set of patches for supporting posix ACLs in
> > > fuse. I
> > > think I've incorporated all the feedback from the last RFC
> > > series, and
> > > so I've dropped the RFC this time.
> [...]
>
> >
> > 4) We really need a better example to check the efficiency of the
> > new
> > interface, but that's hard because we need a "real" filesystem for
> > that and those are rare.  Ntfs-3g is one such, and it would be
> > interesting to "port" it to using the new API.
> >
> > Jean-Pierre, how difficult would that be?
> >
> In order to set up a test environment, I would like to make
> sure I have a correct base.
>
> Are a kernel 4.8.0 and libfuse 2.9.7 relevant for a test ?
>  From these bases, what are the needed patches ?
> (note : I would like to avoid compiling a full kernel).
>
> I will mainly run the Posix tests from
> https://sourceforge.net/p/ntfs-3g/pjd-fstest/ci/master/tree/
> By the way, these tests (including the ACL ones) can be used
> to check other file systems.
>
> Jean-Pierre

Hi Jean-Pierre,

I'm testing this with a 4.7.3 kernel with the following two patches:

https://git.kernel.org/cgit/linux/kernel/git/mszeredi/fuse.git/commit/f
s/fuse?h=for-next&id=169774f3e4fb951c83d71a78130b8a3f507a1ea9
https://git.kernel.org/cgit/linux/kernel/git/mszeredi/fuse.git/commit/f
s/fuse?h=for-next&id=8f40f60a473489f0635d7d49c19ea3b9b6e3e4ff

I'm using Fedora so the FUSE kernel module is loadable. I just grabbed
the kernel source rpm from Fedora, applied these patches to the fs/fuse
directory, and build the FUSE kernel module. Then I unloaded the
existing FUSE kernel module and inserted the one I built.

As for libfuse, I'm using the libfuse3 code from:
https://github.com/sforshee/libfuse/tree/posix-acl

Since my filesystem already supports ACLs through the existing xattr
interface, all I had to do was set FUSE_CAP_POSIX_ACL in the "want"
flags in the "init" callback and write the "setacl" and "getacl"
callbacks that simply wrapped my existing xattr ACL routines.
Unfortunately, I'm not testing the helper code that encodes/decodes the
xattrs since my code already does that.

Regards,
Michael Theall


------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Miklos Szeredi
In reply to this post by Miklos Szeredi
On Wed, Sep 21, 2016 at 11:28 PM, Andreas Grünbacher
<[hidden email]> wrote:

> Using definitions from <sys/acl.h> doesn't necessarily create a
> runtime dependency on libacl.
>
> What does fuse actually need to do with POSIX ACLs? I understand that
> setxattr("system.posix_acl_access") updates both the access ACL and
> the file mode, and so does chmod. Both these operations can be
> implemented quite easily based in the xattr value though; I wouldn't
> choose to use libacl for that.

The xattr representation of posix acls is linux specific as far as I
understand.  By passing this representation to fuse filesystems it
becomes part of the fuse protocol.  This doesn't necessarily mean that
we have to actually decode the acl in the fuse library, but it needs
to be documented at least.

BTW, I find it strange that the xattr representation of posix acls are
not in any uapi header files.

>
>> 3) How will richacl's fit into this?
>
> Richacls work similar to POSIX ACLs from fuse's point of view. The
> algorithms for updating a richacl upon a chmod and for computing the
> new file mode from setxattr("system.richacl") are significantly more
> complicated though; also, librichacl isn't as over engineered as
> libacl, so I would use librichacl for these operations.

Does librichacl have xattr->acl and acl->xattr conversion functions?
Because libacl seems to lack these...

Thanks,
Miklos

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Andreas Grünbacher
2016-09-23 11:01 GMT+02:00 Miklos Szeredi <[hidden email]>:

> On Wed, Sep 21, 2016 at 11:28 PM, Andreas Grünbacher
> <[hidden email]> wrote:
>
>> Using definitions from <sys/acl.h> doesn't necessarily create a
>> runtime dependency on libacl.
>>
>> What does fuse actually need to do with POSIX ACLs? I understand that
>> setxattr("system.posix_acl_access") updates both the access ACL and
>> the file mode, and so does chmod. Both these operations can be
>> implemented quite easily based in the xattr value though; I wouldn't
>> choose to use libacl for that.
>
> The xattr representation of posix acls is linux specific as far as I
> understand.  By passing this representation to fuse filesystems it
> becomes part of the fuse protocol.  This doesn't necessarily mean that
> we have to actually decode the acl in the fuse library, but it needs
> to be documented at least.
>
> BTW, I find it strange that the xattr representation of posix acls are
> not in any uapi header files.

Yes, that should be moved. Those headers are quite a bit older than
the uapi split.

>>> 3) How will richacl's fit into this?
>>
>> Richacls work similar to POSIX ACLs from fuse's point of view. The
>> algorithms for updating a richacl upon a chmod and for computing the
>> new file mode from setxattr("system.richacl") are significantly more
>> complicated though; also, librichacl isn't as over engineered as
>> libacl, so I would use librichacl for these operations.
>
> Does librichacl have xattr->acl and acl->xattr conversion functions?

Yes, for things like ceph and glusterfs.

> Because libacl seems to lack these...

In libacl, those functions are internal to the library. We could start
exporting them, but the in-memory ACL representation isn't exactly
great for many use cases, like fuse probably.

Andreas

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Miklos Szeredi
In reply to this post by Michael Theall
On Wed, Sep 21, 2016 at 10:50 PM, Michael Theall
<[hidden email]> wrote:

> I'm currently trying to port my code to take advantage of this. I've
> found myself in sort of a catch-22 situation: I don't know whether to
> pass default_permissions or not.
>
> Right now, I support ACLs by not using default_permissions. However,
> the FUSE_POSIX_ACL capability is only given when default_permissions is
> turned on. If I unconditionally set FUSE_POSIX_ACL in the "want" flags,
> the capability remains disabled if default_permissions is off.
>
> I need to support ACLs on backlevel platforms with default_permissions
> off, and the coming platforms with default_permissions on. How do you
> suggest I determine whether to provide it or not?

Good point.   FUSE_POSIX_ACL needs to be made independent of (but
implying) default_permissions.

Force pushed to fuse.git#for-next with other minor changes.

Thanks,
Miklos

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Jean-Pierre André
In reply to this post by Michael Theall
Hi Michael,

Michael Theall wrote:
> On Thu, 2016-09-22 at 10:09 +0200, Jean-Pierre André wrote:
>> Hi,
>>
>> Miklos Szeredi wrote:

[...]

>
> Hi Jean-Pierre,
>
> I'm testing this with a 4.7.3 kernel with the following two patches:
>
> https://git.kernel.org/cgit/linux/kernel/git/mszeredi/fuse.git/commit/f
> s/fuse?h=for-next&id=169774f3e4fb951c83d71a78130b8a3f507a1ea9
> https://git.kernel.org/cgit/linux/kernel/git/mszeredi/fuse.git/commit/f
> s/fuse?h=for-next&id=8f40f60a473489f0635d7d49c19ea3b9b6e3e4ff
>
> I'm using Fedora so the FUSE kernel module is loadable. I just grabbed
> the kernel source rpm from Fedora, applied these patches to the fs/fuse
> directory, and build the FUSE kernel module. Then I unloaded the
> existing FUSE kernel module and inserted the one I built.

I have just built a configuration based on kernel 4.7.4
(Fedora 24), and applied your patches mentioned above.
(Now the kernel modules are compressed !)

> As for libfuse, I'm using the libfuse3 code from:
> https://github.com/sforshee/libfuse/tree/posix-acl

I just used the simplified libfuse shipped with ntfs-3g
(which is restricted to the features useful for ntfs-3g),
and only added the passing of FUSE_CAP_POSIX_ACL and
FUSE_POSIX_ACL.

> Since my filesystem already supports ACLs through the existing xattr
> interface, all I had to do was set FUSE_CAP_POSIX_ACL in the "want"
> flags in the "init" callback and write the "setacl" and "getacl"
> callbacks that simply wrapped my existing xattr ACL routines.

I only had to add FUSE_CAP_POSIX_ACL in the want flags,
I have no setacl or getacl callbacks, only setxattr and
friends, which know about the Posix ACLs and translate
them to/from NTFS ACLs (with approximations).

And... the Posix tests run with no error detected !

ntfs-3g can interface either at the high level fuse
interface or the low level one. Both run fine with
the cache disabled, and the low level one also runs
fine with the cache enabled (the high level one does
not, because it cannot cope with hard links).

Congratulations !

Note however that the directory access control from
the cache is not included in these tests (the patch
recently posted by Miklos is next on my todo list).

> Unfortunately, I'm not testing the helper code that encodes/decodes the
> xattrs since my code already does that.

Same for me.

Regards

Jean-Pierre




------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/2] Support for posix ACLs in fuse

Miklos Szeredi
On Fri, Sep 23, 2016 at 6:53 PM, Jean-Pierre André
<[hidden email]> wrote:

> I only had to add FUSE_CAP_POSIX_ACL in the want flags,
> I have no setacl or getacl callbacks, only setxattr and
> friends, which know about the Posix ACLs and translate
> them to/from NTFS ACLs (with approximations).
>
> And... the Posix tests run with no error detected !


Great, thanks for testing.

>> Unfortunately, I'm not testing the helper code that encodes/decodes the
>> xattrs since my code already does that.
>
>
> Same for me.

I guess we should just leave the xattr (de)coding out of libfuse for
now, since there are no actual or potential users of it in sight.

Thanks,
Miklos

------------------------------------------------------------------------------
--
fuse-devel mailing list
To unsubscribe or subscribe, visit https://lists.sourceforge.net/lists/listinfo/fuse-devel
12