| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAHc6FU4OxssRFVNwrQ+x1-C-phNF+8wgty0iZqeOYQDfTy6AyA@mail.gmail.com>
Date: Thu, 14 Jul 2016 22:33:18 +0200
From: Andreas Gruenbacher <agruenba@...hat.com>
To: Jeff Layton <jlayton@...hat.com>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,
Christoph Hellwig <hch@...radead.org>,
"Theodore Ts'o" <tytso@....edu>,
Andreas Dilger <adilger.kernel@...ger.ca>,
"J. Bruce Fields" <bfields@...ldses.org>,
Trond Myklebust <trond.myklebust@...marydata.com>,
Anna Schumaker <anna.schumaker@...app.com>,
Dave Chinner <david@...morbit.com>,
linux-ext4 <linux-ext4@...r.kernel.org>,
XFS Developers <xfs@....sgi.com>,
LKML <linux-kernel@...r.kernel.org>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>,
Linux NFS Mailing List <linux-nfs@...r.kernel.org>,
linux-cifs@...r.kernel.org, Linux API <linux-api@...r.kernel.org>
Subject: Re: [PATCH v23 18/22] richacl: xattr mapping functions
On Tue, Jul 12, 2016 at 2:02 PM, Jeff Layton <jlayton@...hat.com> wrote:
> 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@...hat.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@...hat.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@...hat.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@...hat.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 <jlayton@...hat.com>
Thanks,
Andreas
Powered by blists - more mailing lists