Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1434569 > unrolled thread

[PATCH v23 00/22] Richacls (Core and Ext4)

Started byAndreas Gruenbacher <agruenba@redhat.com>
First post2016-06-30 16:00 +0200
Last post2016-06-30 17:00 +0200
Articles 8 on this page of 28 — 5 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v23 00/22] Richacls (Core and Ext4) Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
    [PATCH v23 14/22] richacl: Update the file masks in chmod() Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
    [PATCH v23 15/22] richacl: Check if an acl is equivalent to a file mode Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
    [PATCH v23 08/22] richacl: Compute maximum file masks from an acl Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
      RE: [PATCH v23 08/22] richacl: Compute maximum file masks from an acl "Frank Filz" <ffilzlnx@mindspring.com> - 2016-07-05 19:10 +0200
    [PATCH v23 16/22] richacl: Create-time inheritance Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
    [PATCH v23 06/22] richacl: In-memory representation and helper functions Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
      Re: [PATCH v23 06/22] richacl: In-memory representation and helper  functions Jeff Layton <jlayton@redhat.com> - 2016-07-05 13:40 +0200
    [PATCH v23 22/22] ext4: Add richacl feature flag Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
    [PATCH v23 21/22] ext4: Add richacl support Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
    [PATCH v23 19/22] richacl: Add richacl xattr handler Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
    [PATCH v23 12/22] vfs: Add get_richacl and set_richacl inode operations Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
      Re: [PATCH v23 12/22] vfs: Add get_richacl and set_richacl inode  operations Jeff Layton <jlayton@redhat.com> - 2016-07-06 20:40 +0200
    [PATCH v23 09/22] richacl: Permission check algorithm Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
      Re: [PATCH v23 09/22] richacl: Permission check algorithm Jeff Layton <jlayton@redhat.com> - 2016-07-05 17:00 +0200
    [PATCH v23 17/22] richacl: Automatic Inheritance Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
    [PATCH v23 20/22] vfs: Add richacl permission checking Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
    [PATCH v23 13/22] vfs: Cache richacl in struct inode Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
      Re: [PATCH v23 13/22] vfs: Cache richacl in struct inode Jeff Layton <jlayton@redhat.com> - 2016-07-06 21:00 +0200
        Re: [PATCH v23 13/22] vfs: Cache richacl in struct inode David Howells <dhowells@redhat.com> - 2016-07-07 16:20 +0200
    [PATCH v23 03/22] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
      Re: [PATCH v23 03/22] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD  permission flags Jeff Layton <jlayton@redhat.com> - 2016-07-05 13:10 +0200
    [PATCH v23 18/22] richacl: xattr mapping functions Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
    [PATCH v23 11/22] vfs: Cache base_acl objects in inodes Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:00 +0200
      Re: [PATCH v23 11/22] vfs: Cache base_acl objects in inodes Jeff Layton <jlayton@redhat.com> - 2016-07-05 18:00 +0200
    [PATCH v23 02/22] vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR permission flags Andreas Gruenbacher <agruenba@redhat.com> - 2016-06-30 16:10 +0200
      Re: [PATCH v23 02/22] vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR  permission flags Jeff Layton <jlayton@redhat.com> - 2016-07-05 13:20 +0200
    Re: [PATCH v23 00/22] Richacls (Core and Ext4) Volker Lendecke <Volker.Lendecke@SerNet.DE> - 2016-06-30 17:00 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1434588 — [PATCH v23 03/22] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags

FromAndreas Gruenbacher <agruenba@redhat.com>
Date2016-06-30 16:00 +0200
Subject[PATCH v23 03/22] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags
Message-ID<rPKFB-1Pr-83@gated-at.bofh.it>
In reply to#1434569
Normally, deleting a file requires MAY_WRITE access to the parent
directory.  With richacls, a file may be deleted with MAY_DELETE_CHILD access
to the parent directory or with MAY_DELETE_SELF access to the file.

To support that, pass the MAY_DELETE_CHILD mask flag to inode_permission()
when checking for delete access inside a directory, and MAY_DELETE_SELF
when checking for delete access to a file itelf.

The MAY_DELETE_SELF permission overrides the sticky directory check.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <bfields@redhat.com>
Reviewed-by: Steve French <steve.french@primarydata.com>
---
 fs/namei.c         | 20 ++++++++++++--------
 include/linux/fs.h |  2 ++
 2 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index dc91858..663933e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -454,9 +454,9 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
  * this, letting us set arbitrary permissions for filesystem access without
  * changing the "normal" UIDs which are used for other things.
  *
- * MAY_WRITE must be set in @mask whenever MAY_APPEND, MAY_CREATE_FILE, or
- * MAY_CREATE_DIR are set.  That way, file systems that don't support these
- * permissions will check for MAY_WRITE instead.
+ * MAY_WRITE must be set in @mask whenever MAY_APPEND, MAY_CREATE_FILE,
+ * MAY_CREATE_DIR, or MAY_DELETE_CHILD are set.  That way, file systems that
+ * don't support these permissions will check for MAY_WRITE instead.
  */
 int inode_permission(struct inode *inode, int mask)
 {
@@ -2778,14 +2778,18 @@ static int may_delete_or_replace(struct inode *dir, struct dentry *victim,
 	BUG_ON(victim->d_parent->d_inode != dir);
 	audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
 
-	error = inode_permission(dir, mask);
+	error = inode_permission(dir, mask | MAY_WRITE | MAY_DELETE_CHILD);
+	if (!error && check_sticky(dir, inode))
+		error = -EPERM;
+	if (error && IS_RICHACL(inode) &&
+	    inode_permission(inode, MAY_DELETE_SELF) == 0 &&
+	    inode_permission(dir, mask) == 0)
+		error = 0;
 	if (error)
 		return error;
 	if (IS_APPEND(dir))
 		return -EPERM;
-
-	if (check_sticky(dir, inode) || IS_APPEND(inode) ||
-	    IS_IMMUTABLE(inode) || IS_SWAPFILE(inode))
+	if (IS_APPEND(inode) || IS_IMMUTABLE(inode) || IS_SWAPFILE(inode))
 		return -EPERM;
 	if (isdir) {
 		if (!d_is_dir(victim))
@@ -2803,7 +2807,7 @@ static int may_delete_or_replace(struct inode *dir, struct dentry *victim,
 
 static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
 {
-	return may_delete_or_replace(dir, victim, isdir, MAY_WRITE | MAY_EXEC);
+	return may_delete_or_replace(dir, victim, isdir, MAY_EXEC);
 }
 
 static int may_replace(struct inode *dir, struct dentry *victim, bool isdir)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index dd614ad..86bfa10 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -86,6 +86,8 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 #define MAY_NOT_BLOCK		0x00000080
 #define MAY_CREATE_FILE		0x00000100
 #define MAY_CREATE_DIR		0x00000200
+#define MAY_DELETE_CHILD	0x00000400
+#define MAY_DELETE_SELF		0x00000800
 
 /*
  * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1436925 — Re: [PATCH v23 03/22] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags

FromJeff Layton <jlayton@redhat.com>
Date2016-07-05 13:10 +0200
SubjectRe: [PATCH v23 03/22] vfs: Add MAY_DELETE_SELF and MAY_DELETE_CHILD permission flags
Message-ID<rRwoN-21i-13@gated-at.bofh.it>
In reply to#1434588
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
> Normally, deleting a file requires MAY_WRITE access to the parent
> directory.  With richacls, a file may be deleted with MAY_DELETE_CHILD access
> to the parent directory or with MAY_DELETE_SELF access to the file.
> 
> To support that, pass the MAY_DELETE_CHILD mask flag to inode_permission()
> when checking for delete access inside a directory, and MAY_DELETE_SELF
> when checking for delete access to a file itelf.
> 

Minor misspelling in changelog above. It should be "itself".

> The MAY_DELETE_SELF permission overrides the sticky directory check.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> Reviewed-by: J. Bruce Fields <bfields@redhat.com>
> Reviewed-by: Steve French <steve.french@primarydata.com>
> ---
>  fs/namei.c         | 20 ++++++++++++--------
>  include/linux/fs.h |  2 ++
>  2 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index dc91858..663933e 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -454,9 +454,9 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
>   * this, letting us set arbitrary permissions for filesystem access without
>   * changing the "normal" UIDs which are used for other things.
>   *
> - * MAY_WRITE must be set in @mask whenever MAY_APPEND, MAY_CREATE_FILE, or
> - * MAY_CREATE_DIR are set.  That way, file systems that don't support these
> - * permissions will check for MAY_WRITE instead.
> + * MAY_WRITE must be set in @mask whenever MAY_APPEND, MAY_CREATE_FILE,
> + * MAY_CREATE_DIR, or MAY_DELETE_CHILD are set.  That way, file systems that
> + * don't support these permissions will check for MAY_WRITE instead.
>   */
>  int inode_permission(struct inode *inode, int mask)
>  {
> @@ -2778,14 +2778,18 @@ static int may_delete_or_replace(struct inode *dir, struct dentry *victim,
>  	BUG_ON(victim->d_parent->d_inode != dir);
>  	audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
>  
> -	error = inode_permission(dir, mask);
> +	error = inode_permission(dir, mask | MAY_WRITE | MAY_DELETE_CHILD);
> +	if (!error && check_sticky(dir, inode))
> +		error = -EPERM;
> +	if (error && IS_RICHACL(inode) &&
> +	    inode_permission(inode, MAY_DELETE_SELF) == 0 &&
> +	    inode_permission(dir, mask) == 0)
> +		error = 0;
>  	if (error)
>  		return error;
>  	if (IS_APPEND(dir))
>  		return -EPERM;
> -
> -	if (check_sticky(dir, inode) || IS_APPEND(inode) ||
> -	    IS_IMMUTABLE(inode) || IS_SWAPFILE(inode))
> +	if (IS_APPEND(inode) || IS_IMMUTABLE(inode) || IS_SWAPFILE(inode))
>  		return -EPERM;
>  	if (isdir) {
>  		if (!d_is_dir(victim))
> @@ -2803,7 +2807,7 @@ static int may_delete_or_replace(struct inode *dir, struct dentry *victim,
>  
>  static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
>  {
> -	return may_delete_or_replace(dir, victim, isdir, MAY_WRITE | MAY_EXEC);
> +	return may_delete_or_replace(dir, victim, isdir, MAY_EXEC);
>  }
>  
>  static int may_replace(struct inode *dir, struct dentry *victim, bool isdir)
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index dd614ad..86bfa10 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -86,6 +86,8 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
>  #define MAY_NOT_BLOCK		0x00000080
>  #define MAY_CREATE_FILE		0x00000100
>  #define MAY_CREATE_DIR		0x00000200
> +#define MAY_DELETE_CHILD	0x00000400
> +#define MAY_DELETE_SELF		0x00000800
>  
>  /*
>   * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond

Reviewed-by: Jeff Layton <jlayton@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1434590 — [PATCH v23 18/22] richacl: xattr mapping functions

FromAndreas Gruenbacher <agruenba@redhat.com>
Date2016-06-30 16:00 +0200
Subject[PATCH v23 18/22] richacl: xattr mapping functions
Message-ID<rPKFB-1Pr-75@gated-at.bofh.it>
In reply to#1434569
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 <linux/sched.h>
+#include <linux/fs.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/richacl_xattr.h>
+
+/**
+ * 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 <uapi/linux/richacl_xattr.h>
+#include <linux/richacl.h>
+
+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 <linux/types.h>
+#include <linux/limits.h>
+
+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 */
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1434592 — [PATCH v23 11/22] vfs: Cache base_acl objects in inodes

FromAndreas Gruenbacher <agruenba@redhat.com>
Date2016-06-30 16:00 +0200
Subject[PATCH v23 11/22] vfs: Cache base_acl objects in inodes
Message-ID<rPKFB-1Pr-81@gated-at.bofh.it>
In reply to#1434569
POSIX ACLs and richacls are both objects allocated by kmalloc() with a
reference count which are freed by kfree_rcu().  An inode can either
cache an access and a default POSIX ACL, or a richacl (richacls do not
have default acls).  To allow an inode to cache either of the two kinds
of acls, introduce a new base_acl type and convert i_acl and
i_default_acl to that type. In most cases, the vfs then doesn't care which
kind of acl an inode caches (if any).

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Cc: Andreas Dilger <adilger@dilger.ca>
---
 drivers/staging/lustre/lustre/llite/llite_lib.c |  2 +-
 fs/9p/acl.c                                     |  8 +--
 fs/f2fs/acl.c                                   |  4 +-
 fs/inode.c                                      | 32 +++++++++++-
 fs/jffs2/acl.c                                  |  6 ++-
 fs/namei.c                                      | 33 ++++++------
 fs/nfs/nfs3acl.c                                | 14 ++---
 fs/posix_acl.c                                  | 69 +++++++------------------
 fs/richacl.c                                    |  4 +-
 include/linux/fs.h                              | 41 +++++++++++++--
 include/linux/posix_acl.h                       | 21 ++++----
 include/linux/richacl.h                         |  9 ++--
 12 files changed, 139 insertions(+), 104 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
index 96c7e9f..7819a7c 100644
--- a/drivers/staging/lustre/lustre/llite/llite_lib.c
+++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
@@ -1087,7 +1087,7 @@ void ll_clear_inode(struct inode *inode)
 	}
 #ifdef CONFIG_FS_POSIX_ACL
 	else if (lli->lli_posix_acl) {
-		LASSERT(atomic_read(&lli->lli_posix_acl->a_refcount) == 1);
+		LASSERT(base_acl_refcount(&lli->lli_posix_acl->a_base) == 1);
 		LASSERT(!lli->lli_remote_perms);
 		posix_acl_release(lli->lli_posix_acl);
 		lli->lli_posix_acl = NULL;
diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index 0576eae..1ce572f 100644
--- a/fs/9p/acl.c
+++ b/fs/9p/acl.c
@@ -87,14 +87,14 @@ int v9fs_get_acl(struct inode *inode, struct p9_fid *fid)
 
 static struct posix_acl *v9fs_get_cached_acl(struct inode *inode, int type)
 {
-	struct posix_acl *acl;
+	struct base_acl *base_acl;
 	/*
 	 * 9p Always cache the acl value when
 	 * instantiating the inode (v9fs_inode_from_fid)
 	 */
-	acl = get_cached_acl(inode, type);
-	BUG_ON(is_uncached_acl(acl));
-	return acl;
+	base_acl = get_cached_acl(inode, type);
+	BUG_ON(is_uncached_acl(base_acl));
+	return posix_acl(base_acl);
 }
 
 struct posix_acl *v9fs_iop_get_acl(struct inode *inode, int type)
diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
index a31c7e8..6079017 100644
--- a/fs/f2fs/acl.c
+++ b/fs/f2fs/acl.c
@@ -267,7 +267,7 @@ static struct posix_acl *f2fs_acl_clone(const struct posix_acl *acl,
 				sizeof(struct posix_acl_entry);
 		clone = kmemdup(acl, size, flags);
 		if (clone)
-			atomic_set(&clone->a_refcount, 1);
+			base_acl_init(&clone->a_base);
 	}
 	return clone;
 }
@@ -279,7 +279,7 @@ static int f2fs_acl_create_masq(struct posix_acl *acl, umode_t *mode_p)
 	umode_t mode = *mode_p;
 	int not_equiv = 0;
 
-	/* assert(atomic_read(acl->a_refcount) == 1); */
+	/* assert(base_acl_refcount(&acl->a_base) == 1); */
 
 	FOREACH_ACL_ENTRY(pa, acl, pe) {
 		switch(pa->e_tag) {
diff --git a/fs/inode.c b/fs/inode.c
index 4ccbc21..40c03a7 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -240,14 +240,42 @@ void __destroy_inode(struct inode *inode)
 
 #ifdef CONFIG_FS_POSIX_ACL
 	if (inode->i_acl && !is_uncached_acl(inode->i_acl))
-		posix_acl_release(inode->i_acl);
+		base_acl_put(inode->i_acl);
 	if (inode->i_default_acl && !is_uncached_acl(inode->i_default_acl))
-		posix_acl_release(inode->i_default_acl);
+		base_acl_put(inode->i_default_acl);
 #endif
 	this_cpu_dec(nr_inodes);
 }
 EXPORT_SYMBOL(__destroy_inode);
 
+#ifdef CONFIG_FS_POSIX_ACL
+struct base_acl *__get_cached_acl(struct base_acl **p)
+{
+	struct base_acl *base_acl;
+
+	for (;;) {
+		rcu_read_lock();
+		base_acl = rcu_dereference(*p);
+		if (!base_acl || is_uncached_acl(base_acl) ||
+		    atomic_inc_not_zero(&base_acl->ba_refcount))
+			break;
+		rcu_read_unlock();
+		cpu_relax();
+	}
+	rcu_read_unlock();
+	return base_acl;
+}
+
+void __forget_cached_acl(struct base_acl **p)
+{
+	struct base_acl *old;
+
+	old = xchg(p, ACL_NOT_CACHED);
+	if (!is_uncached_acl(old))
+		base_acl_put(old);
+}
+#endif
+
 static void i_callback(struct rcu_head *head)
 {
 	struct inode *inode = container_of(head, struct inode, i_rcu);
diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c
index bc2693d..6c11909 100644
--- a/fs/jffs2/acl.c
+++ b/fs/jffs2/acl.c
@@ -292,13 +292,15 @@ int jffs2_init_acl_post(struct inode *inode)
 	int rc;
 
 	if (inode->i_default_acl) {
-		rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_DEFAULT, inode->i_default_acl);
+		rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_DEFAULT,
+				     posix_acl(inode->i_default_acl));
 		if (rc)
 			return rc;
 	}
 
 	if (inode->i_acl) {
-		rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_ACCESS, inode->i_acl);
+		rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_ACCESS,
+				     posix_acl(inode->i_acl));
 		if (rc)
 			return rc;
 	}
diff --git a/fs/namei.c b/fs/namei.c
index 663933e..7a822d0 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -259,25 +259,28 @@ void putname(struct filename *name)
 static int check_acl(struct inode *inode, int mask)
 {
 #ifdef CONFIG_FS_POSIX_ACL
-	struct posix_acl *acl;
-
 	if (mask & MAY_NOT_BLOCK) {
-		acl = get_cached_acl_rcu(inode, ACL_TYPE_ACCESS);
-	        if (!acl)
+		struct base_acl *base_acl;
+
+		base_acl = rcu_dereference(inode->i_acl);
+	        if (!base_acl)
 	                return -EAGAIN;
 		/* no ->get_acl() calls in RCU mode... */
-		if (is_uncached_acl(acl))
+		if (is_uncached_acl(base_acl))
 			return -ECHILD;
-	        return posix_acl_permission(inode, acl, mask & ~MAY_NOT_BLOCK);
-	}
-
-	acl = get_acl(inode, ACL_TYPE_ACCESS);
-	if (IS_ERR(acl))
-		return PTR_ERR(acl);
-	if (acl) {
-	        int error = posix_acl_permission(inode, acl, mask);
-	        posix_acl_release(acl);
-	        return error;
+	        return posix_acl_permission(inode, posix_acl(base_acl),
+					    mask & ~MAY_NOT_BLOCK);
+	} else {
+		struct posix_acl *acl;
+
+		acl = get_acl(inode, ACL_TYPE_ACCESS);
+		if (IS_ERR(acl))
+			return PTR_ERR(acl);
+		if (acl) {
+			int error = posix_acl_permission(inode, acl, mask);
+			posix_acl_release(acl);
+			return error;
+		}
 	}
 #endif
 
diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
index 720d92f5..2b70944 100644
--- a/fs/nfs/nfs3acl.c
+++ b/fs/nfs/nfs3acl.c
@@ -16,28 +16,28 @@
  * caching get_acl results in a race-free way.  See fs/posix_acl.c:get_acl()
  * for explanations.
  */
-static void nfs3_prepare_get_acl(struct posix_acl **p)
+static void nfs3_prepare_get_acl(struct base_acl **p)
 {
-	struct posix_acl *sentinel = uncached_acl_sentinel(current);
+	struct base_acl *sentinel = uncached_acl_sentinel(current);
 
 	if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED) {
 		/* Not the first reader or sentinel already in place. */
 	}
 }
 
-static void nfs3_complete_get_acl(struct posix_acl **p, struct posix_acl *acl)
+static void nfs3_complete_get_acl(struct base_acl **p, struct posix_acl *acl)
 {
-	struct posix_acl *sentinel = uncached_acl_sentinel(current);
+	struct base_acl *sentinel = uncached_acl_sentinel(current);
 
 	/* Only cache the ACL if our sentinel is still in place. */
 	posix_acl_dup(acl);
-	if (cmpxchg(p, sentinel, acl) != sentinel)
+	if (cmpxchg(p, sentinel, &acl->a_base) != sentinel)
 		posix_acl_release(acl);
 }
 
-static void nfs3_abort_get_acl(struct posix_acl **p)
+static void nfs3_abort_get_acl(struct base_acl **p)
 {
-	struct posix_acl *sentinel = uncached_acl_sentinel(current);
+	struct base_acl *sentinel = uncached_acl_sentinel(current);
 
 	/* Remove our sentinel upon failure. */
 	cmpxchg(p, sentinel, ACL_NOT_CACHED);
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
index edc452c..1b685a1 100644
--- a/fs/posix_acl.c
+++ b/fs/posix_acl.c
@@ -21,7 +21,7 @@
 #include <linux/export.h>
 #include <linux/user_namespace.h>
 
-static struct posix_acl **acl_by_type(struct inode *inode, int type)
+static inline struct base_acl **acl_by_type(struct inode *inode, int type)
 {
 	switch (type) {
 	case ACL_TYPE_ACCESS:
@@ -33,51 +33,23 @@ static struct posix_acl **acl_by_type(struct inode *inode, int type)
 	}
 }
 
-struct posix_acl *get_cached_acl(struct inode *inode, int type)
+struct base_acl *get_cached_acl(struct inode *inode, int type)
 {
-	struct posix_acl **p = acl_by_type(inode, type);
-	struct posix_acl *acl;
-
-	for (;;) {
-		rcu_read_lock();
-		acl = rcu_dereference(*p);
-		if (!acl || is_uncached_acl(acl) ||
-		    atomic_inc_not_zero(&acl->a_refcount))
-			break;
-		rcu_read_unlock();
-		cpu_relax();
-	}
-	rcu_read_unlock();
-	return acl;
+	return __get_cached_acl(acl_by_type(inode, type));
 }
 EXPORT_SYMBOL(get_cached_acl);
 
-struct posix_acl *get_cached_acl_rcu(struct inode *inode, int type)
-{
-	return rcu_dereference(*acl_by_type(inode, type));
-}
-EXPORT_SYMBOL(get_cached_acl_rcu);
-
 void set_cached_acl(struct inode *inode, int type, struct posix_acl *acl)
 {
-	struct posix_acl **p = acl_by_type(inode, type);
-	struct posix_acl *old;
+	struct base_acl **p = acl_by_type(inode, type);
+	struct base_acl *old;
 
-	old = xchg(p, posix_acl_dup(acl));
+	old = xchg(p, &posix_acl_dup(acl)->a_base);
 	if (!is_uncached_acl(old))
-		posix_acl_release(old);
+		base_acl_put(old);
 }
 EXPORT_SYMBOL(set_cached_acl);
 
-static void __forget_cached_acl(struct posix_acl **p)
-{
-	struct posix_acl *old;
-
-	old = xchg(p, ACL_NOT_CACHED);
-	if (!is_uncached_acl(old))
-		posix_acl_release(old);
-}
-
 void forget_cached_acl(struct inode *inode, int type)
 {
 	__forget_cached_acl(acl_by_type(inode, type));
@@ -93,25 +65,24 @@ EXPORT_SYMBOL(forget_all_cached_acls);
 
 struct posix_acl *get_acl(struct inode *inode, int type)
 {
-	void *sentinel;
-	struct posix_acl **p;
+	struct base_acl **p = acl_by_type(inode, type);
+	struct base_acl *sentinel, *base_acl;
 	struct posix_acl *acl;
 
+	if (!IS_POSIXACL(inode))
+		return NULL;
+
 	/*
 	 * The sentinel is used to detect when another operation like
 	 * set_cached_acl() or forget_cached_acl() races with get_acl().
 	 * It is guaranteed that is_uncached_acl(sentinel) is true.
 	 */
 
-	acl = get_cached_acl(inode, type);
-	if (!is_uncached_acl(acl))
-		return acl;
-
-	if (!IS_POSIXACL(inode))
-		return NULL;
+	base_acl = __get_cached_acl(p);
+	if (!is_uncached_acl(base_acl))
+		return posix_acl(base_acl);
 
 	sentinel = uncached_acl_sentinel(current);
-	p = acl_by_type(inode, type);
 
 	/*
 	 * If the ACL isn't being read yet, set our sentinel.  Otherwise, the
@@ -151,7 +122,7 @@ struct posix_acl *get_acl(struct inode *inode, int type)
 	 * Cache the result, but only if our sentinel is still in place.
 	 */
 	posix_acl_dup(acl);
-	if (unlikely(cmpxchg(p, sentinel, acl) != sentinel))
+	if (unlikely(cmpxchg(p, sentinel, &acl->a_base) != sentinel))
 		posix_acl_release(acl);
 	return acl;
 }
@@ -163,7 +134,7 @@ EXPORT_SYMBOL(get_acl);
 void
 posix_acl_init(struct posix_acl *acl, int count)
 {
-	atomic_set(&acl->a_refcount, 1);
+	base_acl_init(&acl->a_base);
 	acl->a_count = count;
 }
 EXPORT_SYMBOL(posix_acl_init);
@@ -196,7 +167,7 @@ posix_acl_clone(const struct posix_acl *acl, gfp_t flags)
 		           sizeof(struct posix_acl_entry);
 		clone = kmemdup(acl, size, flags);
 		if (clone)
-			atomic_set(&clone->a_refcount, 1);
+			base_acl_init(&clone->a_base);
 	}
 	return clone;
 }
@@ -418,7 +389,7 @@ static int posix_acl_create_masq(struct posix_acl *acl, umode_t *mode_p)
 	umode_t mode = *mode_p;
 	int not_equiv = 0;
 
-	/* assert(atomic_read(acl->a_refcount) == 1); */
+	/* assert(base_acl_refcount(&acl->a_base) == 1); */
 
 	FOREACH_ACL_ENTRY(pa, acl, pe) {
                 switch(pa->e_tag) {
@@ -473,7 +444,7 @@ static int __posix_acl_chmod_masq(struct posix_acl *acl, umode_t mode)
 	struct posix_acl_entry *group_obj = NULL, *mask_obj = NULL;
 	struct posix_acl_entry *pa, *pe;
 
-	/* assert(atomic_read(acl->a_refcount) == 1); */
+	/* assert(base_acl_refcount(&acl->a_base) == 1); */
 
 	FOREACH_ACL_ENTRY(pa, acl, pe) {
 		switch(pa->e_tag) {
diff --git a/fs/richacl.c b/fs/richacl.c
index cb0ef3f..8971ead 100644
--- a/fs/richacl.c
+++ b/fs/richacl.c
@@ -31,7 +31,7 @@ richacl_alloc(int count, gfp_t gfp)
 	struct richacl *acl = kzalloc(size, gfp);
 
 	if (acl) {
-		atomic_set(&acl->a_refcount, 1);
+		base_acl_init(&acl->a_base);
 		acl->a_count = count;
 	}
 	return acl;
@@ -50,7 +50,7 @@ richacl_clone(const struct richacl *acl, gfp_t gfp)
 
 	if (dup) {
 		memcpy(dup, acl, size);
-		atomic_set(&dup->a_refcount, 1);
+		base_acl_init(&dup->a_base);
 	}
 	return dup;
 }
diff --git a/include/linux/fs.h b/include/linux/fs.h
index bb36561..06a30b0 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -582,17 +582,23 @@ static inline void mapping_allow_writable(struct address_space *mapping)
 #define i_size_ordered_init(inode) do { } while (0)
 #endif
 
+struct base_acl {
+	union {
+		atomic_t ba_refcount;
+		struct rcu_head ba_rcu;
+	};
+};
 struct posix_acl;
 #define ACL_NOT_CACHED ((void *)(-1))
 
-static inline struct posix_acl *
+static inline struct base_acl *
 uncached_acl_sentinel(struct task_struct *task)
 {
 	return (void *)task + 1;
 }
 
 static inline bool
-is_uncached_acl(struct posix_acl *acl)
+is_uncached_acl(struct base_acl *acl)
 {
 	return (long)acl & 1;
 }
@@ -613,9 +619,9 @@ struct inode {
 	kgid_t			i_gid;
 	unsigned int		i_flags;
 
-#ifdef CONFIG_FS_POSIX_ACL
-	struct posix_acl	*i_acl;
-	struct posix_acl	*i_default_acl;
+#if defined(CONFIG_FS_POSIX_ACL)
+	struct base_acl		*i_acl;
+	struct base_acl		*i_default_acl;
 #endif
 
 	const struct inode_operations	*i_op;
@@ -3193,4 +3199,29 @@ static inline bool dir_relax_shared(struct inode *inode)
 extern bool path_noexec(const struct path *path);
 extern void inode_nohighmem(struct inode *inode);
 
+static inline void base_acl_get(struct base_acl *acl)
+{
+	if (acl)
+		atomic_inc(&acl->ba_refcount);
+}
+
+static inline void base_acl_put(struct base_acl *acl)
+{
+	if (acl && atomic_dec_and_test(&acl->ba_refcount))
+		kfree_rcu(acl, ba_rcu);
+}
+
+static inline void base_acl_init(struct base_acl *acl)
+{
+	atomic_set(&acl->ba_refcount, 1);
+}
+
+static inline int base_acl_refcount(struct base_acl *acl)
+{
+	return atomic_read(&acl->ba_refcount);
+}
+
+extern struct base_acl *__get_cached_acl(struct base_acl **);
+extern void __forget_cached_acl(struct base_acl **);
+
 #endif /* _LINUX_FS_H */
diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
index 5b5a80c..daf84fa 100644
--- a/include/linux/posix_acl.h
+++ b/include/linux/posix_acl.h
@@ -43,10 +43,7 @@ struct posix_acl_entry {
 };
 
 struct posix_acl {
-	union {
-		atomic_t		a_refcount;
-		struct rcu_head		a_rcu;
-	};
+	struct base_acl		a_base;  /* must be first, see posix_acl_release() */
 	unsigned int		a_count;
 	struct posix_acl_entry	a_entries[0];
 };
@@ -61,8 +58,7 @@ struct posix_acl {
 static inline struct posix_acl *
 posix_acl_dup(struct posix_acl *acl)
 {
-	if (acl)
-		atomic_inc(&acl->a_refcount);
+	base_acl_get(&acl->a_base);
 	return acl;
 }
 
@@ -72,10 +68,16 @@ posix_acl_dup(struct posix_acl *acl)
 static inline void
 posix_acl_release(struct posix_acl *acl)
 {
-	if (acl && atomic_dec_and_test(&acl->a_refcount))
-		kfree_rcu(acl, a_rcu);
+	BUILD_BUG_ON(offsetof(struct posix_acl, a_base) != 0);
+	base_acl_put(&acl->a_base);
 }
 
+static inline struct posix_acl *
+posix_acl(struct base_acl *base_acl)
+{
+	BUILD_BUG_ON(offsetof(struct posix_acl, a_base) != 0);
+	return container_of(base_acl, struct posix_acl, a_base);
+}
 
 /* posix_acl.c */
 
@@ -99,8 +101,7 @@ extern int posix_acl_create(struct inode *, umode_t *, struct posix_acl **,
 extern int simple_set_acl(struct inode *, struct posix_acl *, int);
 extern int simple_acl_create(struct inode *, struct inode *);
 
-struct posix_acl *get_cached_acl(struct inode *inode, int type);
-struct posix_acl *get_cached_acl_rcu(struct inode *inode, int type);
+struct base_acl *get_cached_acl(struct inode *inode, int type);
 void set_cached_acl(struct inode *inode, int type, struct posix_acl *acl);
 void forget_cached_acl(struct inode *inode, int type);
 void forget_all_cached_acls(struct inode *inode);
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index be9fb65..35a5bcb 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -31,7 +31,7 @@ struct richace {
 };
 
 struct richacl {
-	atomic_t	a_refcount;
+	struct base_acl	a_base;  /* must be first, see richacl_put() */
 	unsigned int	a_owner_mask;
 	unsigned int	a_group_mask;
 	unsigned int	a_other_mask;
@@ -56,8 +56,7 @@ struct richacl {
 static inline struct richacl *
 richacl_get(struct richacl *acl)
 {
-	if (acl)
-		atomic_inc(&acl->a_refcount);
+	base_acl_get(&acl->a_base);
 	return acl;
 }
 
@@ -67,8 +66,8 @@ richacl_get(struct richacl *acl)
 static inline void
 richacl_put(struct richacl *acl)
 {
-	if (acl && atomic_dec_and_test(&acl->a_refcount))
-		kfree(acl);
+	BUILD_BUG_ON(offsetof(struct richacl, a_base) != 0);
+	base_acl_put(&acl->a_base);
 }
 
 /**
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437149 — Re: [PATCH v23 11/22] vfs: Cache base_acl objects in inodes

FromJeff Layton <jlayton@redhat.com>
Date2016-07-05 18:00 +0200
SubjectRe: [PATCH v23 11/22] vfs: Cache base_acl objects in inodes
Message-ID<rRAVt-4Ms-41@gated-at.bofh.it>
In reply to#1434592
On Thu, 2016-06-30 at 15:47 +0200, Andreas Gruenbacher wrote:
> POSIX ACLs and richacls are both objects allocated by kmalloc() with a
> reference count which are freed by kfree_rcu().  An inode can either
> cache an access and a default POSIX ACL, or a richacl (richacls do not
> have default acls).  To allow an inode to cache either of the two kinds
> of acls, introduce a new base_acl type and convert i_acl and
> i_default_acl to that type. In most cases, the vfs then doesn't care which
> kind of acl an inode caches (if any).
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> Cc: Andreas Dilger <adilger@dilger.ca>
> ---
>  drivers/staging/lustre/lustre/llite/llite_lib.c |  2 +-
>  fs/9p/acl.c                                     |  8 +--
>  fs/f2fs/acl.c                                   |  4 +-
>  fs/inode.c                                      | 32 +++++++++++-
>  fs/jffs2/acl.c                                  |  6 ++-
>  fs/namei.c                                      | 33 ++++++------
>  fs/nfs/nfs3acl.c                                | 14 ++---
>  fs/posix_acl.c                                  | 69 +++++++------------------
>  fs/richacl.c                                    |  4 +-
>  include/linux/fs.h                              | 41 +++++++++++++--
>  include/linux/posix_acl.h                       | 21 ++++----
>  include/linux/richacl.h                         |  9 ++--
>  12 files changed, 139 insertions(+), 104 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/llite_lib.c b/drivers/staging/lustre/lustre/llite/llite_lib.c
> index 96c7e9f..7819a7c 100644
> --- a/drivers/staging/lustre/lustre/llite/llite_lib.c
> +++ b/drivers/staging/lustre/lustre/llite/llite_lib.c
> @@ -1087,7 +1087,7 @@ void ll_clear_inode(struct inode *inode)
>  	}
>  #ifdef CONFIG_FS_POSIX_ACL
>  	else if (lli->lli_posix_acl) {
> -		LASSERT(atomic_read(&lli->lli_posix_acl->a_refcount) == 1);
> +		LASSERT(base_acl_refcount(&lli->lli_posix_acl->a_base) == 1);
>  		LASSERT(!lli->lli_remote_perms);
>  		posix_acl_release(lli->lli_posix_acl);
>  		lli->lli_posix_acl = NULL;
> diff --git a/fs/9p/acl.c b/fs/9p/acl.c
> index 0576eae..1ce572f 100644
> --- a/fs/9p/acl.c
> +++ b/fs/9p/acl.c
> @@ -87,14 +87,14 @@ int v9fs_get_acl(struct inode *inode, struct p9_fid *fid)
>  
>  static struct posix_acl *v9fs_get_cached_acl(struct inode *inode, int type)
>  {
> -	struct posix_acl *acl;
> +	struct base_acl *base_acl;
>  	/*
>  	 * 9p Always cache the acl value when
>  	 * instantiating the inode (v9fs_inode_from_fid)
>  	 */
> -	acl = get_cached_acl(inode, type);
> -	BUG_ON(is_uncached_acl(acl));
> -	return acl;
> +	base_acl = get_cached_acl(inode, type);
> +	BUG_ON(is_uncached_acl(base_acl));
> +	return posix_acl(base_acl);
>  }
>  
>  struct posix_acl *v9fs_iop_get_acl(struct inode *inode, int type)
> diff --git a/fs/f2fs/acl.c b/fs/f2fs/acl.c
> index a31c7e8..6079017 100644
> --- a/fs/f2fs/acl.c
> +++ b/fs/f2fs/acl.c
> @@ -267,7 +267,7 @@ static struct posix_acl *f2fs_acl_clone(const struct posix_acl *acl,
>  				sizeof(struct posix_acl_entry);
>  		clone = kmemdup(acl, size, flags);
>  		if (clone)
> -			atomic_set(&clone->a_refcount, 1);
> +			base_acl_init(&clone->a_base);
>  	}
>  	return clone;
>  }
> @@ -279,7 +279,7 @@ static int f2fs_acl_create_masq(struct posix_acl *acl, umode_t *mode_p)
>  	umode_t mode = *mode_p;
>  	int not_equiv = 0;
>  
> -	/* assert(atomic_read(acl->a_refcount) == 1); */
> +	/* assert(base_acl_refcount(&acl->a_base) == 1); */
>  
>  	FOREACH_ACL_ENTRY(pa, acl, pe) {
>  		switch(pa->e_tag) {
> diff --git a/fs/inode.c b/fs/inode.c
> index 4ccbc21..40c03a7 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -240,14 +240,42 @@ void __destroy_inode(struct inode *inode)
>  
>  #ifdef CONFIG_FS_POSIX_ACL
>  	if (inode->i_acl && !is_uncached_acl(inode->i_acl))
> -		posix_acl_release(inode->i_acl);
> +		base_acl_put(inode->i_acl);
>  	if (inode->i_default_acl && !is_uncached_acl(inode->i_default_acl))
> -		posix_acl_release(inode->i_default_acl);
> +		base_acl_put(inode->i_default_acl);
>  #endif
>  	this_cpu_dec(nr_inodes);
>  }
>  EXPORT_SYMBOL(__destroy_inode);
>  
> +#ifdef CONFIG_FS_POSIX_ACL
> +struct base_acl *__get_cached_acl(struct base_acl **p)
> +{
> +	struct base_acl *base_acl;
> +
> +	for (;;) {
> +		rcu_read_lock();
> +		base_acl = rcu_dereference(*p);
> +		if (!base_acl || is_uncached_acl(base_acl) ||
> +		    atomic_inc_not_zero(&base_acl->ba_refcount))
> +			break;
> +		rcu_read_unlock();
> +		cpu_relax();
> +	}
> +	rcu_read_unlock();
> +	return base_acl;
> +}
> +

I know this is basically copied from the existing get_cached_acl
function, but I'm a little uneasy with the above (and also with the
existing code that does the same thing).

The ba_refcount and ba_rcu are unioned. Once you've done the final
base_acl_put on the object, the you'll end up calling kfree_rcu which
is going to clobber the ba_refcount. How is it then safe to rely on it
still being zero for the atomic_inc_not_zero? ISTM that it would be
safer to make those separate fields and not union them.

...or am I missing something that prevents that?

> +void __forget_cached_acl(struct base_acl **p)
> +{
> +	struct base_acl *old;
> +
> +	old = xchg(p, ACL_NOT_CACHED);
> +	if (!is_uncached_acl(old))
> +		base_acl_put(old);
> +}
> +#endif
> +
>  static void i_callback(struct rcu_head *head)
>  {
>  	struct inode *inode = container_of(head, struct inode, i_rcu);
> diff --git a/fs/jffs2/acl.c b/fs/jffs2/acl.c
> index bc2693d..6c11909 100644
> --- a/fs/jffs2/acl.c
> +++ b/fs/jffs2/acl.c
> @@ -292,13 +292,15 @@ int jffs2_init_acl_post(struct inode *inode)
>  	int rc;
>  
>  	if (inode->i_default_acl) {
> -		rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_DEFAULT, inode->i_default_acl);
> +		rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_DEFAULT,
> +				     posix_acl(inode->i_default_acl));
>  		if (rc)
>  			return rc;
>  	}
>  
>  	if (inode->i_acl) {
> -		rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_ACCESS, inode->i_acl);
> +		rc = __jffs2_set_acl(inode, JFFS2_XPREFIX_ACL_ACCESS,
> +				     posix_acl(inode->i_acl));
>  		if (rc)
>  			return rc;
>  	}
> diff --git a/fs/namei.c b/fs/namei.c
> index 663933e..7a822d0 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -259,25 +259,28 @@ void putname(struct filename *name)
>  static int check_acl(struct inode *inode, int mask)
>  {
>  #ifdef CONFIG_FS_POSIX_ACL
> -	struct posix_acl *acl;
> -
>  	if (mask & MAY_NOT_BLOCK) {
> -		acl = get_cached_acl_rcu(inode, ACL_TYPE_ACCESS);
> -	        if (!acl)
> +		struct base_acl *base_acl;
> +
> +		base_acl = rcu_dereference(inode->i_acl);
> +	        if (!base_acl)
>  	                return -EAGAIN;
>  		/* no ->get_acl() calls in RCU mode... */
> -		if (is_uncached_acl(acl))
> +		if (is_uncached_acl(base_acl))
>  			return -ECHILD;
> -	        return posix_acl_permission(inode, acl, mask & ~MAY_NOT_BLOCK);
> -	}
> -
> -	acl = get_acl(inode, ACL_TYPE_ACCESS);
> -	if (IS_ERR(acl))
> -		return PTR_ERR(acl);
> -	if (acl) {
> -	        int error = posix_acl_permission(inode, acl, mask);
> -	        posix_acl_release(acl);
> -	        return error;
> +	        return posix_acl_permission(inode, posix_acl(base_acl),
> +					    mask & ~MAY_NOT_BLOCK);
> +	} else {
> +		struct posix_acl *acl;
> +
> +		acl = get_acl(inode, ACL_TYPE_ACCESS);
> +		if (IS_ERR(acl))
> +			return PTR_ERR(acl);
> +		if (acl) {
> +			int error = posix_acl_permission(inode, acl, mask);
> +			posix_acl_release(acl);
> +			return error;
> +		}
>  	}
>  #endif
>  
> diff --git a/fs/nfs/nfs3acl.c b/fs/nfs/nfs3acl.c
> index 720d92f5..2b70944 100644
> --- a/fs/nfs/nfs3acl.c
> +++ b/fs/nfs/nfs3acl.c
> @@ -16,28 +16,28 @@
>   * caching get_acl results in a race-free way.  See fs/posix_acl.c:get_acl()
>   * for explanations.
>   */
> -static void nfs3_prepare_get_acl(struct posix_acl **p)
> +static void nfs3_prepare_get_acl(struct base_acl **p)
>  {
> -	struct posix_acl *sentinel = uncached_acl_sentinel(current);
> +	struct base_acl *sentinel = uncached_acl_sentinel(current);
>  
>  	if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED) {
>  		/* Not the first reader or sentinel already in place. */
>  	}
>  }
>  
> -static void nfs3_complete_get_acl(struct posix_acl **p, struct posix_acl *acl)
> +static void nfs3_complete_get_acl(struct base_acl **p, struct posix_acl *acl)
>  {
> -	struct posix_acl *sentinel = uncached_acl_sentinel(current);
> +	struct base_acl *sentinel = uncached_acl_sentinel(current);
>  
>  	/* Only cache the ACL if our sentinel is still in place. */
>  	posix_acl_dup(acl);
> -	if (cmpxchg(p, sentinel, acl) != sentinel)
> +	if (cmpxchg(p, sentinel, &acl->a_base) != sentinel)
>  		posix_acl_release(acl);
>  }
>  
> -static void nfs3_abort_get_acl(struct posix_acl **p)
> +static void nfs3_abort_get_acl(struct base_acl **p)
>  {
> -	struct posix_acl *sentinel = uncached_acl_sentinel(current);
> +	struct base_acl *sentinel = uncached_acl_sentinel(current);
>  
>  	/* Remove our sentinel upon failure. */
>  	cmpxchg(p, sentinel, ACL_NOT_CACHED);
> diff --git a/fs/posix_acl.c b/fs/posix_acl.c
> index edc452c..1b685a1 100644
> --- a/fs/posix_acl.c
> +++ b/fs/posix_acl.c
> @@ -21,7 +21,7 @@
>  #include 
>  #include 
>  
> -static struct posix_acl **acl_by_type(struct inode *inode, int type)
> +static inline struct base_acl **acl_by_type(struct inode *inode, int type)
>  {
>  	switch (type) {
>  	case ACL_TYPE_ACCESS:
> @@ -33,51 +33,23 @@ static struct posix_acl **acl_by_type(struct inode *inode, int type)
>  	}
>  }
>  
> -struct posix_acl *get_cached_acl(struct inode *inode, int type)
> +struct base_acl *get_cached_acl(struct inode *inode, int type)
>  {
> -	struct posix_acl **p = acl_by_type(inode, type);
> -	struct posix_acl *acl;
> -
> -	for (;;) {
> -		rcu_read_lock();
> -		acl = rcu_dereference(*p);
> -		if (!acl || is_uncached_acl(acl) ||
> -		    atomic_inc_not_zero(&acl->a_refcount))
> -			break;
> -		rcu_read_unlock();
> -		cpu_relax();
> -	}
> -	rcu_read_unlock();
> -	return acl;
> +	return __get_cached_acl(acl_by_type(inode, type));
>  }
>  EXPORT_SYMBOL(get_cached_acl);
>  
> -struct posix_acl *get_cached_acl_rcu(struct inode *inode, int type)
> -{
> -	return rcu_dereference(*acl_by_type(inode, type));
> -}
> -EXPORT_SYMBOL(get_cached_acl_rcu);
> -
>  void set_cached_acl(struct inode *inode, int type, struct posix_acl *acl)
>  {
> -	struct posix_acl **p = acl_by_type(inode, type);
> -	struct posix_acl *old;
> +	struct base_acl **p = acl_by_type(inode, type);
> +	struct base_acl *old;
>  
> -	old = xchg(p, posix_acl_dup(acl));
> +	old = xchg(p, &posix_acl_dup(acl)->a_base);
>  	if (!is_uncached_acl(old))
> -		posix_acl_release(old);
> +		base_acl_put(old);
>  }
>  EXPORT_SYMBOL(set_cached_acl);
>  
> -static void __forget_cached_acl(struct posix_acl **p)
> -{
> -	struct posix_acl *old;
> -
> -	old = xchg(p, ACL_NOT_CACHED);
> -	if (!is_uncached_acl(old))
> -		posix_acl_release(old);
> -}
> -
>  void forget_cached_acl(struct inode *inode, int type)
>  {
>  	__forget_cached_acl(acl_by_type(inode, type));
> @@ -93,25 +65,24 @@ EXPORT_SYMBOL(forget_all_cached_acls);
>  
>  struct posix_acl *get_acl(struct inode *inode, int type)
>  {
> -	void *sentinel;
> -	struct posix_acl **p;
> +	struct base_acl **p = acl_by_type(inode, type);
> +	struct base_acl *sentinel, *base_acl;
>  	struct posix_acl *acl;
>  
> +	if (!IS_POSIXACL(inode))
> +		return NULL;
> +
>  	/*
>  	 * The sentinel is used to detect when another operation like
>  	 * set_cached_acl() or forget_cached_acl() races with get_acl().
>  	 * It is guaranteed that is_uncached_acl(sentinel) is true.
>  	 */
>  
> -	acl = get_cached_acl(inode, type);
> -	if (!is_uncached_acl(acl))
> -		return acl;
> -
> -	if (!IS_POSIXACL(inode))
> -		return NULL;
> +	base_acl = __get_cached_acl(p);
> +	if (!is_uncached_acl(base_acl))
> +		return posix_acl(base_acl);
>  
>  	sentinel = uncached_acl_sentinel(current);
> -	p = acl_by_type(inode, type);
>  
>  	/*
>  	 * If the ACL isn't being read yet, set our sentinel.  Otherwise, the
> @@ -151,7 +122,7 @@ struct posix_acl *get_acl(struct inode *inode, int type)
>  	 * Cache the result, but only if our sentinel is still in place.
>  	 */
>  	posix_acl_dup(acl);
> -	if (unlikely(cmpxchg(p, sentinel, acl) != sentinel))
> +	if (unlikely(cmpxchg(p, sentinel, &acl->a_base) != sentinel))
>  		posix_acl_release(acl);
>  	return acl;
>  }
> @@ -163,7 +134,7 @@ EXPORT_SYMBOL(get_acl);
>  void
>  posix_acl_init(struct posix_acl *acl, int count)
>  {
> -	atomic_set(&acl->a_refcount, 1);
> +	base_acl_init(&acl->a_base);
>  	acl->a_count = count;
>  }
>  EXPORT_SYMBOL(posix_acl_init);
> @@ -196,7 +167,7 @@ posix_acl_clone(const struct posix_acl *acl, gfp_t flags)
>  		           sizeof(struct posix_acl_entry);
>  		clone = kmemdup(acl, size, flags);
>  		if (clone)
> -			atomic_set(&clone->a_refcount, 1);
> +			base_acl_init(&clone->a_base);
>  	}
>  	return clone;
>  }
> @@ -418,7 +389,7 @@ static int posix_acl_create_masq(struct posix_acl *acl, umode_t *mode_p)
>  	umode_t mode = *mode_p;
>  	int not_equiv = 0;
>  
> -	/* assert(atomic_read(acl->a_refcount) == 1); */
> +	/* assert(base_acl_refcount(&acl->a_base) == 1); */
>  
>  	FOREACH_ACL_ENTRY(pa, acl, pe) {
>                  switch(pa->e_tag) {
> @@ -473,7 +444,7 @@ static int __posix_acl_chmod_masq(struct posix_acl *acl, umode_t mode)
>  	struct posix_acl_entry *group_obj = NULL, *mask_obj = NULL;
>  	struct posix_acl_entry *pa, *pe;
>  
> -	/* assert(atomic_read(acl->a_refcount) == 1); */
> +	/* assert(base_acl_refcount(&acl->a_base) == 1); */
>  
>  	FOREACH_ACL_ENTRY(pa, acl, pe) {
>  		switch(pa->e_tag) {
> diff --git a/fs/richacl.c b/fs/richacl.c
> index cb0ef3f..8971ead 100644
> --- a/fs/richacl.c
> +++ b/fs/richacl.c
> @@ -31,7 +31,7 @@ richacl_alloc(int count, gfp_t gfp)
>  	struct richacl *acl = kzalloc(size, gfp);
>  
>  	if (acl) {
> -		atomic_set(&acl->a_refcount, 1);
> +		base_acl_init(&acl->a_base);
>  		acl->a_count = count;
>  	}
>  	return acl;
> @@ -50,7 +50,7 @@ richacl_clone(const struct richacl *acl, gfp_t gfp)
>  
>  	if (dup) {
>  		memcpy(dup, acl, size);
> -		atomic_set(&dup->a_refcount, 1);
> +		base_acl_init(&dup->a_base);
>  	}
>  	return dup;
>  }
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index bb36561..06a30b0 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -582,17 +582,23 @@ static inline void mapping_allow_writable(struct address_space *mapping)
>  #define i_size_ordered_init(inode) do { } while (0)
>  #endif
>  
> +struct base_acl {
> +	union {
> +		atomic_t ba_refcount;
> +		struct rcu_head ba_rcu;
> +	};
> +};
>  struct posix_acl;
>  #define ACL_NOT_CACHED ((void *)(-1))
>  
> -static inline struct posix_acl *
> +static inline struct base_acl *
>  uncached_acl_sentinel(struct task_struct *task)
>  {
>  	return (void *)task + 1;
>  }
>  
>  static inline bool
> -is_uncached_acl(struct posix_acl *acl)
> +is_uncached_acl(struct base_acl *acl)
>  {
>  	return (long)acl & 1;
>  }
> @@ -613,9 +619,9 @@ struct inode {
>  	kgid_t			i_gid;
>  	unsigned int		i_flags;
>  
> -#ifdef CONFIG_FS_POSIX_ACL
> -	struct posix_acl	*i_acl;
> -	struct posix_acl	*i_default_acl;
> +#if defined(CONFIG_FS_POSIX_ACL)
> +	struct base_acl		*i_acl;
> +	struct base_acl		*i_default_acl;
>  #endif
>  
>  	const struct inode_operations	*i_op;
> @@ -3193,4 +3199,29 @@ static inline bool dir_relax_shared(struct inode *inode)
>  extern bool path_noexec(const struct path *path);
>  extern void inode_nohighmem(struct inode *inode);
>  
> +static inline void base_acl_get(struct base_acl *acl)
> +{
> +	if (acl)
> +		atomic_inc(&acl->ba_refcount);
> +}
> +
> +static inline void base_acl_put(struct base_acl *acl)
> +{
> +	if (acl && atomic_dec_and_test(&acl->ba_refcount))
> +		kfree_rcu(acl, ba_rcu);
> +}
> +
> +static inline void base_acl_init(struct base_acl *acl)
> +{
> +	atomic_set(&acl->ba_refcount, 1);
> +}
> +
> +static inline int base_acl_refcount(struct base_acl *acl)
> +{
> +	return atomic_read(&acl->ba_refcount);
> +}
> +
> +extern struct base_acl *__get_cached_acl(struct base_acl **);
> +extern void __forget_cached_acl(struct base_acl **);
> +
>  #endif /* _LINUX_FS_H */
> diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h
> index 5b5a80c..daf84fa 100644
> --- a/include/linux/posix_acl.h
> +++ b/include/linux/posix_acl.h
> @@ -43,10 +43,7 @@ struct posix_acl_entry {
>  };
>  
>  struct posix_acl {
> -	union {
> -		atomic_t		a_refcount;
> -		struct rcu_head		a_rcu;
> -	};
> +	struct base_acl		a_base;  /* must be first, see posix_acl_release() */
>  	unsigned int		a_count;
>  	struct posix_acl_entry	a_entries[0];
>  };
> @@ -61,8 +58,7 @@ struct posix_acl {
>  static inline struct posix_acl *
>  posix_acl_dup(struct posix_acl *acl)
>  {
> -	if (acl)
> -		atomic_inc(&acl->a_refcount);
> +	base_acl_get(&acl->a_base);
>  	return acl;
>  }
>  
> @@ -72,10 +68,16 @@ posix_acl_dup(struct posix_acl *acl)
>  static inline void
>  posix_acl_release(struct posix_acl *acl)
>  {
> -	if (acl && atomic_dec_and_test(&acl->a_refcount))
> -		kfree_rcu(acl, a_rcu);
> +	BUILD_BUG_ON(offsetof(struct posix_acl, a_base) != 0);
> +	base_acl_put(&acl->a_base);
>  }
>  
> +static inline struct posix_acl *
> +posix_acl(struct base_acl *base_acl)
> +{
> +	BUILD_BUG_ON(offsetof(struct posix_acl, a_base) != 0);
> +	return container_of(base_acl, struct posix_acl, a_base);
> +}
>  
>  /* posix_acl.c */
>  
> @@ -99,8 +101,7 @@ extern int posix_acl_create(struct inode *, umode_t *, struct posix_acl **,
>  extern int simple_set_acl(struct inode *, struct posix_acl *, int);
>  extern int simple_acl_create(struct inode *, struct inode *);
>  
> -struct posix_acl *get_cached_acl(struct inode *inode, int type);
> -struct posix_acl *get_cached_acl_rcu(struct inode *inode, int type);
> +struct base_acl *get_cached_acl(struct inode *inode, int type);
>  void set_cached_acl(struct inode *inode, int type, struct posix_acl *acl);
>  void forget_cached_acl(struct inode *inode, int type);
>  void forget_all_cached_acls(struct inode *inode);
> diff --git a/include/linux/richacl.h b/include/linux/richacl.h
> index be9fb65..35a5bcb 100644
> --- a/include/linux/richacl.h
> +++ b/include/linux/richacl.h
> @@ -31,7 +31,7 @@ struct richace {
>  };
>  
>  struct richacl {
> -	atomic_t	a_refcount;
> +	struct base_acl	a_base;  /* must be first, see richacl_put() */
>  	unsigned int	a_owner_mask;
>  	unsigned int	a_group_mask;
>  	unsigned int	a_other_mask;
> @@ -56,8 +56,7 @@ struct richacl {
>  static inline struct richacl *
>  richacl_get(struct richacl *acl)
>  {
> -	if (acl)
> -		atomic_inc(&acl->a_refcount);
> +	base_acl_get(&acl->a_base);
>  	return acl;
>  }
>  
> @@ -67,8 +66,8 @@ richacl_get(struct richacl *acl)
>  static inline void
>  richacl_put(struct richacl *acl)
>  {
> -	if (acl && atomic_dec_and_test(&acl->a_refcount))
> -		kfree(acl);
> +	BUILD_BUG_ON(offsetof(struct richacl, a_base) != 0);
> +	base_acl_put(&acl->a_base);
>  }
>  
>  /**

--
Jeff Layton <jlayton@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1434594 — [PATCH v23 02/22] vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR permission flags

FromAndreas Gruenbacher <agruenba@redhat.com>
Date2016-06-30 16:10 +0200
Subject[PATCH v23 02/22] vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR permission flags
Message-ID<rPKPf-284-9@gated-at.bofh.it>
In reply to#1434569
Richacls distinguish between creating non-directories and directories. To
support that, add an isdir parameter to may_create(). When checking
inode_permission() for create permission, pass in an additional
MAY_CREATE_FILE or MAY_CREATE_DIR mask flag.

Add may_replace() to allow checking for delete and create access when
replacing an existing file in vfs_rename().

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
Reviewed-by: J. Bruce Fields <bfields@redhat.com>
Reviewed-by: Andreas Dilger <adilger@dilger.ca>
Reviewed-by: Steve French <steve.french@primarydata.com>
---
 fs/namei.c         | 49 +++++++++++++++++++++++++++++++++----------------
 include/linux/fs.h |  2 ++
 2 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 7cc5487..dc91858 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -454,7 +454,9 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
  * this, letting us set arbitrary permissions for filesystem access without
  * changing the "normal" UIDs which are used for other things.
  *
- * When checking for MAY_APPEND, MAY_WRITE must also be set in @mask.
+ * MAY_WRITE must be set in @mask whenever MAY_APPEND, MAY_CREATE_FILE, or
+ * MAY_CREATE_DIR are set.  That way, file systems that don't support these
+ * permissions will check for MAY_WRITE instead.
  */
 int inode_permission(struct inode *inode, int mask)
 {
@@ -2763,7 +2765,8 @@ EXPORT_SYMBOL(__check_sticky);
  * 10. We don't allow removal of NFS sillyrenamed files; it's handled by
  *     nfs_async_unlink().
  */
-static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
+static int may_delete_or_replace(struct inode *dir, struct dentry *victim,
+				 bool isdir, int mask)
 {
 	struct inode *inode = d_backing_inode(victim);
 	int error;
@@ -2775,7 +2778,7 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
 	BUG_ON(victim->d_parent->d_inode != dir);
 	audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
 
-	error = inode_permission(dir, MAY_WRITE | MAY_EXEC);
+	error = inode_permission(dir, mask);
 	if (error)
 		return error;
 	if (IS_APPEND(dir))
@@ -2798,6 +2801,18 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
 	return 0;
 }
 
+static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
+{
+	return may_delete_or_replace(dir, victim, isdir, MAY_WRITE | MAY_EXEC);
+}
+
+static int may_replace(struct inode *dir, struct dentry *victim, bool isdir)
+{
+	int mask = isdir ? MAY_CREATE_DIR : MAY_CREATE_FILE;
+
+	return may_delete_or_replace(dir, victim, isdir, mask | MAY_WRITE | MAY_EXEC);
+}
+
 /*	Check whether we can create an object with dentry child in directory
  *  dir.
  *  1. We can't do it if child already exists (open has special treatment for
@@ -2806,14 +2821,16 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
  *  3. We should have write and exec permissions on dir
  *  4. We can't do it if dir is immutable (done in permission())
  */
-static inline int may_create(struct inode *dir, struct dentry *child)
+static inline int may_create(struct inode *dir, struct dentry *child, bool isdir)
 {
+	int mask = isdir ? MAY_CREATE_DIR : MAY_CREATE_FILE;
+
 	audit_inode_child(dir, child, AUDIT_TYPE_CHILD_CREATE);
 	if (child->d_inode)
 		return -EEXIST;
 	if (IS_DEADDIR(dir))
 		return -ENOENT;
-	return inode_permission(dir, MAY_WRITE | MAY_EXEC);
+	return inode_permission(dir, MAY_WRITE | MAY_EXEC | mask);
 }
 
 /*
@@ -2863,7 +2880,7 @@ EXPORT_SYMBOL(unlock_rename);
 int vfs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 		bool want_excl)
 {
-	int error = may_create(dir, dentry);
+	int error = may_create(dir, dentry, false);
 	if (error)
 		return error;
 
@@ -3650,7 +3667,7 @@ EXPORT_SYMBOL(user_path_create);
 
 int vfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, dev_t dev)
 {
-	int error = may_create(dir, dentry);
+	int error = may_create(dir, dentry, false);
 
 	if (error)
 		return error;
@@ -3744,7 +3761,7 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
 
 int vfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 {
-	int error = may_create(dir, dentry);
+	int error = may_create(dir, dentry, true);
 	unsigned max_links = dir->i_sb->s_max_links;
 
 	if (error)
@@ -3800,7 +3817,7 @@ SYSCALL_DEFINE2(mkdir, const char __user *, pathname, umode_t, mode)
 
 int vfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
-	int error = may_delete(dir, dentry, 1);
+	int error = may_delete(dir, dentry, true);
 
 	if (error)
 		return error;
@@ -3922,7 +3939,7 @@ SYSCALL_DEFINE1(rmdir, const char __user *, pathname)
 int vfs_unlink(struct inode *dir, struct dentry *dentry, struct inode **delegated_inode)
 {
 	struct inode *target = dentry->d_inode;
-	int error = may_delete(dir, dentry, 0);
+	int error = may_delete(dir, dentry, false);
 
 	if (error)
 		return error;
@@ -4056,7 +4073,7 @@ SYSCALL_DEFINE1(unlink, const char __user *, pathname)
 
 int vfs_symlink(struct inode *dir, struct dentry *dentry, const char *oldname)
 {
-	int error = may_create(dir, dentry);
+	int error = may_create(dir, dentry, false);
 
 	if (error)
 		return error;
@@ -4139,7 +4156,7 @@ int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_de
 	if (!inode)
 		return -ENOENT;
 
-	error = may_create(dir, new_dentry);
+	error = may_create(dir, new_dentry, false);
 	if (error)
 		return error;
 
@@ -4336,14 +4353,14 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		return error;
 
 	if (!target) {
-		error = may_create(new_dir, new_dentry);
+		error = may_create(new_dir, new_dentry, is_dir);
 	} else {
 		new_is_dir = d_is_dir(new_dentry);
 
 		if (!(flags & RENAME_EXCHANGE))
-			error = may_delete(new_dir, new_dentry, is_dir);
+			error = may_replace(new_dir, new_dentry, is_dir);
 		else
-			error = may_delete(new_dir, new_dentry, new_is_dir);
+			error = may_replace(new_dir, new_dentry, new_is_dir);
 	}
 	if (error)
 		return error;
@@ -4606,7 +4623,7 @@ SYSCALL_DEFINE2(rename, const char __user *, oldname, const char __user *, newna
 
 int vfs_whiteout(struct inode *dir, struct dentry *dentry)
 {
-	int error = may_create(dir, dentry);
+	int error = may_create(dir, dentry, false);
 	if (error)
 		return error;
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4ad130c..dd614ad 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -84,6 +84,8 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 #define MAY_CHDIR		0x00000040
 /* called from RCU mode, don't block */
 #define MAY_NOT_BLOCK		0x00000080
+#define MAY_CREATE_FILE		0x00000100
+#define MAY_CREATE_DIR		0x00000200
 
 /*
  * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1436927 — Re: [PATCH v23 02/22] vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR permission flags

FromJeff Layton <jlayton@redhat.com>
Date2016-07-05 13:20 +0200
SubjectRe: [PATCH v23 02/22] vfs: Add MAY_CREATE_FILE and MAY_CREATE_DIR permission flags
Message-ID<rRwyt-24B-1@gated-at.bofh.it>
In reply to#1434594
On Thu, 2016-06-30 at 15:46 +0200, Andreas Gruenbacher wrote:
> Richacls distinguish between creating non-directories and directories. To
> support that, add an isdir parameter to may_create(). When checking
> inode_permission() for create permission, pass in an additional
> MAY_CREATE_FILE or MAY_CREATE_DIR mask flag.
> 
> Add may_replace() to allow checking for delete and create access when
> replacing an existing file in vfs_rename().
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> Reviewed-by: J. Bruce Fields <bfields@redhat.com>
> Reviewed-by: Andreas Dilger <adilger@dilger.ca>
> Reviewed-by: Steve French <steve.french@primarydata.com>
> ---
>  fs/namei.c         | 49 +++++++++++++++++++++++++++++++++----------------
>  include/linux/fs.h |  2 ++
>  2 files changed, 35 insertions(+), 16 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 7cc5487..dc91858 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -454,7 +454,9 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
>   * this, letting us set arbitrary permissions for filesystem access without
>   * changing the "normal" UIDs which are used for other things.
>   *
> - * When checking for MAY_APPEND, MAY_WRITE must also be set in @mask.
> + * MAY_WRITE must be set in @mask whenever MAY_APPEND, MAY_CREATE_FILE, or
> + * MAY_CREATE_DIR are set.  That way, file systems that don't support these
> + * permissions will check for MAY_WRITE instead.
>   */
>  int inode_permission(struct inode *inode, int mask)
>  {
> @@ -2763,7 +2765,8 @@ EXPORT_SYMBOL(__check_sticky);
>   * 10. We don't allow removal of NFS sillyrenamed files; it's handled by
>   *     nfs_async_unlink().
>   */
> -static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
> +static int may_delete_or_replace(struct inode *dir, struct dentry *victim,
> +				 bool isdir, int mask)
>  {
>  	struct inode *inode = d_backing_inode(victim);
>  	int error;
> @@ -2775,7 +2778,7 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
>  	BUG_ON(victim->d_parent->d_inode != dir);
>  	audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
>  
> -	error = inode_permission(dir, MAY_WRITE | MAY_EXEC);
> +	error = inode_permission(dir, mask);
>  	if (error)
>  		return error;
>  	if (IS_APPEND(dir))
> @@ -2798,6 +2801,18 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
>  	return 0;
>  }
>  
> +static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
> +{
> +	return may_delete_or_replace(dir, victim, isdir, MAY_WRITE | MAY_EXEC);
> +}
> +
> +static int may_replace(struct inode *dir, struct dentry *victim, bool isdir)
> +{
> +	int mask = isdir ? MAY_CREATE_DIR : MAY_CREATE_FILE;
> +
> +	return may_delete_or_replace(dir, victim, isdir, mask | MAY_WRITE | MAY_EXEC);
> +}
> +
>  /*	Check whether we can create an object with dentry child in directory
>   *  dir.
>   *  1. We can't do it if child already exists (open has special treatment for
> @@ -2806,14 +2821,16 @@ static int may_delete(struct inode *dir, struct dentry *victim, bool isdir)
>   *  3. We should have write and exec permissions on dir
>   *  4. We can't do it if dir is immutable (done in permission())
>   */
> -static inline int may_create(struct inode *dir, struct dentry *child)
> +static inline int may_create(struct inode *dir, struct dentry *child, bool isdir)
>  {
> +	int mask = isdir ? MAY_CREATE_DIR : MAY_CREATE_FILE;
> +
>  	audit_inode_child(dir, child, AUDIT_TYPE_CHILD_CREATE);
>  	if (child->d_inode)
>  		return -EEXIST;
>  	if (IS_DEADDIR(dir))
>  		return -ENOENT;
> -	return inode_permission(dir, MAY_WRITE | MAY_EXEC);
> +	return inode_permission(dir, MAY_WRITE | MAY_EXEC | mask);
>  }
>  
>  /*
> @@ -2863,7 +2880,7 @@ EXPORT_SYMBOL(unlock_rename);
>  int vfs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
>  		bool want_excl)
>  {
> -	int error = may_create(dir, dentry);
> +	int error = may_create(dir, dentry, false);
>  	if (error)
>  		return error;
>  
> @@ -3650,7 +3667,7 @@ EXPORT_SYMBOL(user_path_create);
>  
>  int vfs_mknod(struct inode *dir, struct dentry *dentry, umode_t mode, dev_t dev)
>  {
> -	int error = may_create(dir, dentry);
> +	int error = may_create(dir, dentry, false);
>  
>  	if (error)
>  		return error;
> @@ -3744,7 +3761,7 @@ SYSCALL_DEFINE3(mknod, const char __user *, filename, umode_t, mode, unsigned, d
>  
>  int vfs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
>  {
> -	int error = may_create(dir, dentry);
> +	int error = may_create(dir, dentry, true);
>  	unsigned max_links = dir->i_sb->s_max_links;
>  
>  	if (error)
> @@ -3800,7 +3817,7 @@ SYSCALL_DEFINE2(mkdir, const char __user *, pathname, umode_t, mode)
>  
>  int vfs_rmdir(struct inode *dir, struct dentry *dentry)
>  {
> -	int error = may_delete(dir, dentry, 1);
> +	int error = may_delete(dir, dentry, true);
>  
>  	if (error)
>  		return error;
> @@ -3922,7 +3939,7 @@ SYSCALL_DEFINE1(rmdir, const char __user *, pathname)
>  int vfs_unlink(struct inode *dir, struct dentry *dentry, struct inode **delegated_inode)
>  {
>  	struct inode *target = dentry->d_inode;
> -	int error = may_delete(dir, dentry, 0);
> +	int error = may_delete(dir, dentry, false);
>  
>  	if (error)
>  		return error;
> @@ -4056,7 +4073,7 @@ SYSCALL_DEFINE1(unlink, const char __user *, pathname)
>  
>  int vfs_symlink(struct inode *dir, struct dentry *dentry, const char *oldname)
>  {
> -	int error = may_create(dir, dentry);
> +	int error = may_create(dir, dentry, false);
>  
>  	if (error)
>  		return error;
> @@ -4139,7 +4156,7 @@ int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_de
>  	if (!inode)
>  		return -ENOENT;
>  
> -	error = may_create(dir, new_dentry);
> +	error = may_create(dir, new_dentry, false);
>  	if (error)
>  		return error;
>  
> @@ -4336,14 +4353,14 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  		return error;
>  
>  	if (!target) {
> -		error = may_create(new_dir, new_dentry);
> +		error = may_create(new_dir, new_dentry, is_dir);
>  	} else {
>  		new_is_dir = d_is_dir(new_dentry);
>  
>  		if (!(flags & RENAME_EXCHANGE))
> -			error = may_delete(new_dir, new_dentry, is_dir);
> +			error = may_replace(new_dir, new_dentry, is_dir);
>  		else
> -			error = may_delete(new_dir, new_dentry, new_is_dir);
> +			error = may_replace(new_dir, new_dentry, new_is_dir);
>  	}
>  	if (error)
>  		return error;
> @@ -4606,7 +4623,7 @@ SYSCALL_DEFINE2(rename, const char __user *, oldname, const char __user *, newna
>  
>  int vfs_whiteout(struct inode *dir, struct dentry *dentry)
>  {
> -	int error = may_create(dir, dentry);
> +	int error = may_create(dir, dentry, false);
>  	if (error)
>  		return error;
>  
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 4ad130c..dd614ad 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -84,6 +84,8 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
>  #define MAY_CHDIR		0x00000040
>  /* called from RCU mode, don't block */
>  #define MAY_NOT_BLOCK		0x00000080
> +#define MAY_CREATE_FILE		0x00000100
> +#define MAY_CREATE_DIR		0x00000200
>  
>  /*
>   * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond

Reviewed-by: Jeff Layton <jlayton@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1434641

FromVolker Lendecke <Volker.Lendecke@SerNet.DE>
Date2016-06-30 17:00 +0200
Message-ID<rPLBE-2p3-17@gated-at.bofh.it>
In reply to#1434569
On Thu, Jun 30, 2016 at 03:46:51PM +0200, Andreas Gruenbacher wrote:
> What more needs to be done to get this initial set of patches merged?

Bind Christoph to some tree in the Harz hills, so that he can't reject
them? :-)

Christoph, is there anything realistic that can be done to get over
your concerns?

Volker

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web