2016-02-29 08:17:24

by Andreas Gruenbacher

[permalink] [raw]
Subject: [PATCH v18 19/22] richacl: Add richacl xattr handler

Add richacl xattr handler implementing the xattr operations based on the
get_richacl and set_richacl inode operations.

Signed-off-by: Andreas Gruenbacher <[email protected]>
---
fs/richacl_xattr.c | 73 +++++++++++++++++++++++++++++++++++++++++++
include/linux/richacl_xattr.h | 2 ++
2 files changed, 75 insertions(+)

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 */
--
2.4.3


2016-03-11 14:17:35

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v18 19/22] richacl: Add richacl xattr handler

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.

2016-03-11 14:19:05

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH v18 19/22] richacl: Add richacl xattr handler

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.

2016-03-15 07:10:14

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v18 19/22] richacl: Add richacl xattr handler

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.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

2016-03-15 21:05:26

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH v18 19/22] richacl: Add richacl xattr handler

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.

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

2016-03-21 16:09:22

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v18 19/22] richacl: Add richacl xattr handler

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.