xfs
[Top] [All Lists]

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

To: Andreas Gruenbacher <agruenba@xxxxxxxxxx>, Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH v23 18/22] richacl: xattr mapping functions
From: Jeff Layton <jlayton@xxxxxxxxxx>
Date: Tue, 12 Jul 2016 08:02:45 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Theodore Ts'o <tytso@xxxxxxx>, Andreas Dilger <adilger.kernel@xxxxxxxxx>, "J. Bruce Fields" <bfields@xxxxxxxxxxxx>, Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>, Anna Schumaker <anna.schumaker@xxxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx>, linux-ext4@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, linux-fsdevel@xxxxxxxxxxxxxxx, linux-nfs@xxxxxxxxxxxxxxx, linux-cifs@xxxxxxxxxxxxxxx, linux-api@xxxxxxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1467294433-3222-19-git-send-email-agruenba@xxxxxxxxxx>
References: <1467294433-3222-1-git-send-email-agruenba@xxxxxxxxxx> <1467294433-3222-19-git-send-email-agruenba@xxxxxxxxxx>
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
> Map between "system.richacl" xattrs and the in-kernel representation.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@xxxxxxxxxx>
> ---
> Â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@xxxxxxxxxx>
> + *
> + * 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@xxxxxxxxxx>
> + *
> + * 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@xxxxxxxxxx>
> + *
> + * 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?

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

Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>

<Prev in Thread] Current Thread [Next in Thread>