On Wed, Jun 29, 2016 at 02:24:14PM -0500, Michael j Theall wrote: > > Going by the patch I posted a couple of years ago: > https://sourceforge.net/p/fuse/mailman/message/33033653/ > > The only hole I see in your patch is that in setattr() you are not updating > the cached acl if the ATTR_MODE is updated. Okay, thanks. > The other major difference is > that my version uses the get_acl/set_acl inode operations but you use that > plus the xattr handlers. I'm not up-to-speed on the kernel so I'm not sure > if you actually need to implement both. The xattr handlers use the get_acl/set_acl callbacks. Without using the xattr handlers acl xattr reads won't be pulled from the acl cache, for one thing. Thanks, Seth > > Regards, > Michael Theall > > Seth Forshee <seth.forshee@xxxxxxxxxxxxx> wrote on 06/29/2016 02:07:31 PM: > > > From: Seth Forshee <seth.forshee@xxxxxxxxxxxxx> > > To: fuse-devel@xxxxxxxxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx > > Cc: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>, Miklos Szeredi > > <miklos@xxxxxxxxxx> > > Date: 06/29/2016 02:08 PM > > Subject: [fuse-devel] [RFC] fuse: Support posix ACLs > > > > 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 <miklos@xxxxxxxxxx>"); > > 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; > > > > > > > ------------------------------------------------------------------------------ > > > Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San > > Francisco, CA to explore cutting-edge tech and listen to tech luminaries > > present their vision of the future. This family event has something for > > everyone, including kids. Get more information and register today. > > http://sdm.link/attshape > > -- > > fuse-devel mailing list > > To unsubscribe or subscribe, visit https://lists.sourceforge.net/ > > lists/listinfo/fuse-devel > > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
