| Message ID | 20160629190731.GF53123@ubuntu-hedt (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
"Michael j Theall" <mtheall@us.ibm.com> writes: > 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. 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. That makes an interesting question. Is it desirable to keep inode->i_mode in sync with the posix acls in fuse or should a filesystem that supports posix acls worry about that? The fact that MS_POSIXACL is set indicates that fuse supports posix acls. Have posix acls never worked then? Eric -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@canonical.com> wrote on 06/29/2016 02:07:31 PM: > > > From: Seth Forshee <seth.forshee@canonical.com> > > To: fuse-devel@lists.sourceforge.net, linux-fsdevel@vger.kernel.org > > Cc: "Eric W. Biederman" <ebiederm@xmission.com>, Miklos Szeredi > > <miklos@szeredi.hu> > > 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@szeredi.hu>"); > > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 29, 2016 at 02:52:37PM -0500, Michael j Theall wrote: > > More comments: > > My patch does set_cached_acl() in fuse_get_acl(). I would investigate if > this is necessary. Hmm, on the one hand it looks like a lot of filesystems do call it, on the other hand it looks like get_acl is caching it. I'll have to take a closer look. > Since your patch invalidates the cached acls on getattr(), I would > investigate on whether you also need to invalidate cached acls on the > entries returned by readdirplus. Possibly so. I missed the fact that fuse_direntplus_link might update the inode attributes. > > 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? > > I wouldn't exactly call it "support". Basically by turning off > default_permissions, the fuser server can do what it wants permission-wise. > My filesystem turns off default_permissions in order to support ACLs, but > as far as fuse is concerned, it is totally oblivious and blindly does what > my server says is permitted. That being said, there still lies the problem > of checking permissions along the path. The low-level interface only gives > you the final inode, which in the case of hard links, the path is > ambiguous. This means you can't check each path component for permissions > (and the kernel certainly won't! since you turned off default_permissions). > Therefore the only "correct" way to implement ACL support on the fuse > server is by setting the attr/entry timeout to 0 to force the lookup every > time, which defeats many purposes of using fuse in the first place. You can > read all the gory details in the thread I linked containing my original > patch. > > Fortunately, this patch (and mine) allows you to use default_permissions > *and* have acl support, meaning permission checks will be performed in > kernel for each path component. The extra benefit is that it allows any > backing filesystem which supports xattrs to support acls, provided your > fuse server has unencumbered privileges for that filesystem. But if we're going through the posix acl xattr handlers then the acls will be cached. What I worry about is that somehow we could end up in a situation where the cached acls don't match those that the filesystem is using, and userspace sees one thing in response to getfacl but something different is being enforced. Thanks, Seth > > Regards, > Michael Theall > > Michael j Theall/Houston/IBM@IBMUS wrote on 06/29/2016 02:24:14 PM: > > > From: Michael j Theall/Houston/IBM@IBMUS > > To: Seth Forshee <seth.forshee@canonical.com> > > Cc: fuse-devel@lists.sourceforge.net, linux-fsdevel@vger.kernel.org, > > Miklos@mx0a-001b2d01.pphosted.com, Szeredi <miklos@szeredi.hu> > > Date: 06/29/2016 02:37 PM > > Subject: Re: [fuse-devel] [RFC] fuse: Support posix ACLs > > > > 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. 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. > > > > Regards, > > Michael Theall > > > > Seth Forshee <seth.forshee@canonical.com> wrote on 06/29/2016 02:07:31 > PM: > > > > > From: Seth Forshee <seth.forshee@canonical.com> > > > To: fuse-devel@lists.sourceforge.net, linux-fsdevel@vger.kernel.org > > > Cc: "Eric W. Biederman" <ebiederm@xmission.com>, Miklos Szeredi > > > <miklos@szeredi.hu> > > > 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@szeredi.hu>"); > > > 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 > > > > > > ------------------------------------------------------------------------------ > > > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Seth Forshee wrote: > 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 My best wishes go with you. > 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? ntfs-3g has both variants implemented. When supporting ACLs within the userspace, it does not set default_permissions, and it uses null cache timeouts. When expecting ACLs supported at the kernel level, it sets default_permissions and it uses non_null cache timeouts. It sets FUSE_CAP_DONT_MASK in both cases. I would expect default_permissions to make a clear divide between those conditions, avoiding any conflicts. Jean-Pierre -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
ebiederm@xmission.com (Eric W. Biederman) wrote: > "Michael j Theall" <mtheall@us.ibm.com> writes: > >> 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. 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. > > That makes an interesting question. Is it desirable to keep > inode->i_mode in sync with the posix acls in fuse or should a filesystem > that supports posix acls worry about that? Using a former implementation of ACLs within fuse at the kernel level, I got the result below. File systems expect consistency. # Using the low level interface of fuse, with use of ACLs # intended to be checked in the kernel, but not related to # access control rm -rf trydir mkdir trydir echo file > trydir/file ls -l trydir/file setfacl -m 'u::7,g::5,o::5' trydir/file ls -l trydir/file sleep 1 ls -l trydir/file -rw-r--r-- 1 root root 5 2009-09-12 12:02 trydir/file -rw-r--r-- 1 root root 5 2009-09-12 12:02 trydir/file -rwxr-xr-x 1 root root 5 2009-09-12 12:02 trydir/file Jean-Pierre -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 29, 2016 at 03:18:24PM -0500, Eric W. Biederman wrote: > "Michael j Theall" <mtheall@us.ibm.com> writes: > > > 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. 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. > > That makes an interesting question. Is it desirable to keep > inode->i_mode in sync with the posix acls in fuse or should a filesystem > that supports posix acls worry about that? My first blush opinion is that the kernel should take care of this, not the filesystems. Then a fuse filesystem which supports xattrs gets acl support for free. Otherwise if a filesystem supports xattrs but not acls internally, we have no way of knowing that in the kernel and they get out of sync. However if some filesystems are already doing this internally then we have redundancy. Presumably this would be harmless aside from the wasted effort. Seth -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Seth Forshee <seth.forshee@canonical.com> writes: > On Wed, Jun 29, 2016 at 03:18:24PM -0500, Eric W. Biederman wrote: >> "Michael j Theall" <mtheall@us.ibm.com> writes: >> >> > 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. 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. >> >> That makes an interesting question. Is it desirable to keep >> inode->i_mode in sync with the posix acls in fuse or should a filesystem >> that supports posix acls worry about that? > > My first blush opinion is that the kernel should take care of this, not > the filesystems. Then a fuse filesystem which supports xattrs gets acl > support for free. Otherwise if a filesystem supports xattrs but not acls > internally, we have no way of knowing that in the kernel and they get > out of sync. > > However if some filesystems are already doing this internally then we > have redundancy. Presumably this would be harmless aside from the wasted > effort. Which means that in set_acl we need to something like: if (type == ACL_TYPE_ACCESS) { struct iattr attr; attr.ia_valid = ATTR_MODE; attr.ia_mode = inode->i_mode; ret = posix_acl_equiv_mode(acl, &attr.ia_mode); if (ret < 0) return ret; if (ret == 0) acl = NULL; if (attr.ia_mode != inode->i_mode) { ret = fuse_do_setattr(inode, &attr, NULL); if (ret < 0) return ret; } } In fuse_setattr should wind up looking something like: static int fuse_setattr(struct dentry *entry, struct iattr *attr) { struct inode *inode = d_inode(entry); int ret; if (!fuse_allow_current_process(get_fuse_conn(inode))) return -EACCES; if (attr->ia_valid & ATTR_FILE) ret = fuse_do_setattr(inode, attr, attr->ia_file); else ret = fuse_do_setattr(inode, attr, NULL); if (ret == 0 && attr->ia_valid & ATTR_MODE) ret = posix_acl_chmod(inode, inode->i_mode); return ret; } That should be enough to keep everything in sync with the existing fuse protocol. And then fuse filesystems won't have to care in general about the contents of acls (unless they choose to care). Eric -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 30, 2016 at 11:25:32AM -0500, Eric W. Biederman wrote: > Seth Forshee <seth.forshee@canonical.com> writes: > > > On Wed, Jun 29, 2016 at 03:18:24PM -0500, Eric W. Biederman wrote: > >> "Michael j Theall" <mtheall@us.ibm.com> writes: > >> > >> > 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. 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. > >> > >> That makes an interesting question. Is it desirable to keep > >> inode->i_mode in sync with the posix acls in fuse or should a filesystem > >> that supports posix acls worry about that? > > > > My first blush opinion is that the kernel should take care of this, not > > the filesystems. Then a fuse filesystem which supports xattrs gets acl > > support for free. Otherwise if a filesystem supports xattrs but not acls > > internally, we have no way of knowing that in the kernel and they get > > out of sync. > > > > However if some filesystems are already doing this internally then we > > have redundancy. Presumably this would be harmless aside from the wasted > > effort. > > Which means that in set_acl we need to something like: > > if (type == ACL_TYPE_ACCESS) { > struct iattr attr; > attr.ia_valid = ATTR_MODE; > attr.ia_mode = inode->i_mode; > ret = posix_acl_equiv_mode(acl, &attr.ia_mode); > if (ret < 0) > return ret; > if (ret == 0) > acl = NULL; > if (attr.ia_mode != inode->i_mode) { > ret = fuse_do_setattr(inode, &attr, NULL); > if (ret < 0) > return ret; > } > } > > In fuse_setattr should wind up looking something like: > > static int fuse_setattr(struct dentry *entry, struct iattr *attr) > { > struct inode *inode = d_inode(entry); > int ret; > > if (!fuse_allow_current_process(get_fuse_conn(inode))) > return -EACCES; > > if (attr->ia_valid & ATTR_FILE) > ret = fuse_do_setattr(inode, attr, attr->ia_file); > else > ret = fuse_do_setattr(inode, attr, NULL); > > if (ret == 0 && attr->ia_valid & ATTR_MODE) > ret = posix_acl_chmod(inode, inode->i_mode); > return ret; > } > > That should be enough to keep everything in sync with the existing > fuse protocol. And then fuse filesystems won't have to care in general > about the contents of acls (unless they choose to care). Yes, I've already written pretty much the same code and am attempting to test it. The problem I'm having is finding a good filesystem to test with. fusexmp works but is probably unfair as the underlying filesystem is handling the acls and updating the mode. I haven't found any filesystem yet that fully supports xattrs but doesn't do something special with the posix acl xattrs. Can anyone suggest a good filesystem for me to test with? Thanks, Seth -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jun 29 2016, Seth Forshee <seth.forshee@canonical.com> wrote: > 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? > I think as long as the kernel interprets ACLs only if default_permission is used, you should be fine. Best, -Nikolaus
On Jun 29 2016, ebiederm@xmission.com (Eric W. Biederman) wrote: > "Michael j Theall" <mtheall@us.ibm.com> writes: > >> 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. 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. > > That makes an interesting question. Is it desirable to keep > inode->i_mode in sync with the posix acls in fuse or should a filesystem > that supports posix acls worry about that? A FUSE file system should be able to support ACLs without requiring the file system process to do more than support extended attributes. I believe this means that the kernel should keep i_mode and the ACLs in sync -- it would be a rather bug prone and redundant for each FUSE file system to implement its own parser for format in which the ACLs are stored in xattrs. Best, -Nikolaus
On Jun 30 2016, Seth Forshee <seth.forshee@canonical.com> wrote: > Yes, I've already written pretty much the same code and am attempting to > test it. The problem I'm having is finding a good filesystem to test > with. fusexmp works but is probably unfair as the underlying filesystem > is handling the acls and updating the mode. I haven't found any > filesystem yet that fully supports xattrs but doesn't do something > special with the posix acl xattrs. > > Can anyone suggest a good filesystem for me to test with? S3QL should do it with a small patch - at the moment it deliberately returns an error when the kernel tries to acces system.posix_acl*. This is done specifically so that the kernel doesn't think that ACLs are supported when they actually aren't (because of the missing synchronization with the permission bits). Download from https://bitbucket.org/nikratio/s3ql/ and comment out the check in src/s3ql/fs.py:getxattr(): # http://code.google.com/p/s3ql/issues/detail?id=385 elif name in (b'system.posix_acl_access', b'system.posix_acl_default'): raise FUSEError(ACL_ERRNO) Best, -Nikolaus
On Fri, Jul 01, 2016 at 12:33:40PM -0700, Nikolaus Rath wrote: > On Jun 29 2016, ebiederm@xmission.com (Eric W. Biederman) wrote: > > "Michael j Theall" <mtheall@us.ibm.com> writes: > > > >> 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. 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. > > > > That makes an interesting question. Is it desirable to keep > > inode->i_mode in sync with the posix acls in fuse or should a filesystem > > that supports posix acls worry about that? > > A FUSE file system should be able to support ACLs without requiring the > file system process to do more than support extended attributes. I > believe this means that the kernel should keep i_mode and the ACLs in > sync -- it would be a rather bug prone and redundant for each FUSE file > system to implement its own parser for format in which the ACLs are > stored in xattrs. The most recent patch I posted keeps them in sync. Thanks, Seth -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 01, 2016 at 12:29:24PM -0700, Nikolaus Rath wrote: > On Jun 29 2016, Seth Forshee <seth.forshee@canonical.com> wrote: > > 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? > > > I think as long as the kernel interprets ACLs only if default_permission > is used, you should be fine. With !default_permission fuse never calls generic_permission so the kernel won't enforce the acls regardless. For the purpose of user namespace mounts it's still useful if the kernel intercepts them so that the posix acl layer can do the uid/gid translation before passing it to the filesystem. The xattrs still get sent on to the filesystem, however cached acls if present would be used to satisfy reads of the acl xatts. Thanks, Seth -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@szeredi.hu>"); 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;