Re: [PATCH v23 18/22] richacl: xattr mapping functions

On Tue, Jul 12, 2016 at 2:02 PM, Jeff Layton [off-list ref] wrote:

On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:

quoted

Map between "system.richacl" xattrs and the in-kernel representation.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/Makefile                        |   2 +-
 fs/richacl_xattr.c                 | 161 +++++++++++++++++++++++++++++++++++++
 include/linux/richacl_xattr.h      |  29 +++++++
 include/uapi/linux/Kbuild          |   1 +
 include/uapi/linux/richacl_xattr.h |  44 ++++++++++
 include/uapi/linux/xattr.h         |   2 +
 6 files changed, 238 insertions(+), 1 deletion(-)
 create mode 100644 fs/richacl_xattr.c
 create mode 100644 include/linux/richacl_xattr.h
 create mode 100644 include/uapi/linux/richacl_xattr.h

diff --git a/fs/Makefile b/fs/Makefile
index 2b3e6f1..262fd67 100644
--- a/fs/Makefile
+++ b/fs/Makefile

@@ -49,7 +49,7 @@ obj-$(CONFIG_COREDUMP)              += coredump.o
 obj-$(CONFIG_SYSCTL)         += drop_caches.o

 obj-$(CONFIG_FHANDLE)                += fhandle.o
-obj-$(CONFIG_FS_RICHACL)     += richacl.o
+obj-$(CONFIG_FS_RICHACL)     += richacl.o richacl_xattr.o

 obj-y                                += quota/

diff --git a/fs/richacl_xattr.c b/fs/richacl_xattr.c
new file mode 100644
index 0000000..dc1ad36
--- /dev/null
+++ b/fs/richacl_xattr.c

@@ -0,0 +1,161 @@
+/*
+ * Copyright (C) 2006, 2010  Novell, Inc.
+ * Copyright (C) 2015  Red Hat, Inc.
+ * Written by Andreas Gruenbacher <agruenba@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+/**
+ * richacl_from_xattr  -  convert a richacl xattr into the in-memory representation
+ */
+struct richacl *
+richacl_from_xattr(struct user_namespace *user_ns,
+                const void *value, size_t size, int invalid_error)
+{
+     const struct richacl_xattr *xattr_acl = value;
+     const struct richace_xattr *xattr_ace = (void *)(xattr_acl + 1);
+     struct richacl *acl;
+     struct richace *ace;
+     int count;
+
+     if (size < sizeof(*xattr_acl) ||
+         xattr_acl->a_version != RICHACL_XATTR_VERSION ||
+         (xattr_acl->a_flags & ~RICHACL_VALID_FLAGS))
+             goto invalid;
+     size -= sizeof(*xattr_acl);
+     count = le16_to_cpu(xattr_acl->a_count);
+     if (count > RICHACL_XATTR_MAX_COUNT)
+             goto invalid;
+     if (size != count * sizeof(*xattr_ace))
+             goto invalid;
+
+     acl = richacl_alloc(count, GFP_NOFS);
+     if (!acl)
+             return ERR_PTR(-ENOMEM);
+
+     acl->a_flags = xattr_acl->a_flags;
+     acl->a_owner_mask = le32_to_cpu(xattr_acl->a_owner_mask);
+     if (acl->a_owner_mask & ~RICHACE_VALID_MASK)
+             goto put_invalid;
+     acl->a_group_mask = le32_to_cpu(xattr_acl->a_group_mask);
+     if (acl->a_group_mask & ~RICHACE_VALID_MASK)
+             goto put_invalid;
+     acl->a_other_mask = le32_to_cpu(xattr_acl->a_other_mask);
+     if (acl->a_other_mask & ~RICHACE_VALID_MASK)
+             goto put_invalid;
+
+     richacl_for_each_entry(ace, acl) {
+             ace->e_type  = le16_to_cpu(xattr_ace->e_type);
+             ace->e_flags = le16_to_cpu(xattr_ace->e_flags);
+             ace->e_mask  = le32_to_cpu(xattr_ace->e_mask);
+
+             if (ace->e_flags & ~RICHACE_VALID_FLAGS)
+                     goto put_invalid;
+             if (ace->e_flags & RICHACE_SPECIAL_WHO) {
+                     ace->e_id.special = le32_to_cpu(xattr_ace->e_id);
+                     if (ace->e_id.special > RICHACE_EVERYONE_SPECIAL_ID)
+                             goto put_invalid;
+             } else if (ace->e_flags & RICHACE_IDENTIFIER_GROUP) {
+                     u32 id = le32_to_cpu(xattr_ace->e_id);
+
+                     ace->e_id.gid = make_kgid(user_ns, id);
+                     if (!gid_valid(ace->e_id.gid))
+                             goto put_invalid;
+             } else {
+                     u32 id = le32_to_cpu(xattr_ace->e_id);
+
+                     ace->e_id.uid = make_kuid(user_ns, id);
+                     if (!uid_valid(ace->e_id.uid))
+                             goto put_invalid;
+             }
+             if (ace->e_type > RICHACE_ACCESS_DENIED_ACE_TYPE ||
+                 (ace->e_mask & ~RICHACE_VALID_MASK))
+                     goto put_invalid;
+
+             xattr_ace++;
+     }
+
+     return acl;
+
+put_invalid:
+     richacl_put(acl);
+invalid:
+     return ERR_PTR(invalid_error);
+}
+EXPORT_SYMBOL_GPL(richacl_from_xattr);
+
+/**
+ * richacl_xattr_size  -  compute the size of the xattr representation of @acl
+ */
+size_t
+richacl_xattr_size(const struct richacl *acl)
+{
+     size_t size = sizeof(struct richacl_xattr);
+
+     size += sizeof(struct richace_xattr) * acl->a_count;
+     return size;
+}
+EXPORT_SYMBOL_GPL(richacl_xattr_size);
+
+/**
+ * richacl_to_xattr  -  convert @acl into its xattr representation
+ * @acl:     the richacl to convert
+ * @buffer:  buffer for the result
+ * @size:    size of @buffer
+ */
+int
+richacl_to_xattr(struct user_namespace *user_ns,
+              const struct richacl *acl, void *buffer, size_t size)
+{
+     struct richacl_xattr *xattr_acl = buffer;
+     struct richace_xattr *xattr_ace;
+     const struct richace *ace;
+     size_t real_size;
+
+     real_size = richacl_xattr_size(acl);
+     if (!buffer)
+             return real_size;
+     if (real_size > size)
+             return -ERANGE;
+
+     xattr_acl->a_version = RICHACL_XATTR_VERSION;
+     xattr_acl->a_flags = acl->a_flags;
+     xattr_acl->a_count = cpu_to_le16(acl->a_count);
+
+     xattr_acl->a_owner_mask = cpu_to_le32(acl->a_owner_mask);
+     xattr_acl->a_group_mask = cpu_to_le32(acl->a_group_mask);
+     xattr_acl->a_other_mask = cpu_to_le32(acl->a_other_mask);
+
+     xattr_ace = (void *)(xattr_acl + 1);
+     richacl_for_each_entry(ace, acl) {
+             xattr_ace->e_type = cpu_to_le16(ace->e_type);
+             xattr_ace->e_flags = cpu_to_le16(ace->e_flags);
+             xattr_ace->e_mask = cpu_to_le32(ace->e_mask);
+             if (ace->e_flags & RICHACE_SPECIAL_WHO)
+                     xattr_ace->e_id = cpu_to_le32(ace->e_id.special);
+             else if (ace->e_flags & RICHACE_IDENTIFIER_GROUP)
+                     xattr_ace->e_id =
+                             cpu_to_le32(from_kgid(user_ns, ace->e_id.gid));
+             else
+                     xattr_ace->e_id =
+                             cpu_to_le32(from_kuid(user_ns, ace->e_id.uid));
+             xattr_ace++;
+     }
+     return real_size;
+}
+EXPORT_SYMBOL_GPL(richacl_to_xattr);

diff --git a/include/linux/richacl_xattr.h b/include/linux/richacl_xattr.h
new file mode 100644
index 0000000..0efa14b
--- /dev/null
+++ b/include/linux/richacl_xattr.h

@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2006, 2010  Novell, Inc.
+ * Copyright (C) 2015  Red Hat, Inc.
+ * Written by Andreas Gruenbacher <agruenba@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2, or (at your option) any
+ * later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#ifndef __RICHACL_XATTR_H
+#define __RICHACL_XATTR_H
+
+#include
+#include
+
+extern struct richacl *richacl_from_xattr(struct user_namespace *, const void *,
+                                       size_t, int);
+extern size_t richacl_xattr_size(const struct richacl *);
+extern int richacl_to_xattr(struct user_namespace *, const struct richacl *,
+                         void *, size_t);
+
+#endif /* __RICHACL_XATTR_H */

diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index abeaa98..c24e106 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild

@@ -356,6 +356,7 @@ header-y += reiserfs_fs.h
 header-y += reiserfs_xattr.h
 header-y += resource.h
 header-y += richacl.h
+header-y += richacl_xattr.h
 header-y += rfkill.h
 header-y += rio_mport_cdev.h
 header-y += romfs_fs.h

diff --git a/include/uapi/linux/richacl_xattr.h b/include/uapi/linux/richacl_xattr.h
new file mode 100644
index 0000000..20da204
--- /dev/null
+++ b/include/uapi/linux/richacl_xattr.h

@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2006, 2010  Novell, Inc.
+ * Copyright (C) 2015  Red Hat, Inc.
+ * Written by Andreas Gruenbacher <agruenba@redhat.com>
+ *
+ * This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This file is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ */
+
+#ifndef __UAPI_RICHACL_XATTR_H
+#define __UAPI_RICHACL_XATTR_H
+
+#include
+#include
+
+struct richace_xattr {
+     __le16  e_type;
+     __le16  e_flags;
+     __le32  e_mask;
+     __le32  e_id;
+};
+
+struct richacl_xattr {
+     __u8    a_version;
+     __u8    a_flags;
+     __le16  a_count;
+     __le32  a_owner_mask;
+     __le32  a_group_mask;
+     __le32  a_other_mask;
+};
+
+#define RICHACL_XATTR_VERSION 0
+#define RICHACL_XATTR_MAX_COUNT \
+     ((XATTR_SIZE_MAX - sizeof(struct richacl_xattr)) / \
+      sizeof(struct richace_xattr))
+
+#endif  /* __UAPI_RICHACL_XATTR_H */

diff --git a/include/uapi/linux/xattr.h b/include/uapi/linux/xattr.h
index 1590c49..1996903 100644
--- a/include/uapi/linux/xattr.h
+++ b/include/uapi/linux/xattr.h

@@ -73,5 +73,7 @@
 #define XATTR_POSIX_ACL_DEFAULT  "posix_acl_default"
 #define XATTR_NAME_POSIX_ACL_DEFAULT XATTR_SYSTEM_PREFIX XATTR_POSIX_ACL_DEFAULT

+#define XATTR_RICHACL "richacl"
+#define XATTR_NAME_RICHACL XATTR_SYSTEM_PREFIX XATTR_RICHACL

 #endif /* _UAPI_LINUX_XATTR_H */

Fair enough. I do wonder a bit whether we might be better served with a
new set of syscalls for this instead of using xattrs (as I think
Christoph has suggested). What _is_ the rationale for doing this with
xattrs, btw?

Xattrs are intended for exactly this kind of auxiliary file
information. They are used for POSIX ACLs as well, and they are not
going to go away. A number of utilities now include support for
xattrs. By using xattrs for Richacls, those tools will automatically
have at least basic Richacl support (for backup/restore and similar
purposes, for example). In the kernel, supporting a new set of xattrs
is pretty cheap and easy. So I really don't see the point of inventing
yet another interface for basically the same type of information.

Regardless, this patch looks fine to me, assuming that we really do
want to do this xattrs:

Reviewed-by: Jeff Layton <redacted>

Thanks,
Andreas