From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752167AbcGNUd2 (ORCPT ); Thu, 14 Jul 2016 16:33:28 -0400 Received: from mail-vk0-f54.google.com ([209.85.213.54]:35086 "EHLO mail-vk0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751734AbcGNUdU (ORCPT ); Thu, 14 Jul 2016 16:33:20 -0400 MIME-Version: 1.0 In-Reply-To: <1468324965.7798.17.camel@redhat.com> References: <1467294433-3222-1-git-send-email-agruenba@redhat.com> <1467294433-3222-19-git-send-email-agruenba@redhat.com> <1468324965.7798.17.camel@redhat.com> From: Andreas Gruenbacher Date: Thu, 14 Jul 2016 22:33:18 +0200 Message-ID: Subject: Re: [PATCH v23 18/22] richacl: xattr mapping functions To: Jeff Layton Cc: Alexander Viro , Christoph Hellwig , "Theodore Ts'o" , Andreas Dilger , "J. Bruce Fields" , Trond Myklebust , Anna Schumaker , Dave Chinner , linux-ext4 , XFS Developers , LKML , linux-fsdevel , Linux NFS Mailing List , linux-cifs@vger.kernel.org, Linux API Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jul 12, 2016 at 2:02 PM, Jeff Layton 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 >> --- >> 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 >> + * >> + * 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 >> + * >> + * 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 >> + * >> + * 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 Thanks, Andreas