| Message ID | 1456733847-17982-20-git-send-email-agruenba@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 29, 2016 at 09:17:24AM +0100, Andreas Gruenbacher wrote: > Add richacl xattr handler implementing the xattr operations based on the > get_richacl and set_richacl inode operations. Given all the issues with Posix ACLs and selinux attributes these really should be proper syscalls instead of abusing the xattr interface. -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 11, 2016 at 06:17:35AM -0800, Christoph Hellwig wrote: > On Mon, Feb 29, 2016 at 09:17:24AM +0100, Andreas Gruenbacher wrote: > > Add richacl xattr handler implementing the xattr operations based on the > > get_richacl and set_richacl inode operations. > > Given all the issues with Posix ACLs and selinux attributes these really > should be proper syscalls instead of abusing the xattr interface. What are those problems exactly? --b. -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Mar 11, 2016 at 09:19:05AM -0500, J. Bruce Fields wrote: > On Fri, Mar 11, 2016 at 06:17:35AM -0800, Christoph Hellwig wrote: > > On Mon, Feb 29, 2016 at 09:17:24AM +0100, Andreas Gruenbacher wrote: > > > Add richacl xattr handler implementing the xattr operations based on the > > > get_richacl and set_richacl inode operations. > > > > Given all the issues with Posix ACLs and selinux attributes these really > > should be proper syscalls instead of abusing the xattr interface. > > What are those problems exactly? That people get confused between the attr used by the xattr syscall interface and the attr used to store things on disk or the protocol. This has happened every time we have non-native support, e.g. XFS, NFS, CIFS, ntfs, etc. And it's only going to become worse. -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 15, 2016 at 12:10:14AM -0700, Christoph Hellwig wrote: > On Fri, Mar 11, 2016 at 09:19:05AM -0500, J. Bruce Fields wrote: > > On Fri, Mar 11, 2016 at 06:17:35AM -0800, Christoph Hellwig wrote: > > > On Mon, Feb 29, 2016 at 09:17:24AM +0100, Andreas Gruenbacher wrote: > > > > Add richacl xattr handler implementing the xattr operations based on the > > > > get_richacl and set_richacl inode operations. > > > > > > Given all the issues with Posix ACLs and selinux attributes these really > > > should be proper syscalls instead of abusing the xattr interface. > > > > What are those problems exactly? > > That people get confused between the attr used by the xattr syscall > interface and the attr used to store things on disk or the protocol. > This has happened every time we have non-native support, e.g. XFS, NFS, > CIFS, ntfs, etc. And it's only going to become worse. How has that confusion caused problems in practice? --b. -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 15, 2016 at 05:05:26PM -0400, J. Bruce Fields wrote: > > That people get confused between the attr used by the xattr syscall > > interface and the attr used to store things on disk or the protocol. > > This has happened every time we have non-native support, e.g. XFS, NFS, > > CIFS, ntfs, etc. And it's only going to become worse. > > How has that confusion caused problems in practice? We had all kinds of bugs in this area that were only slowly uncovered. We also had all kind of privilegue escalations with (non-ACLs) xattrs as people never grasped the way different free-form namespaces have different permission checking. -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" 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/richacl_xattr.c b/fs/richacl_xattr.c index a273139..afa2859 100644 --- a/fs/richacl_xattr.c +++ b/fs/richacl_xattr.c @@ -18,7 +18,9 @@ #include <linux/fs.h> #include <linux/slab.h> #include <linux/module.h> +#include <linux/xattr.h> #include <linux/richacl_xattr.h> +#include <uapi/linux/xattr.h> MODULE_LICENSE("GPL"); @@ -160,3 +162,74 @@ richacl_to_xattr(struct user_namespace *user_ns, return real_size; } EXPORT_SYMBOL_GPL(richacl_to_xattr); + +static bool +richacl_xattr_list(struct dentry *dentry) +{ + return IS_RICHACL(d_backing_inode(dentry)); +} + +static int +richacl_xattr_get(const struct xattr_handler *handler, + struct dentry *dentry, const char *name, void *buffer, + size_t buffer_size) +{ + struct inode *inode = d_backing_inode(dentry); + struct richacl *acl; + int error; + + if (*name) + return -EINVAL; + if (!IS_RICHACL(inode)) + return -EOPNOTSUPP; + if (S_ISLNK(inode->i_mode)) + return -EOPNOTSUPP; + acl = get_richacl(inode); + if (IS_ERR(acl)) + return PTR_ERR(acl); + if (acl == NULL) + return -ENODATA; + error = richacl_to_xattr(current_user_ns(), acl, buffer, buffer_size); + richacl_put(acl); + return error; +} + +static int +richacl_xattr_set(const struct xattr_handler *handler, + struct dentry *dentry, const char *name, + const void *value, size_t size, int flags) +{ + struct inode *inode = d_backing_inode(dentry); + struct richacl *acl = NULL; + int ret; + + if (*name) + return -EINVAL; + if (!IS_RICHACL(inode)) + return -EOPNOTSUPP; + if (!inode->i_op->set_richacl) + return -EOPNOTSUPP; + + if (!uid_eq(current_fsuid(), inode->i_uid) && + inode_permission(inode, MAY_CHMOD) && + !capable(CAP_FOWNER)) + return -EPERM; + + if (value) { + acl = richacl_from_xattr(current_user_ns(), value, size); + if (IS_ERR(acl)) + return PTR_ERR(acl); + } + + ret = inode->i_op->set_richacl(inode, acl); + richacl_put(acl); + return ret; +} + +struct xattr_handler richacl_xattr_handler = { + .name = XATTR_NAME_RICHACL, + .list = richacl_xattr_list, + .get = richacl_xattr_get, + .set = richacl_xattr_set, +}; +EXPORT_SYMBOL(richacl_xattr_handler); diff --git a/include/linux/richacl_xattr.h b/include/linux/richacl_xattr.h index ab67af2..ad4a56e 100644 --- a/include/linux/richacl_xattr.h +++ b/include/linux/richacl_xattr.h @@ -26,4 +26,6 @@ extern size_t richacl_xattr_size(const struct richacl *); extern int richacl_to_xattr(struct user_namespace *, const struct richacl *, void *, size_t); +extern struct xattr_handler richacl_xattr_handler; + #endif /* __RICHACL_XATTR_H */
Add richacl xattr handler implementing the xattr operations based on the get_richacl and set_richacl inode operations. Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com> --- fs/richacl_xattr.c | 73 +++++++++++++++++++++++++++++++++++++++++++ include/linux/richacl_xattr.h | 2 ++ 2 files changed, 75 insertions(+)