[PATCH -V1 00/22] New ACL format for better NFSv4 acl interoperability

[PATCH -V1 00/22] New ACL format for better NFSv4 acl interoperability

[Aneesh Kumar K.V]

Hi

As per LSF/MM summit discussion I am reposting the richacl patchset for
upstream inclusion. The patchset includes minimal changes required to implement
a new acl model similar to NFSv4 ACL. The acl model selection is based on
file system feature flag. 

The following set of patches implements VFS and ext4 changes needed to implement
a new acl model for linux. Rich ACLs are an implementation of NFSv4 ACLs,
extended by file masks to fit into the standard POSIX file permission model.
They are designed to work seamlessly locally as well as across the NFSv4 and
CIFS/SMB2 network file system protocols.

A user-space utility for displaying and changing richacls is available at [1]
(a number of examples can be found at http://acl.bestbits.at/richacl/examples.html).

[1] git://github.com/kvaneesh/richacl-tools.git master

To test richacl on ext4, create the file sytem with richacl feature flag
(mkfs.ext4 -O richacl or  tune2fs -O richacl). With richacl feature enabled
using mount option "acl" will switch to using richacl instead of posixacl.

More details regarding richacl can be found at
http://acl.bestbits.at/richacl/

Previous posting of the patchset can be found at:
http://mid.gmane.org/1319391835-5829-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com
"[PATCH -V8 00/26]  New ACL format for better NFSv4 acl interoperability"

The complete patchset can also be found at:
https://github.com/kvaneesh/linux/commits/richacl-for-upstream

Since we are trying to get the changes merged upstream after a long time, I am
posting this as V1 again. I Also dropped the Acked-by tag from
David Howells <dhowells@redhat.com> and J. Bruce Fields <bfields@redhat.com>.
Please let me know if I can add them back again.

-aneesh

[PATCH -V1 01/22] vfs: Add generic IS_ACL() test for acl support

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

When IS_POSIXACL() is true, the vfs does not apply the umask.  Other acl
models will need the same exception, so introduce a separate IS_ACL()
test.

The IS_POSIX_ACL() test is still needed so that nfsd can determine when
the underlying file system supports POSIX ACLs (as opposed to some other
kind).

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/namei.c              | 8 ++++----
 include/linux/fs.h      | 6 ++++++
 include/uapi/linux/fs.h | 2 +-
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index c6157c894fce..c7fee619691f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2650,7 +2650,7 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
 	}
 
 	mode = op->mode;
-	if ((open_flag & O_CREAT) && !IS_POSIXACL(dir))
+	if ((open_flag & O_CREAT) && !IS_ACL(dir))
 		mode &= ~current_umask();
 
 	excl = (open_flag & (O_EXCL | O_CREAT)) == (O_EXCL | O_CREAT);
@@ -2834,7 +2834,7 @@ static int lookup_open(struct nameidata *nd, struct path *path,
 	/* Negative dentry, just create the file */
 	if (!dentry->d_inode && (op->open_flag & O_CREAT)) {
 		umode_t mode = op->mode;
-		if (!IS_POSIXACL(dir->d_inode))
+		if (!IS_ACL(dir->d_inode))
 			mode &= ~current_umask();
 		/*
 		 * This write is needed to ensure that a
@@ -3422,7 +3422,7 @@ retry:
 	if (IS_ERR(dentry))
 		return PTR_ERR(dentry);
 
-	if (!IS_POSIXACL(path.dentry->d_inode))
+	if (!IS_ACL(path.dentry->d_inode))
 		mode &= ~current_umask();
 	error = security_path_mknod(&path, dentry, mode, dev);
 	if (error)
@@ -3491,7 +3491,7 @@ retry:
 	if (IS_ERR(dentry))
 		return PTR_ERR(dentry);
 
-	if (!IS_POSIXACL(path.dentry->d_inode))
+	if (!IS_ACL(path.dentry->d_inode))
 		mode &= ~current_umask();
 	error = security_path_mkdir(&path, dentry, mode);
 	if (!error)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7a9c5bca2b76..9fb63b71a014 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1611,6 +1611,12 @@ struct super_operations {
 #define IS_NOSEC(inode)		((inode)->i_flags & S_NOSEC)
 
 /*
+ * IS_ACL() tells the VFS to not apply the umask
+ * and use check_acl for acl permission checks when defined.
+ */
+#define IS_ACL(inode)		__IS_FLG(inode, MS_POSIXACL)
+
+/*
  * Inode state bits.  Protected by inode->i_lock
  *
  * Three bits determine the dirty state of the inode, I_DIRTY_SYNC,
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index ca1a11bb4443..1e14b9c82703 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -80,7 +80,7 @@ struct inodes_stat_t {
 #define MS_VERBOSE	32768	/* War is peace. Verbosity is silence.
 				   MS_VERBOSE is deprecated. */
 #define MS_SILENT	32768
-#define MS_POSIXACL	(1<<16)	/* VFS does not apply the umask */
+#define MS_POSIXACL	(1<<16)	/* Supports POSIX ACLs */
 #define MS_UNBINDABLE	(1<<17)	/* change to unbindable */
 #define MS_PRIVATE	(1<<18)	/* change to private */
 #define MS_SLAVE	(1<<19)	/* change to slave */
-- 
1.9.1

[PATCH -V1 03/22] vfs: Optimize out IS_RICHACL() if CONFIG_FS_RICHACL is not defined

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

if CONFIG_FS_RICHACL is not defined optimize out
the ACL check function.

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/Kconfig         | 3 +++
 include/linux/fs.h | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/fs/Kconfig b/fs/Kconfig
index 312393f32948..facd968f8e84 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -50,6 +50,9 @@ endif # BLOCK
 config FS_POSIX_ACL
 	def_bool n
 
+config FS_RICHACL
+	def_bool n
+
 config EXPORTFS
 	tristate
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1a7fe920f956..1f524b4b652d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1601,7 +1601,12 @@ struct super_operations {
 #define IS_APPEND(inode)	((inode)->i_flags & S_APPEND)
 #define IS_IMMUTABLE(inode)	((inode)->i_flags & S_IMMUTABLE)
 #define IS_POSIXACL(inode)	__IS_FLG(inode, MS_POSIXACL)
+
+#ifdef CONFIG_FS_RICHACL
 #define IS_RICHACL(inode)	__IS_FLG(inode, MS_RICHACL)
+#else
+#define IS_RICHACL(inode)	0
+#endif
 
 #define IS_DEADDIR(inode)	((inode)->i_flags & S_DEAD)
 #define IS_NOCMTIME(inode)	((inode)->i_flags & S_NOCMTIME)
-- 
1.9.1

[PATCH -V1 04/22] vfs: check for directory early

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/namei.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index c7fee619691f..7d93d195f0e5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3868,6 +3868,9 @@ int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_de
 	if (!inode)
 		return -ENOENT;
 
+	if (S_ISDIR(inode->i_mode))
+		return -EPERM;
+
 	error = may_create(dir, new_dentry);
 	if (error)
 		return error;
@@ -3882,8 +3885,6 @@ int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_de
 		return -EPERM;
 	if (!dir->i_op->link)
 		return -EPERM;
-	if (S_ISDIR(inode->i_mode))
-		return -EPERM;
 
 	error = security_inode_link(old_dentry, dir, new_dentry);
 	if (error)
-- 
1.9.1

[PATCH -V1 05/22] vfs: Add new file and directory create permission flags

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

Some permission models distinguish between the permission to create a
non-directory and a directory.  Pass this information down to
inode_permission() as mask flags

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/namei.c         | 21 ++++++++++++---------
 include/linux/fs.h |  2 ++
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 7d93d195f0e5..028bc8bcf77c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -445,7 +445,8 @@ 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.
+ * When checking for MAY_APPEND, MAY_CREATE_FILE, MAY_CREATE_DIR,
+ * MAY_WRITE must also be set in @mask.
  */
 int inode_permission(struct inode *inode, int mask)
 {
@@ -2444,14 +2445,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);
 }
 
 /*
@@ -2501,7 +2504,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, 0);
 	if (error)
 		return error;
 
@@ -3363,7 +3366,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, 0);
 
 	if (error)
 		return error;
@@ -3455,7 +3458,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, 1);
 	unsigned max_links = dir->i_sb->s_max_links;
 
 	if (error)
@@ -3785,7 +3788,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, 0);
 
 	if (error)
 		return error;
@@ -3871,7 +3874,7 @@ int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_de
 	if (S_ISDIR(inode->i_mode))
 		return -EPERM;
 
-	error = may_create(dir, new_dentry);
+	error = may_create(dir, new_dentry, 0);
 	if (error)
 		return error;
 
@@ -4062,7 +4065,7 @@ 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);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 1f524b4b652d..da5521de04ab 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -77,6 +77,8 @@ typedef void (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
-- 
1.9.1

Re: [PATCH -V1 05/22] vfs: Add new file and directory create permission flags

[Jeff Layton]

On Sun, 27 Apr 2014 21:44:36 +0530
"Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> wrote:

> From: Andreas Gruenbacher <agruen@kernel.org>
> 
> Some permission models distinguish between the permission to create a
> non-directory and a directory.  Pass this information down to
> inode_permission() as mask flags
> 
> Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  fs/namei.c         | 21 ++++++++++++---------
>  include/linux/fs.h |  2 ++
>  2 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 7d93d195f0e5..028bc8bcf77c 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -445,7 +445,8 @@ 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.
> + * When checking for MAY_APPEND, MAY_CREATE_FILE, MAY_CREATE_DIR,
> + * MAY_WRITE must also be set in @mask.
>   */
>  int inode_permission(struct inode *inode, int mask)
>  {
> @@ -2444,14 +2445,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);
>  }
>  
>  /*
> @@ -2501,7 +2504,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, 0);

nit: since the third argument here is a bool, this should be "false"
and not "0". Ditto for all of the other calls of may_create.

>  	if (error)
>  		return error;
>  
> @@ -3363,7 +3366,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, 0);
>  
>  	if (error)
>  		return error;
> @@ -3455,7 +3458,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, 1);
>  	unsigned max_links = dir->i_sb->s_max_links;
>  
>  	if (error)
> @@ -3785,7 +3788,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, 0);
>  
>  	if (error)
>  		return error;
> @@ -3871,7 +3874,7 @@ int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_de
>  	if (S_ISDIR(inode->i_mode))
>  		return -EPERM;
>  
> -	error = may_create(dir, new_dentry);
> +	error = may_create(dir, new_dentry, 0);
>  	if (error)
>  		return error;
>  
> @@ -4062,7 +4065,7 @@ 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);
>  
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 1f524b4b652d..da5521de04ab 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -77,6 +77,8 @@ typedef void (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


-- 
Jeff Layton <jlayton@poochiereds.net>

Re: [PATCH -V1 05/22] vfs: Add new file and directory create permission flags

[Dave Chinner]

On Mon, Apr 28, 2014 at 07:23:01AM -0400, Jeff Layton wrote:
> On Sun, 27 Apr 2014 21:44:36 +0530
> "Aneesh Kumar K.V" <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
> 
> > From: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > 
> > Some permission models distinguish between the permission to create a
> > non-directory and a directory.  Pass this information down to
> > inode_permission() as mask flags
> > 
> > Signed-off-by: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> > ---
> >  fs/namei.c         | 21 ++++++++++++---------
> >  include/linux/fs.h |  2 ++
> >  2 files changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fs/namei.c b/fs/namei.c
> > index 7d93d195f0e5..028bc8bcf77c 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -445,7 +445,8 @@ 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.
> > + * When checking for MAY_APPEND, MAY_CREATE_FILE, MAY_CREATE_DIR,
> > + * MAY_WRITE must also be set in @mask.
> >   */
> >  int inode_permission(struct inode *inode, int mask)
> >  {
> > @@ -2444,14 +2445,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);
> >  }
> >  
> >  /*
> > @@ -2501,7 +2504,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, 0);
> 
> nit: since the third argument here is a bool, this should be "false"
> and not "0". Ditto for all of the other calls of may_create.

IMO, the third argument should be MAY_CREATE_FILE or MAY_CREATE_DIR,
which is what the boolean evaluates to in may_create()....

Cheers,

Dave.
-- 
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -V1 05/22] vfs: Add new file and directory create permission flags

[Aneesh Kumar K.V]

Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> writes:

> On Mon, Apr 28, 2014 at 07:23:01AM -0400, Jeff Layton wrote:
>> On Sun, 27 Apr 2014 21:44:36 +0530
>> "Aneesh Kumar K.V" <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
>> 
>> > From: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> > 
>> > Some permission models distinguish between the permission to create a
>> > non-directory and a directory.  Pass this information down to
>> > inode_permission() as mask flags
>> > 
>> > Signed-off-by: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> > ---
>> >  fs/namei.c         | 21 ++++++++++++---------
>> >  include/linux/fs.h |  2 ++
>> >  2 files changed, 14 insertions(+), 9 deletions(-)
>> > 
>> > diff --git a/fs/namei.c b/fs/namei.c
>> > index 7d93d195f0e5..028bc8bcf77c 100644
>> > --- a/fs/namei.c
>> > +++ b/fs/namei.c
>> > @@ -445,7 +445,8 @@ 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.
>> > + * When checking for MAY_APPEND, MAY_CREATE_FILE, MAY_CREATE_DIR,
>> > + * MAY_WRITE must also be set in @mask.
>> >   */
>> >  int inode_permission(struct inode *inode, int mask)
>> >  {
>> > @@ -2444,14 +2445,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);
>> >  }
>> >  
>> >  /*
>> > @@ -2501,7 +2504,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, 0);
>> 
>> nit: since the third argument here is a bool, this should be "false"
>> and not "0". Ditto for all of the other calls of may_create.
>
> IMO, the third argument should be MAY_CREATE_FILE or MAY_CREATE_DIR,
> which is what the boolean evaluates to in may_create()....

Will do that.

Thanks
-aneesh

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH -V1 06/22] vfs: Add delete child and delete self permission flags

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

Normally, deleting a file requires write access to the parent directory.
Some permission models use a different permission on the parent
directory to indicate delete access.  In addition, a process can have
per-file delete access even without delete access on the parent
directory.

Introduce two new inode_permission() mask flags and use them in
may_delete()

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/namei.c         | 45 ++++++++++++++++++++++++++++++++++-----------
 include/linux/fs.h |  2 ++
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 028bc8bcf77c..56ac7613fbca 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -446,7 +446,7 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
  * changing the "normal" UIDs which are used for other things.
  *
  * When checking for MAY_APPEND, MAY_CREATE_FILE, MAY_CREATE_DIR,
- * MAY_WRITE must also be set in @mask.
+ * MAY_DELETE_CHILD, MAY_DELETE_SELF, MAY_WRITE must also be set in @mask.
  */
 int inode_permission(struct inode *inode, int mask)
 {
@@ -2366,11 +2366,25 @@ kern_path_mountpoint(int dfd, const char *name, struct path *path,
 }
 EXPORT_SYMBOL(kern_path_mountpoint);
 
+
+/*
+ * We should have exec permission on directory and MAY_DELETE_SELF
+ * on the object being deleted.
+ */
+static int richacl_may_selfdelete(struct inode *dir,
+				  struct inode *inode, int replace_mask)
+{
+	return (IS_RICHACL(inode) &&
+		(inode_permission(dir, MAY_EXEC | replace_mask) == 0) &&
+		(inode_permission(inode, MAY_DELETE_SELF) == 0));
+}
+
 /*
  * It's inline, so penalty for filesystems that don't use sticky bit is
  * minimal.
  */
-static inline int check_sticky(struct inode *dir, struct inode *inode)
+static inline int check_sticky(struct inode *dir,
+			       struct inode *inode, int replace_mask)
 {
 	kuid_t fsuid = current_fsuid();
 
@@ -2380,6 +2394,8 @@ static inline int check_sticky(struct inode *dir, struct inode *inode)
 		return 0;
 	if (uid_eq(dir->i_uid, fsuid))
 		return 0;
+	if (richacl_may_selfdelete(dir, inode, replace_mask))
+		return 0;
 	return !inode_capable(inode, CAP_FOWNER);
 }
 
@@ -2402,10 +2418,11 @@ static inline int check_sticky(struct inode *dir, struct inode *inode)
  * 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(struct inode *dir, struct dentry *victim,
+		      bool isdir, bool replace)
 {
 	struct inode *inode = victim->d_inode;
-	int error;
+	int error, mask, replace_mask = 0;
 
 	if (d_is_negative(victim))
 		return -ENOENT;
@@ -2414,13 +2431,19 @@ 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);
+	mask = MAY_WRITE | MAY_EXEC | MAY_DELETE_CHILD;
+	if (replace)
+		replace_mask = S_ISDIR(inode->i_mode) ?
+				MAY_CREATE_DIR : MAY_CREATE_FILE;
+	error = inode_permission(dir, mask | replace_mask);
+	if (error && richacl_may_selfdelete(dir, inode, replace_mask))
+		error = 0;
 	if (error)
 		return error;
 	if (IS_APPEND(dir))
 		return -EPERM;
 
-	if (check_sticky(dir, inode) || IS_APPEND(inode) ||
+	if (check_sticky(dir, inode, replace_mask) || IS_APPEND(inode) ||
 	    IS_IMMUTABLE(inode) || IS_SWAPFILE(inode))
 		return -EPERM;
 	if (isdir) {
@@ -3539,7 +3562,7 @@ EXPORT_SYMBOL(dentry_unhash);
 
 int vfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
-	int error = may_delete(dir, dentry, 1);
+	int error = may_delete(dir, dentry, 1, 0);
 
 	if (error)
 		return error;
@@ -3658,7 +3681,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, 0, 0);
 
 	if (error)
 		return error;
@@ -4060,7 +4083,7 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 	if (source == target)
 		return 0;
 
-	error = may_delete(old_dir, old_dentry, is_dir);
+	error = may_delete(old_dir, old_dentry, is_dir, 0);
 	if (error)
 		return error;
 
@@ -4070,9 +4093,9 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		new_is_dir = d_is_dir(new_dentry);
 
 		if (!(flags & RENAME_EXCHANGE))
-			error = may_delete(new_dir, new_dentry, is_dir);
+			error = may_delete(new_dir, new_dentry, is_dir, 1);
 		else
-			error = may_delete(new_dir, new_dentry, new_is_dir);
+			error = may_delete(new_dir, new_dentry, new_is_dir, 1);
 	}
 	if (error)
 		return error;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index da5521de04ab..3f0ad0f2bce8 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -79,6 +79,8 @@ typedef void (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
-- 
1.9.1

Re: [PATCH -V1 06/22] vfs: Add delete child and delete self permission flags

[Dave Chinner]

On Sun, Apr 27, 2014 at 09:44:37PM +0530, Aneesh Kumar K.V wrote:
> From: Andreas Gruenbacher <agruen@kernel.org>
> 
> Normally, deleting a file requires write access to the parent directory.
> Some permission models use a different permission on the parent
> directory to indicate delete access.  In addition, a process can have
> per-file delete access even without delete access on the parent
> directory.
> 
> Introduce two new inode_permission() mask flags and use them in
> may_delete()
> 
> Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  fs/namei.c         | 45 ++++++++++++++++++++++++++++++++++-----------
>  include/linux/fs.h |  2 ++
>  2 files changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 028bc8bcf77c..56ac7613fbca 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -446,7 +446,7 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
>   * changing the "normal" UIDs which are used for other things.
>   *
>   * When checking for MAY_APPEND, MAY_CREATE_FILE, MAY_CREATE_DIR,
> - * MAY_WRITE must also be set in @mask.
> + * MAY_DELETE_CHILD, MAY_DELETE_SELF, MAY_WRITE must also be set in @mask.
>   */
>  int inode_permission(struct inode *inode, int mask)
>  {
> @@ -2366,11 +2366,25 @@ kern_path_mountpoint(int dfd, const char *name, struct path *path,
>  }
>  EXPORT_SYMBOL(kern_path_mountpoint);
>  
> +
> +/*
> + * We should have exec permission on directory and MAY_DELETE_SELF
> + * on the object being deleted.
> + */
> +static int richacl_may_selfdelete(struct inode *dir,
> +				  struct inode *inode, int replace_mask)
> +{
> +	return (IS_RICHACL(inode) &&
> +		(inode_permission(dir, MAY_EXEC | replace_mask) == 0) &&
> +		(inode_permission(inode, MAY_DELETE_SELF) == 0));
> +}

Can't say I like these "richacl" prefixes. Why not just "may_*"
like all the other permission checks?


> @@ -2414,13 +2431,19 @@ 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);
> +	mask = MAY_WRITE | MAY_EXEC | MAY_DELETE_CHILD;
> +	if (replace)
> +		replace_mask = S_ISDIR(inode->i_mode) ?
> +				MAY_CREATE_DIR : MAY_CREATE_FILE;
> +	error = inode_permission(dir, mask | replace_mask);
> +	if (error && richacl_may_selfdelete(dir, inode, replace_mask))
> +		error = 0;
>  	if (error)
>  		return error;
>  	if (IS_APPEND(dir))
>  		return -EPERM;
>  
> -	if (check_sticky(dir, inode) || IS_APPEND(inode) ||
> +	if (check_sticky(dir, inode, replace_mask) || IS_APPEND(inode) ||
>  	    IS_IMMUTABLE(inode) || IS_SWAPFILE(inode))
>  		return -EPERM;
>  	if (isdir) {
> @@ -3539,7 +3562,7 @@ EXPORT_SYMBOL(dentry_unhash);
>  
>  int vfs_rmdir(struct inode *dir, struct dentry *dentry)
>  {
> -	int error = may_delete(dir, dentry, 1);
> +	int error = may_delete(dir, dentry, 1, 0);
>  
>  	if (error)
>  		return error;
> @@ -3658,7 +3681,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, 0, 0);
>  
>  	if (error)
>  		return error;
> @@ -4060,7 +4083,7 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  	if (source == target)
>  		return 0;
>  
> -	error = may_delete(old_dir, old_dentry, is_dir);
> +	error = may_delete(old_dir, old_dentry, is_dir, 0);
>  	if (error)
>  		return error;
>  
> @@ -4070,9 +4093,9 @@ int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
>  		new_is_dir = d_is_dir(new_dentry);
>  
>  		if (!(flags & RENAME_EXCHANGE))
> -			error = may_delete(new_dir, new_dentry, is_dir);
> +			error = may_delete(new_dir, new_dentry, is_dir, 1);
>  		else
> -			error = may_delete(new_dir, new_dentry, new_is_dir);
> +			error = may_delete(new_dir, new_dentry, new_is_dir, 1);

Another boolean parameter that means nothing at the call site.  This
should really be passing a flags field, not a bunch of booleans that
are simply evaluated into flags...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH -V1 06/22] vfs: Add delete child and delete self permission flags

[Aneesh Kumar K.V]

Dave Chinner <david@fromorbit.com> writes:

> On Sun, Apr 27, 2014 at 09:44:37PM +0530, Aneesh Kumar K.V wrote:
>> From: Andreas Gruenbacher <agruen@kernel.org>
>> 
>> Normally, deleting a file requires write access to the parent directory.
>> Some permission models use a different permission on the parent
>> directory to indicate delete access.  In addition, a process can have
>> per-file delete access even without delete access on the parent
>> directory.
>> 
>> Introduce two new inode_permission() mask flags and use them in
>> may_delete()
>> 
>> Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>>  fs/namei.c         | 45 ++++++++++++++++++++++++++++++++++-----------
>>  include/linux/fs.h |  2 ++
>>  2 files changed, 36 insertions(+), 11 deletions(-)
>> 
>> diff --git a/fs/namei.c b/fs/namei.c
>> index 028bc8bcf77c..56ac7613fbca 100644
>> --- a/fs/namei.c
>> +++ b/fs/namei.c
>> @@ -446,7 +446,7 @@ static int sb_permission(struct super_block *sb, struct inode *inode, int mask)
>>   * changing the "normal" UIDs which are used for other things.
>>   *
>>   * When checking for MAY_APPEND, MAY_CREATE_FILE, MAY_CREATE_DIR,
>> - * MAY_WRITE must also be set in @mask.
>> + * MAY_DELETE_CHILD, MAY_DELETE_SELF, MAY_WRITE must also be set in @mask.
>>   */
>>  int inode_permission(struct inode *inode, int mask)
>>  {
>> @@ -2366,11 +2366,25 @@ kern_path_mountpoint(int dfd, const char *name, struct path *path,
>>  }
>>  EXPORT_SYMBOL(kern_path_mountpoint);
>>  
>> +
>> +/*
>> + * We should have exec permission on directory and MAY_DELETE_SELF
>> + * on the object being deleted.
>> + */
>> +static int richacl_may_selfdelete(struct inode *dir,
>> +				  struct inode *inode, int replace_mask)
>> +{
>> +	return (IS_RICHACL(inode) &&
>> +		(inode_permission(dir, MAY_EXEC | replace_mask) == 0) &&
>> +		(inode_permission(inode, MAY_DELETE_SELF) == 0));
>> +}
>
> Can't say I like these "richacl" prefixes. Why not just "may_*"
> like all the other permission checks?

Will update.

>
>
>> @@ -2414,13 +2431,19 @@ 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);
>> +	mask = MAY_WRITE | MAY_EXEC | MAY_DELETE_CHILD;
>> +	if (replace)
>> +		replace_mask = S_ISDIR(inode->i_mode) ?
>> +				MAY_CREATE_DIR : MAY_CREATE_FILE;
>> +	error = inode_permission(dir, mask | replace_mask);
>> +	if (error && richacl_may_selfdelete(dir, inode, replace_mask))
>> +		error = 0;

....

>>  
>>  		if (!(flags & RENAME_EXCHANGE))
>> -			error = may_delete(new_dir, new_dentry, is_dir);
>> +			error = may_delete(new_dir, new_dentry, is_dir, 1);
>>  		else
>> -			error = may_delete(new_dir, new_dentry, new_is_dir);
>> +			error = may_delete(new_dir, new_dentry, new_is_dir, 1);
>
> Another boolean parameter that means nothing at the call site.  This
> should really be passing a flags field, not a bunch of booleans that
> are simply evaluated into flags...
>

Will update

Thanks
-aneesh

[PATCH -V1 07/22] vfs: Make the inode passed to inode_change_ok non-const

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

We will need to call iop->permission and iop->get_acl from
inode_change_ok() for additional permission checks, and both take a
non-const inode.

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/attr.c          | 2 +-
 include/linux/fs.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index 5d4e59d56e85..1d158c972442 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -28,7 +28,7 @@
  * Should be called as the first thing in ->setattr implementations,
  * possibly after taking additional locks.
  */
-int inode_change_ok(const struct inode *inode, struct iattr *attr)
+int inode_change_ok(struct inode *inode, struct iattr *attr)
 {
 	unsigned int ia_valid = attr->ia_valid;
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 3f0ad0f2bce8..33da154dd27d 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2618,7 +2618,7 @@ extern int buffer_migrate_page(struct address_space *,
 #define buffer_migrate_page NULL
 #endif
 
-extern int inode_change_ok(const struct inode *, struct iattr *);
+extern int inode_change_ok(struct inode *, struct iattr *);
 extern int inode_newsize_ok(const struct inode *, loff_t offset);
 extern void setattr_copy(struct inode *inode, const struct iattr *attr);
 
-- 
1.9.1

[PATCH -V1 08/22] vfs: Add permission flags for setting file attributes

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

Some permission models can allow processes to take ownership of a file,
change the file permissions, and set the file timestamps.  Introduce new
permission mask flags and check for those permissions in
inode_change_ok().

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/attr.c          | 68 ++++++++++++++++++++++++++++++++++++++++++++----------
 fs/namei.c         |  2 +-
 include/linux/fs.h |  4 ++++
 3 files changed, 61 insertions(+), 13 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index 1d158c972442..e468d4f2dca8 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -16,6 +16,54 @@
 #include <linux/evm.h>
 #include <linux/ima.h>
 
+static int richacl_change_ok(struct inode *inode, int mask)
+{
+	if (!IS_RICHACL(inode))
+		return -EPERM;
+
+	if (inode->i_op->permission)
+		return inode->i_op->permission(inode, mask);
+
+	return check_acl(inode, mask);
+}
+
+static bool inode_uid_change_ok(struct inode *inode, kuid_t ia_uid)
+{
+	if (uid_eq(current_fsuid(), inode->i_uid) &&
+	    uid_eq(ia_uid, inode->i_uid))
+		return true;
+	if (uid_eq(current_fsuid(), ia_uid) &&
+	    richacl_change_ok(inode, MAY_TAKE_OWNERSHIP) == 0)
+		return true;
+	if (capable(CAP_CHOWN))
+		return true;
+	return false;
+}
+
+static bool inode_gid_change_ok(struct inode *inode, kgid_t ia_gid)
+{
+	int in_group = in_group_p(ia_gid);
+	if (uid_eq(current_fsuid(), inode->i_uid) &&
+	    (in_group || gid_eq(ia_gid, inode->i_gid)))
+		return true;
+	if (in_group && richacl_change_ok(inode, MAY_TAKE_OWNERSHIP) == 0)
+		return true;
+	if (capable(CAP_CHOWN))
+		return true;
+	return false;
+}
+
+static bool inode_owner_permitted_or_capable(struct inode *inode, int mask)
+{
+	if (uid_eq(current_fsuid(), inode->i_uid))
+		return true;
+	if (richacl_change_ok(inode, mask) == 0)
+		return true;
+	if (inode_capable(inode, CAP_FOWNER))
+		return true;
+	return false;
+}
+
 /**
  * inode_change_ok - check if attribute changes to an inode are allowed
  * @inode:	inode to check
@@ -47,22 +95,18 @@ int inode_change_ok(struct inode *inode, struct iattr *attr)
 		return 0;
 
 	/* Make sure a caller can chown. */
-	if ((ia_valid & ATTR_UID) &&
-	    (!uid_eq(current_fsuid(), inode->i_uid) ||
-	     !uid_eq(attr->ia_uid, inode->i_uid)) &&
-	    !inode_capable(inode, CAP_CHOWN))
-		return -EPERM;
+	if (ia_valid & ATTR_UID)
+		if (!inode_uid_change_ok(inode, attr->ia_uid))
+			return -EPERM;
 
 	/* Make sure caller can chgrp. */
-	if ((ia_valid & ATTR_GID) &&
-	    (!uid_eq(current_fsuid(), inode->i_uid) ||
-	    (!in_group_p(attr->ia_gid) && !gid_eq(attr->ia_gid, inode->i_gid))) &&
-	    !inode_capable(inode, CAP_CHOWN))
-		return -EPERM;
+	if (ia_valid & ATTR_GID)
+		if (!inode_gid_change_ok(inode, attr->ia_gid))
+			return -EPERM;
 
 	/* Make sure a caller can chmod. */
 	if (ia_valid & ATTR_MODE) {
-		if (!inode_owner_or_capable(inode))
+		if (!inode_owner_permitted_or_capable(inode, MAY_CHMOD))
 			return -EPERM;
 		/* Also check the setgid bit! */
 		if (!in_group_p((ia_valid & ATTR_GID) ? attr->ia_gid :
@@ -73,7 +117,7 @@ int inode_change_ok(struct inode *inode, struct iattr *attr)
 
 	/* Check for setting the inode time. */
 	if (ia_valid & (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET)) {
-		if (!inode_owner_or_capable(inode))
+		if (!inode_owner_permitted_or_capable(inode, MAY_SET_TIMES))
 			return -EPERM;
 	}
 
diff --git a/fs/namei.c b/fs/namei.c
index 56ac7613fbca..26b9a8212837 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -249,7 +249,7 @@ void putname(struct filename *name)
 }
 #endif
 
-static int check_acl(struct inode *inode, int mask)
+int check_acl(struct inode *inode, int mask)
 {
 #ifdef CONFIG_FS_POSIX_ACL
 	struct posix_acl *acl;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 33da154dd27d..22d85798b520 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -81,6 +81,9 @@ typedef void (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 #define MAY_CREATE_DIR		0x00000200
 #define MAY_DELETE_CHILD	0x00000400
 #define MAY_DELETE_SELF		0x00000800
+#define MAY_TAKE_OWNERSHIP	0x00001000
+#define MAY_CHMOD		0x00002000
+#define MAY_SET_TIMES		0x00004000
 
 /*
  * flags in file.f_mode.  Note that FMODE_READ and FMODE_WRITE must correspond
@@ -2248,6 +2251,7 @@ extern sector_t bmap(struct inode *, sector_t);
 extern int notify_change(struct dentry *, struct iattr *, struct inode **);
 extern int inode_permission(struct inode *, int);
 extern int generic_permission(struct inode *, int);
+extern int check_acl(struct inode *, int);
 
 static inline bool execute_ok(struct inode *inode)
 {
-- 
1.9.1

Re: [PATCH -V1 08/22] vfs: Add permission flags for setting file attributes

[Dave Chinner]

On Sun, Apr 27, 2014 at 09:44:39PM +0530, Aneesh Kumar K.V wrote:
> From: Andreas Gruenbacher <agruen@kernel.org>
> 
> Some permission models can allow processes to take ownership of a file,
> change the file permissions, and set the file timestamps.  Introduce new
> permission mask flags and check for those permissions in
> inode_change_ok().
> 
> Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  fs/attr.c          | 68 ++++++++++++++++++++++++++++++++++++++++++++----------
>  fs/namei.c         |  2 +-
>  include/linux/fs.h |  4 ++++
>  3 files changed, 61 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/attr.c b/fs/attr.c
> index 1d158c972442..e468d4f2dca8 100644
> --- a/fs/attr.c
> +++ b/fs/attr.c
> @@ -16,6 +16,54 @@
>  #include <linux/evm.h>
>  #include <linux/ima.h>
>  
> +static int richacl_change_ok(struct inode *inode, int mask)
> +{

acl_change_ok()

> +	if (!IS_RICHACL(inode))
> +		return -EPERM;
> +
> +	if (inode->i_op->permission)
> +		return inode->i_op->permission(inode, mask);
> +
> +	return check_acl(inode, mask);
> +}
> +
> +static bool inode_uid_change_ok(struct inode *inode, kuid_t ia_uid)
> +{
> +	if (uid_eq(current_fsuid(), inode->i_uid) &&
> +	    uid_eq(ia_uid, inode->i_uid))
> +		return true;
> +	if (uid_eq(current_fsuid(), ia_uid) &&
> +	    richacl_change_ok(inode, MAY_TAKE_OWNERSHIP) == 0)
> +		return true;
> +	if (capable(CAP_CHOWN))
> +		return true;
> +	return false;
> +}
> +
> +static bool inode_gid_change_ok(struct inode *inode, kgid_t ia_gid)
> +{
> +	int in_group = in_group_p(ia_gid);
> +	if (uid_eq(current_fsuid(), inode->i_uid) &&
> +	    (in_group || gid_eq(ia_gid, inode->i_gid)))
> +		return true;
> +	if (in_group && richacl_change_ok(inode, MAY_TAKE_OWNERSHIP) == 0)
> +		return true;
> +	if (capable(CAP_CHOWN))
> +		return true;
> +	return false;
> +}
> +
> +static bool inode_owner_permitted_or_capable(struct inode *inode, int mask)
> +{
> +	if (uid_eq(current_fsuid(), inode->i_uid))
> +		return true;
> +	if (richacl_change_ok(inode, mask) == 0)
> +		return true;
> +	if (inode_capable(inode, CAP_FOWNER))
> +		return true;
> +	return false;
> +}

Some comments on when and why these need to be used instead of
inode_owner_or_capable() would be useful. I can see people getting
this wrong in future.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH -V1 08/22] vfs: Add permission flags for setting file attributes

[Aneesh Kumar K.V]

Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> writes:

> On Sun, Apr 27, 2014 at 09:44:39PM +0530, Aneesh Kumar K.V wrote:
>> From: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> 
>> Some permission models can allow processes to take ownership of a file,
>> change the file permissions, and set the file timestamps.  Introduce new
>> permission mask flags and check for those permissions in
>> inode_change_ok().
>> 
>> Signed-off-by: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> ---
>>  fs/attr.c          | 68 ++++++++++++++++++++++++++++++++++++++++++++----------
>>  fs/namei.c         |  2 +-
>>  include/linux/fs.h |  4 ++++
>>  3 files changed, 61 insertions(+), 13 deletions(-)
>> 
>> diff --git a/fs/attr.c b/fs/attr.c
>> index 1d158c972442..e468d4f2dca8 100644
>> --- a/fs/attr.c
>> +++ b/fs/attr.c
>> @@ -16,6 +16,54 @@
>>  #include <linux/evm.h>
>>  #include <linux/ima.h>
>>  
>> +static int richacl_change_ok(struct inode *inode, int mask)
>> +{
>
> acl_change_ok()


Will update.

>
>> +	if (!IS_RICHACL(inode))
>> +		return -EPERM;
>> +
>> +	if (inode->i_op->permission)
>> +		return inode->i_op->permission(inode, mask);
>> +
>> +	return check_acl(inode, mask);
>> +}
>> +
>> +static bool inode_uid_change_ok(struct inode *inode, kuid_t ia_uid)
>> +{
>> +	if (uid_eq(current_fsuid(), inode->i_uid) &&
>> +	    uid_eq(ia_uid, inode->i_uid))
>> +		return true;
>> +	if (uid_eq(current_fsuid(), ia_uid) &&
>> +	    richacl_change_ok(inode, MAY_TAKE_OWNERSHIP) == 0)
>> +		return true;
>> +	if (capable(CAP_CHOWN))
>> +		return true;
>> +	return false;
>> +}
>> +
>> +static bool inode_gid_change_ok(struct inode *inode, kgid_t ia_gid)
>> +{
>> +	int in_group = in_group_p(ia_gid);
>> +	if (uid_eq(current_fsuid(), inode->i_uid) &&
>> +	    (in_group || gid_eq(ia_gid, inode->i_gid)))
>> +		return true;
>> +	if (in_group && richacl_change_ok(inode, MAY_TAKE_OWNERSHIP) == 0)
>> +		return true;
>> +	if (capable(CAP_CHOWN))
>> +		return true;
>> +	return false;
>> +}
>> +
>> +static bool inode_owner_permitted_or_capable(struct inode *inode, int mask)
>> +{
>> +	if (uid_eq(current_fsuid(), inode->i_uid))
>> +		return true;
>> +	if (richacl_change_ok(inode, mask) == 0)
>> +		return true;
>> +	if (inode_capable(inode, CAP_FOWNER))
>> +		return true;
>> +	return false;
>> +}
>
> Some comments on when and why these need to be used instead of
> inode_owner_or_capable() would be useful. I can see people getting
> this wrong in future.
>

Ok.

-aneesh

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH -V1 09/22] vfs: Make acl_permission_check() work for richacls

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/namei.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/fs/namei.c b/fs/namei.c
index 26b9a8212837..06474553c08d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -284,6 +284,19 @@ static int acl_permission_check(struct inode *inode, int mask)
 {
 	unsigned int mode = inode->i_mode;
 
+	if (IS_RICHACL(inode)) {
+		int error = check_acl(inode, mask);
+		if (error != -EAGAIN)
+			return error;
+		if (mask & (MAY_DELETE_SELF | MAY_TAKE_OWNERSHIP |
+			    MAY_CHMOD | MAY_SET_TIMES)) {
+			/*
+			 * The file permission bit cannot grant these
+			 * permissions.
+			 */
+			return -EACCES;
+		}
+	}
 	if (likely(uid_eq(current_fsuid(), inode->i_uid)))
 		mode >>= 6;
 	else {
-- 
1.9.1

Re: [PATCH -V1 09/22] vfs: Make acl_permission_check() work for richacls

[Dave Chinner]

On Sun, Apr 27, 2014 at 09:44:40PM +0530, Aneesh Kumar K.V wrote:
> From: Andreas Gruenbacher <agruen@kernel.org>
> 
> Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  fs/namei.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 26b9a8212837..06474553c08d 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -284,6 +284,19 @@ static int acl_permission_check(struct inode *inode, int mask)
>  {
>  	unsigned int mode = inode->i_mode;
>  
> +	if (IS_RICHACL(inode)) {
> +		int error = check_acl(inode, mask);
> +		if (error != -EAGAIN)
> +			return error;
> +		if (mask & (MAY_DELETE_SELF | MAY_TAKE_OWNERSHIP |
> +			    MAY_CHMOD | MAY_SET_TIMES)) {
> +			/*
> +			 * The file permission bit cannot grant these
> +			 * permissions.
> +			 */
> +			return -EACCES;
> +		}
> +	}
>  	if (likely(uid_eq(current_fsuid(), inode->i_uid)))
>  		mode >>= 6;
>  	else {

why does this take priority over a simple uid match? Some comments
explaining this for people unfamiliar with richacls would be nice.
Not to mention the commit message should also explain the change...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH -V1 09/22] vfs: Make acl_permission_check() work for richacls

[Aneesh Kumar K.V]

Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> writes:

> On Sun, Apr 27, 2014 at 09:44:40PM +0530, Aneesh Kumar K.V wrote:
>> From: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> 
>> Signed-off-by: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>> ---
>>  fs/namei.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>> 
>> diff --git a/fs/namei.c b/fs/namei.c
>> index 26b9a8212837..06474553c08d 100644
>> --- a/fs/namei.c
>> +++ b/fs/namei.c
>> @@ -284,6 +284,19 @@ static int acl_permission_check(struct inode *inode, int mask)
>>  {
>>  	unsigned int mode = inode->i_mode;
>>  
>> +	if (IS_RICHACL(inode)) {
>> +		int error = check_acl(inode, mask);
>> +		if (error != -EAGAIN)
>> +			return error;
>> +		if (mask & (MAY_DELETE_SELF | MAY_TAKE_OWNERSHIP |
>> +			    MAY_CHMOD | MAY_SET_TIMES)) {
>> +			/*
>> +			 * The file permission bit cannot grant these
>> +			 * permissions.
>> +			 */
>> +			return -EACCES;
>> +		}
>> +	}
>>  	if (likely(uid_eq(current_fsuid(), inode->i_uid)))
>>  		mode >>= 6;
>>  	else {
>
> why does this take priority over a simple uid match? Some comments
> explaining this for people unfamiliar with richacls would be nice.
> Not to mention the commit message should also explain the change...
>

Richacl can have further limitation on file owner. Will add more
comments around the function.

-aneesh

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH -V1 10/22] richacl: In-memory representation and helper functions

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

A richacl consists of an NFSv4 acl and an owner, group, and other mask.
These three masks correspond to the owner, group, and other file
permission bits, but they contain NFSv4 permissions instead of POSIX
permissions.

Each entry in the NFSv4 acl applies to the file owner (OWNER@), the
owning group (GROUP@), literally everyone (EVERYONE@), or to a specific
uid or gid.

As in the standard POSIX file permission model, each process is the
owner, group, or other file class.  A richacl grants a requested access
only if the NFSv4 acl in the richacl grants the access (according to the
NFSv4 permission check algorithm), and the file mask that applies to the
process includes the requested permissions.

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/Makefile             |   2 +
 fs/richacl_base.c       |  69 ++++++++++++++
 include/linux/richacl.h | 237 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 308 insertions(+)
 create mode 100644 fs/richacl_base.c
 create mode 100644 include/linux/richacl.h

diff --git a/fs/Makefile b/fs/Makefile
index f9cb9876e466..5b7ed5d022ec 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -48,6 +48,8 @@ obj-$(CONFIG_COREDUMP)		+= coredump.o
 obj-$(CONFIG_SYSCTL)		+= drop_caches.o
 
 obj-$(CONFIG_FHANDLE)		+= fhandle.o
+obj-$(CONFIG_FS_RICHACL)	+= richacl.o
+richacl-y			:= richacl_base.o
 
 obj-y				+= quota/
 
diff --git a/fs/richacl_base.c b/fs/richacl_base.c
new file mode 100644
index 000000000000..689e1a6dace7
--- /dev/null
+++ b/fs/richacl_base.c
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2006, 2010  Novell, Inc.
+ * Written by Andreas Gruenbacher <agruen@kernel.org>
+ *
+ * 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/module.h>
+#include <linux/fs.h>
+#include <linux/richacl.h>
+
+MODULE_LICENSE("GPL");
+
+/**
+ * richacl_alloc  -  allocate a richacl
+ * @count:	number of entries
+ */
+struct richacl *
+richacl_alloc(int count)
+{
+	size_t size = sizeof(struct richacl) + count * sizeof(struct richace);
+	struct richacl *acl = kzalloc(size, GFP_KERNEL);
+
+	if (acl) {
+		atomic_set(&acl->a_refcount, 1);
+		acl->a_count = count;
+	}
+	return acl;
+}
+EXPORT_SYMBOL_GPL(richacl_alloc);
+
+/**
+ * richacl_clone  -  create a copy of a richacl
+ */
+static struct richacl *
+richacl_clone(const struct richacl *acl)
+{
+	int count = acl->a_count;
+	size_t size = sizeof(struct richacl) + count * sizeof(struct richace);
+	struct richacl *dup = kmalloc(size, GFP_KERNEL);
+
+	if (dup) {
+		memcpy(dup, acl, size);
+		atomic_set(&dup->a_refcount, 1);
+	}
+	return dup;
+}
+
+/**
+ * richace_is_same_identifier  -  are both identifiers the same?
+ */
+int
+richace_is_same_identifier(const struct richace *a, const struct richace *b)
+{
+#define WHO_FLAGS (ACE4_SPECIAL_WHO | ACE4_IDENTIFIER_GROUP)
+	if ((a->e_flags & WHO_FLAGS) != (b->e_flags & WHO_FLAGS))
+		return 0;
+	return a->e_id == b->e_id;
+#undef WHO_FLAGS
+}
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
new file mode 100644
index 000000000000..51d6937651f1
--- /dev/null
+++ b/include/linux/richacl.h
@@ -0,0 +1,237 @@
+/*
+ * Copyright (C) 2006, 2010  Novell, Inc.
+ * Written by Andreas Gruenbacher <agruen@kernel.org>
+ *
+ * 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_H
+#define __RICHACL_H
+#include <linux/slab.h>
+
+#define ACE_OWNER_ID		130
+#define ACE_GROUP_ID		131
+#define ACE_EVERYONE_ID		110
+
+struct richace {
+	unsigned short	e_type;
+	unsigned short	e_flags;
+	unsigned int	e_mask;
+	unsigned int	e_id;
+};
+
+struct richacl {
+	atomic_t	a_refcount;
+	unsigned int	a_owner_mask;
+	unsigned int	a_group_mask;
+	unsigned int	a_other_mask;
+	unsigned short	a_count;
+	unsigned short	a_flags;
+	struct richace	a_entries[0];
+};
+
+#define richacl_for_each_entry(_ace, _acl) \
+	for (_ace = _acl->a_entries; \
+	     _ace != _acl->a_entries + _acl->a_count; \
+	     _ace++)
+
+#define richacl_for_each_entry_reverse(_ace, _acl) \
+	for (_ace = _acl->a_entries + _acl->a_count - 1; \
+	     _ace != _acl->a_entries - 1; \
+	     _ace--)
+
+/* Flag values defined by rich-acl */
+#define ACL4_MASKED			0x80
+
+#define ACL4_VALID_FLAGS (			\
+		ACL4_MASKED)
+
+/* e_type values */
+#define ACE4_ACCESS_ALLOWED_ACE_TYPE	0x0000
+#define ACE4_ACCESS_DENIED_ACE_TYPE	0x0001
+/*#define ACE4_SYSTEM_AUDIT_ACE_TYPE	0x0002*/
+/*#define ACE4_SYSTEM_ALARM_ACE_TYPE	0x0003*/
+
+/* e_flags bitflags */
+#define ACE4_FILE_INHERIT_ACE		0x0001
+#define ACE4_DIRECTORY_INHERIT_ACE	0x0002
+#define ACE4_NO_PROPAGATE_INHERIT_ACE	0x0004
+#define ACE4_INHERIT_ONLY_ACE		0x0008
+/*#define ACE4_SUCCESSFUL_ACCESS_ACE_FLAG	0x0010*/
+/*#define ACE4_FAILED_ACCESS_ACE_FLAG	0x0020*/
+#define ACE4_IDENTIFIER_GROUP		0x0040
+/* richacl specific flag values */
+#define ACE4_SPECIAL_WHO		0x4000
+
+#define ACE4_VALID_FLAGS (			\
+	ACE4_FILE_INHERIT_ACE |			\
+	ACE4_DIRECTORY_INHERIT_ACE |		\
+	ACE4_NO_PROPAGATE_INHERIT_ACE |		\
+	ACE4_INHERIT_ONLY_ACE |			\
+	ACE4_IDENTIFIER_GROUP |			\
+	ACE4_SPECIAL_WHO)
+
+/* e_mask bitflags */
+#define ACE4_READ_DATA			0x00000001
+#define ACE4_LIST_DIRECTORY		0x00000001
+#define ACE4_WRITE_DATA			0x00000002
+#define ACE4_ADD_FILE			0x00000002
+#define ACE4_APPEND_DATA		0x00000004
+#define ACE4_ADD_SUBDIRECTORY		0x00000004
+#define ACE4_READ_NAMED_ATTRS		0x00000008
+#define ACE4_WRITE_NAMED_ATTRS		0x00000010
+#define ACE4_EXECUTE			0x00000020
+#define ACE4_DELETE_CHILD		0x00000040
+#define ACE4_READ_ATTRIBUTES		0x00000080
+#define ACE4_WRITE_ATTRIBUTES		0x00000100
+#define ACE4_WRITE_RETENTION		0x00000200
+#define ACE4_WRITE_RETENTION_HOLD	0x00000400
+#define ACE4_DELETE			0x00010000
+#define ACE4_READ_ACL			0x00020000
+#define ACE4_WRITE_ACL			0x00040000
+#define ACE4_WRITE_OWNER		0x00080000
+#define ACE4_SYNCHRONIZE		0x00100000
+
+/* Valid ACE4_* flags for directories and non-directories */
+#define ACE4_VALID_MASK (				\
+	ACE4_READ_DATA | ACE4_LIST_DIRECTORY |		\
+	ACE4_WRITE_DATA | ACE4_ADD_FILE |		\
+	ACE4_APPEND_DATA | ACE4_ADD_SUBDIRECTORY |	\
+	ACE4_READ_NAMED_ATTRS |				\
+	ACE4_WRITE_NAMED_ATTRS |			\
+	ACE4_EXECUTE |					\
+	ACE4_DELETE_CHILD |				\
+	ACE4_READ_ATTRIBUTES |				\
+	ACE4_WRITE_ATTRIBUTES |				\
+	ACE4_WRITE_RETENTION |				\
+	ACE4_WRITE_RETENTION_HOLD |			\
+	ACE4_DELETE |					\
+	ACE4_READ_ACL |					\
+	ACE4_WRITE_ACL |				\
+	ACE4_WRITE_OWNER |				\
+	ACE4_SYNCHRONIZE)
+
+/**
+ * richacl_get  -  grab another reference to a richacl handle
+ */
+static inline struct richacl *
+richacl_get(struct richacl *acl)
+{
+	if (acl)
+		atomic_inc(&acl->a_refcount);
+	return acl;
+}
+
+/**
+ * richacl_put  -  free a richacl handle
+ */
+static inline void
+richacl_put(struct richacl *acl)
+{
+	if (acl && atomic_dec_and_test(&acl->a_refcount))
+		kfree(acl);
+}
+
+/**
+ * richace_is_owner  -  check if @ace is an OWNER@ entry
+ */
+static inline int
+richace_is_owner(const struct richace *ace)
+{
+	return (ace->e_flags & ACE4_SPECIAL_WHO) &&
+	       ace->e_id == ACE_OWNER_ID;
+}
+
+/**
+ * richace_is_group  -  check if @ace is a GROUP@ entry
+ */
+static inline int
+richace_is_group(const struct richace *ace)
+{
+	return (ace->e_flags & ACE4_SPECIAL_WHO) &&
+	       ace->e_id == ACE_GROUP_ID;
+}
+
+/**
+ * richace_is_everyone  -  check if @ace is an EVERYONE@ entry
+ */
+static inline int
+richace_is_everyone(const struct richace *ace)
+{
+	return (ace->e_flags & ACE4_SPECIAL_WHO) &&
+	       ace->e_id == ACE_EVERYONE_ID;
+}
+
+/**
+ * richace_is_unix_id  -  check if @ace applies to a specific uid or gid
+ */
+static inline int
+richace_is_unix_id(const struct richace *ace)
+{
+	return !(ace->e_flags & ACE4_SPECIAL_WHO);
+}
+
+/**
+ * richace_is_inherit_only  -  check if @ace is for inheritance only
+ *
+ * ACEs with the %ACE4_INHERIT_ONLY_ACE flag set have no effect during
+ * permission checking.
+ */
+static inline int
+richace_is_inherit_only(const struct richace *ace)
+{
+	return ace->e_flags & ACE4_INHERIT_ONLY_ACE;
+}
+
+/**
+ * richace_is_inheritable  -  check if @ace is inheritable
+ */
+static inline int
+richace_is_inheritable(const struct richace *ace)
+{
+	return ace->e_flags & (ACE4_FILE_INHERIT_ACE |
+			       ACE4_DIRECTORY_INHERIT_ACE);
+}
+
+/**
+ * richace_clear_inheritance_flags  - clear all inheritance flags in @ace
+ */
+static inline void
+richace_clear_inheritance_flags(struct richace *ace)
+{
+	ace->e_flags &= ~(ACE4_FILE_INHERIT_ACE |
+			  ACE4_DIRECTORY_INHERIT_ACE |
+			  ACE4_NO_PROPAGATE_INHERIT_ACE |
+			  ACE4_INHERIT_ONLY_ACE);
+}
+
+/**
+ * richace_is_allow  -  check if @ace is an %ALLOW type entry
+ */
+static inline int
+richace_is_allow(const struct richace *ace)
+{
+	return ace->e_type == ACE4_ACCESS_ALLOWED_ACE_TYPE;
+}
+
+/**
+ * richace_is_deny  -  check if @ace is a %DENY type entry
+ */
+static inline int
+richace_is_deny(const struct richace *ace)
+{
+	return ace->e_type == ACE4_ACCESS_DENIED_ACE_TYPE;
+}
+
+extern struct richacl *richacl_alloc(int);
+extern int richace_is_same_identifier(const struct richace *,
+				      const struct richace *);
+#endif /* __RICHACL_H */
-- 
1.9.1

Re: [PATCH -V1 10/22] richacl: In-memory representation and helper functions

[Dave Chinner]

On Sun, Apr 27, 2014 at 09:44:41PM +0530, Aneesh Kumar K.V wrote:
> From: Andreas Gruenbacher <agruen@kernel.org>
> 
> A richacl consists of an NFSv4 acl and an owner, group, and other mask.
> These three masks correspond to the owner, group, and other file
> permission bits, but they contain NFSv4 permissions instead of POSIX
> permissions.
> 
> Each entry in the NFSv4 acl applies to the file owner (OWNER@), the
> owning group (GROUP@), literally everyone (EVERYONE@), or to a specific
> uid or gid.
> 
> As in the standard POSIX file permission model, each process is the
> owner, group, or other file class.  A richacl grants a requested access
> only if the NFSv4 acl in the richacl grants the access (according to the
> NFSv4 permission check algorithm), and the file mask that applies to the
> process includes the requested permissions.
> 
> Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
....
> +
> +/**
> + * richace_is_same_identifier  -  are both identifiers the same?
> + */
> +int
> +richace_is_same_identifier(const struct richace *a, const struct richace *b)
> +{
> +#define WHO_FLAGS (ACE4_SPECIAL_WHO | ACE4_IDENTIFIER_GROUP)
> +	if ((a->e_flags & WHO_FLAGS) != (b->e_flags & WHO_FLAGS))
> +		return 0;
> +	return a->e_id == b->e_id;
> +#undef WHO_FLAGS

Ugh.

....

> +#define richacl_for_each_entry(_ace, _acl) \
> +	for (_ace = _acl->a_entries; \
> +	     _ace != _acl->a_entries + _acl->a_count; \
> +	     _ace++)
> +
> +#define richacl_for_each_entry_reverse(_ace, _acl) \
> +	for (_ace = _acl->a_entries + _acl->a_count - 1; \
> +	     _ace != _acl->a_entries - 1; \
> +	     _ace--)

somewhat lacking in ()...

> +/* Flag values defined by rich-acl */
> +#define ACL4_MASKED			0x80
> +
> +#define ACL4_VALID_FLAGS (			\
> +		ACL4_MASKED)
> +
> +/* e_type values */
> +#define ACE4_ACCESS_ALLOWED_ACE_TYPE	0x0000
> +#define ACE4_ACCESS_DENIED_ACE_TYPE	0x0001
> +/*#define ACE4_SYSTEM_AUDIT_ACE_TYPE	0x0002*/
> +/*#define ACE4_SYSTEM_ALARM_ACE_TYPE	0x0003*/

What's with all the commented out types?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH -V1 10/22] richacl: In-memory representation and helper functions

[Aneesh Kumar K.V]

Dave Chinner <david@fromorbit.com> writes:

> On Sun, Apr 27, 2014 at 09:44:41PM +0530, Aneesh Kumar K.V wrote:
>> From: Andreas Gruenbacher <agruen@kernel.org>
>> 
>> A richacl consists of an NFSv4 acl and an owner, group, and other mask.
>> These three masks correspond to the owner, group, and other file
>> permission bits, but they contain NFSv4 permissions instead of POSIX
>> permissions.
>> 
>> Each entry in the NFSv4 acl applies to the file owner (OWNER@), the
>> owning group (GROUP@), literally everyone (EVERYONE@), or to a specific
>> uid or gid.
>> 
>> As in the standard POSIX file permission model, each process is the
>> owner, group, or other file class.  A richacl grants a requested access
>> only if the NFSv4 acl in the richacl grants the access (according to the
>> NFSv4 permission check algorithm), and the file mask that applies to the
>> process includes the requested permissions.
>> 
>> Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ....
>> +
>> +/**
>> + * richace_is_same_identifier  -  are both identifiers the same?
>> + */
>> +int
>> +richace_is_same_identifier(const struct richace *a, const struct richace *b)
>> +{
>> +#define WHO_FLAGS (ACE4_SPECIAL_WHO | ACE4_IDENTIFIER_GROUP)
>> +	if ((a->e_flags & WHO_FLAGS) != (b->e_flags & WHO_FLAGS))
>> +		return 0;
>> +	return a->e_id == b->e_id;
>> +#undef WHO_FLAGS
>
> Ugh.
>
> ....


Will remove.


>
>> +#define richacl_for_each_entry(_ace, _acl) \
>> +	for (_ace = _acl->a_entries; \
>> +	     _ace != _acl->a_entries + _acl->a_count; \
>> +	     _ace++)
>> +
>> +#define richacl_for_each_entry_reverse(_ace, _acl) \
>> +	for (_ace = _acl->a_entries + _acl->a_count - 1; \
>> +	     _ace != _acl->a_entries - 1; \
>> +	     _ace--)
>
> somewhat lacking in ()...


Will add

>
>> +/* Flag values defined by rich-acl */
>> +#define ACL4_MASKED			0x80
>> +
>> +#define ACL4_VALID_FLAGS (			\
>> +		ACL4_MASKED)
>> +
>> +/* e_type values */
>> +#define ACE4_ACCESS_ALLOWED_ACE_TYPE	0x0000
>> +#define ACE4_ACCESS_DENIED_ACE_TYPE	0x0001
>> +/*#define ACE4_SYSTEM_AUDIT_ACE_TYPE	0x0002*/
>> +/*#define ACE4_SYSTEM_ALARM_ACE_TYPE	0x0003*/
>
> What's with all the commented out types?
>

These are NFSv4 ACL ACE types which we are not implementing. Just left
it there for documentation.

-aneesh

Re: [PATCH -V1 10/22] richacl: In-memory representation and helper functions

[Kinglong Mee]

于 4/28/2014 00:14, Aneesh Kumar K.V 写道:
> From: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> 
> A richacl consists of an NFSv4 acl and an owner, group, and other mask.
> These three masks correspond to the owner, group, and other file
> permission bits, but they contain NFSv4 permissions instead of POSIX
> permissions.
> 
> Each entry in the NFSv4 acl applies to the file owner (OWNER@), the
> owning group (GROUP@), literally everyone (EVERYONE@), or to a specific
> uid or gid.
> 
> As in the standard POSIX file permission model, each process is the
> owner, group, or other file class.  A richacl grants a requested access
> only if the NFSv4 acl in the richacl grants the access (according to the
> NFSv4 permission check algorithm), and the file mask that applies to the
> process includes the requested permissions.
> 
> Signed-off-by: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> ---
>  fs/Makefile             |   2 +
>  fs/richacl_base.c       |  69 ++++++++++++++
>  include/linux/richacl.h | 237 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 308 insertions(+)
>  create mode 100644 fs/richacl_base.c
>  create mode 100644 include/linux/richacl.h
> 
> diff --git a/fs/Makefile b/fs/Makefile
> index f9cb9876e466..5b7ed5d022ec 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -48,6 +48,8 @@ obj-$(CONFIG_COREDUMP)		+= coredump.o
>  obj-$(CONFIG_SYSCTL)		+= drop_caches.o
>  
>  obj-$(CONFIG_FHANDLE)		+= fhandle.o
> +obj-$(CONFIG_FS_RICHACL)	+= richacl.o
> +richacl-y			:= richacl_base.o
>  
>  obj-y				+= quota/
>  
> diff --git a/fs/richacl_base.c b/fs/richacl_base.c
> new file mode 100644
> index 000000000000..689e1a6dace7
> --- /dev/null
> +++ b/fs/richacl_base.c
> @@ -0,0 +1,69 @@
> +/*
> + * Copyright (C) 2006, 2010  Novell, Inc.
> + * Written by Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> + *
> + * 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/module.h>
> +#include <linux/fs.h>
> +#include <linux/richacl.h>
> +
> +MODULE_LICENSE("GPL");
> +
> +/**
> + * richacl_alloc  -  allocate a richacl
> + * @count:	number of entries
> + */
> +struct richacl *
> +richacl_alloc(int count)
> +{
> +	size_t size = sizeof(struct richacl) + count * sizeof(struct richace);
> +	struct richacl *acl = kzalloc(size, GFP_KERNEL);
> +
> +	if (acl) {
> +		atomic_set(&acl->a_refcount, 1);
> +		acl->a_count = count;
> +	}
> +	return acl;
> +}
> +EXPORT_SYMBOL_GPL(richacl_alloc);
> +
> +/**
> + * richacl_clone  -  create a copy of a richacl
> + */
> +static struct richacl *
> +richacl_clone(const struct richacl *acl)
> +{
> +	int count = acl->a_count;
> +	size_t size = sizeof(struct richacl) + count * sizeof(struct richace);
> +	struct richacl *dup = kmalloc(size, GFP_KERNEL);
> +
> +	if (dup) {
> +		memcpy(dup, acl, size);
> +		atomic_set(&dup->a_refcount, 1);
> +	}

I'd like kmemdup as posix_acl_clone.

> +	return dup;
> +}
> +
> +/**
> + * richace_is_same_identifier  -  are both identifiers the same?
> + */
> +int
> +richace_is_same_identifier(const struct richace *a, const struct richace *b)
> +{
> +#define WHO_FLAGS (ACE4_SPECIAL_WHO | ACE4_IDENTIFIER_GROUP)
> +	if ((a->e_flags & WHO_FLAGS) != (b->e_flags & WHO_FLAGS))
> +		return 0;
> +	return a->e_id == b->e_id;
> +#undef WHO_FLAGS
> +}
> diff --git a/include/linux/richacl.h b/include/linux/richacl.h
> new file mode 100644
> index 000000000000..51d6937651f1
> --- /dev/null
> +++ b/include/linux/richacl.h
> @@ -0,0 +1,237 @@
> +/*
> + * Copyright (C) 2006, 2010  Novell, Inc.
> + * Written by Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> + *
> + * 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_H
> +#define __RICHACL_H
> +#include <linux/slab.h>
> +
> +#define ACE_OWNER_ID		130

why start from 130 ?

> +#define ACE_GROUP_ID		131
> +#define ACE_EVERYONE_ID		110

why not 132 for ACE_EVERYONE_ID?

> +
> +struct richace {
> +	unsigned short	e_type;
> +	unsigned short	e_flags;
> +	unsigned int	e_mask;
> +	unsigned int	e_id;
> +};
> +
> +struct richacl {
> +	atomic_t	a_refcount;
> +	unsigned int	a_owner_mask;
> +	unsigned int	a_group_mask;
> +	unsigned int	a_other_mask;
> +	unsigned short	a_count;
> +	unsigned short	a_flags;
> +	struct richace	a_entries[0];
> +};
> +
> +#define richacl_for_each_entry(_ace, _acl) \
> +	for (_ace = _acl->a_entries; \
> +	     _ace != _acl->a_entries + _acl->a_count; \
> +	     _ace++)
> +
> +#define richacl_for_each_entry_reverse(_ace, _acl) \
> +	for (_ace = _acl->a_entries + _acl->a_count - 1; \
> +	     _ace != _acl->a_entries - 1; \
> +	     _ace--)
> +
> +/* Flag values defined by rich-acl */
> +#define ACL4_MASKED			0x80

For rich-acl, should not using ACL4_*** or ACE4_***.

thanks,
Kinglong Mee

> +
> +#define ACL4_VALID_FLAGS (			\
> +		ACL4_MASKED)
> +
> +/* e_type values */
> +#define ACE4_ACCESS_ALLOWED_ACE_TYPE	0x0000
> +#define ACE4_ACCESS_DENIED_ACE_TYPE	0x0001
> +/*#define ACE4_SYSTEM_AUDIT_ACE_TYPE	0x0002*/
> +/*#define ACE4_SYSTEM_ALARM_ACE_TYPE	0x0003*/
> +
> +/* e_flags bitflags */
> +#define ACE4_FILE_INHERIT_ACE		0x0001
> +#define ACE4_DIRECTORY_INHERIT_ACE	0x0002
> +#define ACE4_NO_PROPAGATE_INHERIT_ACE	0x0004
> +#define ACE4_INHERIT_ONLY_ACE		0x0008
> +/*#define ACE4_SUCCESSFUL_ACCESS_ACE_FLAG	0x0010*/
> +/*#define ACE4_FAILED_ACCESS_ACE_FLAG	0x0020*/
> +#define ACE4_IDENTIFIER_GROUP		0x0040
> +/* richacl specific flag values */
> +#define ACE4_SPECIAL_WHO		0x4000
> +
> +#define ACE4_VALID_FLAGS (			\
> +	ACE4_FILE_INHERIT_ACE |			\
> +	ACE4_DIRECTORY_INHERIT_ACE |		\
> +	ACE4_NO_PROPAGATE_INHERIT_ACE |		\
> +	ACE4_INHERIT_ONLY_ACE |			\
> +	ACE4_IDENTIFIER_GROUP |			\
> +	ACE4_SPECIAL_WHO)
> +
> +/* e_mask bitflags */
> +#define ACE4_READ_DATA			0x00000001
> +#define ACE4_LIST_DIRECTORY		0x00000001
> +#define ACE4_WRITE_DATA			0x00000002
> +#define ACE4_ADD_FILE			0x00000002
> +#define ACE4_APPEND_DATA		0x00000004
> +#define ACE4_ADD_SUBDIRECTORY		0x00000004
> +#define ACE4_READ_NAMED_ATTRS		0x00000008
> +#define ACE4_WRITE_NAMED_ATTRS		0x00000010
> +#define ACE4_EXECUTE			0x00000020
> +#define ACE4_DELETE_CHILD		0x00000040
> +#define ACE4_READ_ATTRIBUTES		0x00000080
> +#define ACE4_WRITE_ATTRIBUTES		0x00000100
> +#define ACE4_WRITE_RETENTION		0x00000200
> +#define ACE4_WRITE_RETENTION_HOLD	0x00000400
> +#define ACE4_DELETE			0x00010000
> +#define ACE4_READ_ACL			0x00020000
> +#define ACE4_WRITE_ACL			0x00040000
> +#define ACE4_WRITE_OWNER		0x00080000
> +#define ACE4_SYNCHRONIZE		0x00100000
> +
> +/* Valid ACE4_* flags for directories and non-directories */
> +#define ACE4_VALID_MASK (				\
> +	ACE4_READ_DATA | ACE4_LIST_DIRECTORY |		\
> +	ACE4_WRITE_DATA | ACE4_ADD_FILE |		\
> +	ACE4_APPEND_DATA | ACE4_ADD_SUBDIRECTORY |	\
> +	ACE4_READ_NAMED_ATTRS |				\
> +	ACE4_WRITE_NAMED_ATTRS |			\
> +	ACE4_EXECUTE |					\
> +	ACE4_DELETE_CHILD |				\
> +	ACE4_READ_ATTRIBUTES |				\
> +	ACE4_WRITE_ATTRIBUTES |				\
> +	ACE4_WRITE_RETENTION |				\
> +	ACE4_WRITE_RETENTION_HOLD |			\
> +	ACE4_DELETE |					\
> +	ACE4_READ_ACL |					\
> +	ACE4_WRITE_ACL |				\
> +	ACE4_WRITE_OWNER |				\
> +	ACE4_SYNCHRONIZE)
> +
> +/**
> + * richacl_get  -  grab another reference to a richacl handle
> + */
> +static inline struct richacl *
> +richacl_get(struct richacl *acl)
> +{
> +	if (acl)
> +		atomic_inc(&acl->a_refcount);
> +	return acl;
> +}
> +
> +/**
> + * richacl_put  -  free a richacl handle
> + */
> +static inline void
> +richacl_put(struct richacl *acl)
> +{
> +	if (acl && atomic_dec_and_test(&acl->a_refcount))
> +		kfree(acl);
> +}
> +
> +/**
> + * richace_is_owner  -  check if @ace is an OWNER@ entry
> + */
> +static inline int
> +richace_is_owner(const struct richace *ace)
> +{
> +	return (ace->e_flags & ACE4_SPECIAL_WHO) &&
> +	       ace->e_id == ACE_OWNER_ID;
> +}
> +
> +/**
> + * richace_is_group  -  check if @ace is a GROUP@ entry
> + */
> +static inline int
> +richace_is_group(const struct richace *ace)
> +{
> +	return (ace->e_flags & ACE4_SPECIAL_WHO) &&
> +	       ace->e_id == ACE_GROUP_ID;
> +}
> +
> +/**
> + * richace_is_everyone  -  check if @ace is an EVERYONE@ entry
> + */
> +static inline int
> +richace_is_everyone(const struct richace *ace)
> +{
> +	return (ace->e_flags & ACE4_SPECIAL_WHO) &&
> +	       ace->e_id == ACE_EVERYONE_ID;
> +}
> +
> +/**
> + * richace_is_unix_id  -  check if @ace applies to a specific uid or gid
> + */
> +static inline int
> +richace_is_unix_id(const struct richace *ace)
> +{
> +	return !(ace->e_flags & ACE4_SPECIAL_WHO);
> +}
> +
> +/**
> + * richace_is_inherit_only  -  check if @ace is for inheritance only
> + *
> + * ACEs with the %ACE4_INHERIT_ONLY_ACE flag set have no effect during
> + * permission checking.
> + */
> +static inline int
> +richace_is_inherit_only(const struct richace *ace)
> +{
> +	return ace->e_flags & ACE4_INHERIT_ONLY_ACE;
> +}
> +
> +/**
> + * richace_is_inheritable  -  check if @ace is inheritable
> + */
> +static inline int
> +richace_is_inheritable(const struct richace *ace)
> +{
> +	return ace->e_flags & (ACE4_FILE_INHERIT_ACE |
> +			       ACE4_DIRECTORY_INHERIT_ACE);
> +}
> +
> +/**
> + * richace_clear_inheritance_flags  - clear all inheritance flags in @ace
> + */
> +static inline void
> +richace_clear_inheritance_flags(struct richace *ace)
> +{
> +	ace->e_flags &= ~(ACE4_FILE_INHERIT_ACE |
> +			  ACE4_DIRECTORY_INHERIT_ACE |
> +			  ACE4_NO_PROPAGATE_INHERIT_ACE |
> +			  ACE4_INHERIT_ONLY_ACE);
> +}
> +
> +/**
> + * richace_is_allow  -  check if @ace is an %ALLOW type entry
> + */
> +static inline int
> +richace_is_allow(const struct richace *ace)
> +{
> +	return ace->e_type == ACE4_ACCESS_ALLOWED_ACE_TYPE;
> +}
> +
> +/**
> + * richace_is_deny  -  check if @ace is a %DENY type entry
> + */
> +static inline int
> +richace_is_deny(const struct richace *ace)
> +{
> +	return ace->e_type == ACE4_ACCESS_DENIED_ACE_TYPE;
> +}
> +
> +extern struct richacl *richacl_alloc(int);
> +extern int richace_is_same_identifier(const struct richace *,
> +				      const struct richace *);
> +#endif /* __RICHACL_H */
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH -V1 11/22] richacl: Permission mapping functions

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

We need to map from POSIX permissions to NFSv4 permissions when a
chmod() is done, from NFSv4 permissions to POSIX permissions when an acl
is set (which implicitly sets the file permission bits), and from the
MAY_READ/MAY_WRITE/MAY_EXEC/MAY_APPEND flags to NFSv4 permissions when
doing an access check in a richacl.

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/richacl_base.c       | 117 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/richacl.h |  47 +++++++++++++++++++
 2 files changed, 164 insertions(+)

diff --git a/fs/richacl_base.c b/fs/richacl_base.c
index 689e1a6dace7..bca20936aed1 100644
--- a/fs/richacl_base.c
+++ b/fs/richacl_base.c
@@ -56,6 +56,123 @@ richacl_clone(const struct richacl *acl)
 }
 
 /**
+ * richacl_mask_to_mode  -  compute the file permission bits which correspond to @mask
+ * @mask:	%ACE4_* permission mask
+ *
+ * See richacl_masks_to_mode().
+ */
+static int
+richacl_mask_to_mode(unsigned int mask)
+{
+	int mode = 0;
+
+	if (mask & ACE4_POSIX_MODE_READ)
+		mode |= MAY_READ;
+	if (mask & ACE4_POSIX_MODE_WRITE)
+		mode |= MAY_WRITE;
+	if (mask & ACE4_POSIX_MODE_EXEC)
+		mode |= MAY_EXEC;
+
+	return mode;
+}
+
+/**
+ * richacl_masks_to_mode  -  compute the file permission bits from the file masks
+ *
+ * When setting a richacl, we set the file permission bits to indicate maximum
+ * permissions: for example, we set the Write permission when a mask contains
+ * ACE4_APPEND_DATA even if it does not also contain ACE4_WRITE_DATA.
+ *
+ * Permissions which are not in ACE4_POSIX_MODE_READ, ACE4_POSIX_MODE_WRITE, or
+ * ACE4_POSIX_MODE_EXEC cannot be represented in the file permission bits.
+ * Such permissions can still be effective, but not for new files or after a
+ * chmod(), and only if they were set explicitly, for example, by setting a
+ * richacl.
+ */
+int
+richacl_masks_to_mode(const struct richacl *acl)
+{
+	return richacl_mask_to_mode(acl->a_owner_mask) << 6 |
+	       richacl_mask_to_mode(acl->a_group_mask) << 3 |
+	       richacl_mask_to_mode(acl->a_other_mask);
+}
+EXPORT_SYMBOL_GPL(richacl_masks_to_mode);
+
+/**
+ * richacl_mode_to_mask  - compute a file mask from the lowest three mode bits
+ *
+ * When the file permission bits of a file are set with chmod(), this specifies
+ * the maximum permissions that processes will get.  All permissions beyond
+ * that will be removed from the file masks, and become ineffective.
+ *
+ * We also add in the permissions which are always allowed no matter what the
+ * acl says.
+ */
+unsigned int
+richacl_mode_to_mask(mode_t mode)
+{
+	unsigned int mask = ACE4_POSIX_ALWAYS_ALLOWED;
+
+	if (mode & MAY_READ)
+		mask |= ACE4_POSIX_MODE_READ;
+	if (mode & MAY_WRITE)
+		mask |= ACE4_POSIX_MODE_WRITE;
+	if (mode & MAY_EXEC)
+		mask |= ACE4_POSIX_MODE_EXEC;
+
+	return mask;
+}
+
+/**
+ * richacl_want_to_mask  - convert the iop->permission want argument to a mask
+ * @want:	@want argument of the permission inode operation
+ *
+ * When checking for append, @want is (MAY_WRITE | MAY_APPEND).
+ *
+ * Richacls use the iop->may_create and iop->may_delete hooks which are
+ * used for checking if creating and deleting files is allowed.  These hooks do
+ * not use richacl_want_to_mask(), so we do not have to deal with mapping
+ * MAY_WRITE to ACE4_ADD_FILE, ACE4_ADD_SUBDIRECTORY, and ACE4_DELETE_CHILD
+ * here.
+ */
+unsigned int
+richacl_want_to_mask(unsigned int want)
+{
+	unsigned int mask = 0;
+
+	if (want & MAY_READ)
+		mask |= ACE4_READ_DATA;
+	if (want & MAY_DELETE_SELF)
+		mask |= ACE4_DELETE;
+	if (want & MAY_TAKE_OWNERSHIP)
+		mask |= ACE4_WRITE_OWNER;
+	if (want & MAY_CHMOD)
+		mask |= ACE4_WRITE_ACL;
+	if (want & MAY_SET_TIMES)
+		mask |= ACE4_WRITE_ATTRIBUTES;
+	if (want & MAY_EXEC)
+		mask |= ACE4_EXECUTE;
+	/*
+	 * differentiate MAY_WRITE from these request
+	 */
+	if (want & (MAY_APPEND |
+		    MAY_CREATE_FILE | MAY_CREATE_DIR |
+		    MAY_DELETE_CHILD)) {
+		if (want & MAY_APPEND)
+			mask |= ACE4_APPEND_DATA;
+		if (want & MAY_CREATE_FILE)
+			mask |= ACE4_ADD_FILE;
+		if (want & MAY_CREATE_DIR)
+			mask |= ACE4_ADD_SUBDIRECTORY;
+		if (want & MAY_DELETE_CHILD)
+			mask |= ACE4_DELETE_CHILD;
+	} else if (want & MAY_WRITE)
+		mask |= ACE4_WRITE_DATA;
+	return mask;
+}
+EXPORT_SYMBOL_GPL(richacl_want_to_mask);
+
+/**
  * richace_is_same_identifier  -  are both identifiers the same?
  */
 int
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index 51d6937651f1..fe875fe24636 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -119,6 +119,49 @@ struct richacl {
 	ACE4_WRITE_OWNER |				\
 	ACE4_SYNCHRONIZE)
 
+/*
+ * The POSIX permissions are supersets of the following NFSv4 permissions:
+ *
+ *  - MAY_READ maps to READ_DATA or LIST_DIRECTORY, depending on the type
+ *    of the file system object.
+ *
+ *  - MAY_WRITE maps to WRITE_DATA or ACE4_APPEND_DATA for files, and to
+ *    ADD_FILE, ACE4_ADD_SUBDIRECTORY, or ACE4_DELETE_CHILD for directories.
+ *
+ *  - MAY_EXECUTE maps to ACE4_EXECUTE.
+ *
+ *  (Some of these NFSv4 permissions have the same bit values.)
+ */
+#define ACE4_POSIX_MODE_READ (			\
+		ACE4_READ_DATA |		\
+		ACE4_LIST_DIRECTORY)
+#define ACE4_POSIX_MODE_WRITE (			\
+		ACE4_WRITE_DATA |		\
+		ACE4_ADD_FILE |			\
+		ACE4_APPEND_DATA |		\
+		ACE4_ADD_SUBDIRECTORY |		\
+		ACE4_DELETE_CHILD)
+#define ACE4_POSIX_MODE_EXEC ACE4_EXECUTE
+#define ACE4_POSIX_MODE_ALL (			\
+		ACE4_POSIX_MODE_READ |		\
+		ACE4_POSIX_MODE_WRITE |		\
+		ACE4_POSIX_MODE_EXEC)
+/*
+ * These permissions are always allowed
+ * no matter what the acl says.
+ */
+#define ACE4_POSIX_ALWAYS_ALLOWED (	\
+		ACE4_SYNCHRONIZE |	\
+		ACE4_READ_ATTRIBUTES |	\
+		ACE4_READ_ACL)
+/*
+ * The owner is implicitly granted
+ * these permissions under POSIX.
+ */
+#define ACE4_POSIX_OWNER_ALLOWED (		\
+		ACE4_WRITE_ATTRIBUTES |		\
+		ACE4_WRITE_OWNER |		\
+		ACE4_WRITE_ACL)
 /**
  * richacl_get  -  grab another reference to a richacl handle
  */
@@ -234,4 +277,8 @@ richace_is_deny(const struct richace *ace)
 extern struct richacl *richacl_alloc(int);
 extern int richace_is_same_identifier(const struct richace *,
 				      const struct richace *);
+extern int richacl_masks_to_mode(const struct richacl *);
+extern unsigned int richacl_mode_to_mask(mode_t);
+extern unsigned int richacl_want_to_mask(unsigned int);
+
 #endif /* __RICHACL_H */
-- 
1.9.1

[PATCH -V1 12/22] richacl: Compute maximum file masks from an acl

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

Compute upper bound owner, group, and other file masks with as few
permissions as possible without denying any permissions that the NFSv4
acl in a richacl grants.

This algorithm is used when a file inherits an acl at create time and
when an acl is set via a mechanism that does not specify file modes
(such as via nfsd).  When user-space sets an acl, the file masks are
passed in as part of the xattr.

When setting a richacl, the file masks determine what the file
permission bits will be set to; see richacl_masks_to_mode().

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/richacl_base.c       | 128 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/richacl.h |   1 +
 2 files changed, 129 insertions(+)

diff --git a/fs/richacl_base.c b/fs/richacl_base.c
index bca20936aed1..d73f1b0a9c5f 100644
--- a/fs/richacl_base.c
+++ b/fs/richacl_base.c
@@ -184,3 +184,131 @@ richace_is_same_identifier(const struct richace *a, const struct richace *b)
 	return a->e_id == b->e_id;
 #undef WHO_FLAGS
 }
+
+/**
+ * richacl_allowed_to_who  -  mask flags allowed to a specific who value
+ *
+ * Computes the mask values allowed to a specific who value, taking
+ * EVERYONE@ entries into account.
+ */
+static unsigned int richacl_allowed_to_who(struct richacl *acl,
+					   struct richace *who)
+{
+	struct richace *ace;
+	unsigned int allowed = 0;
+
+	richacl_for_each_entry_reverse(ace, acl) {
+		if (richace_is_inherit_only(ace))
+			continue;
+		if (richace_is_same_identifier(ace, who) ||
+		    richace_is_everyone(ace)) {
+			if (richace_is_allow(ace))
+				allowed |= ace->e_mask;
+			else if (richace_is_deny(ace))
+				allowed &= ~ace->e_mask;
+		}
+	}
+	return allowed;
+}
+
+/**
+ * richacl_group_class_allowed  -  maximum permissions the group class is allowed
+ *
+ * See richacl_compute_max_masks().
+ */
+static unsigned int richacl_group_class_allowed(struct richacl *acl)
+{
+	struct richace *ace;
+	unsigned int everyone_allowed = 0, group_class_allowed = 0;
+	int had_group_ace = 0;
+
+	richacl_for_each_entry_reverse(ace, acl) {
+		if (richace_is_inherit_only(ace) ||
+		    richace_is_owner(ace))
+			continue;
+
+		if (richace_is_everyone(ace)) {
+			if (richace_is_allow(ace))
+				everyone_allowed |= ace->e_mask;
+			else if (richace_is_deny(ace))
+				everyone_allowed &= ~ace->e_mask;
+		} else {
+			group_class_allowed |=
+				richacl_allowed_to_who(acl, ace);
+
+			if (richace_is_group(ace))
+				had_group_ace = 1;
+		}
+	}
+	if (!had_group_ace)
+		group_class_allowed |= everyone_allowed;
+	return group_class_allowed;
+}
+
+/**
+ * richacl_compute_max_masks  -  compute upper bound masks
+ *
+ * Computes upper bound owner, group, and other masks so that none of
+ * the mask flags allowed by the acl are disabled (for any choice of the
+ * file owner or group membership).
+ */
+void richacl_compute_max_masks(struct richacl *acl)
+{
+	unsigned int gmask = ~0;
+	struct richace *ace;
+
+	/*
+	 * @gmask contains all permissions which the group class is ever
+	 * allowed.  We use it to avoid adding permissions to the group mask
+	 * from everyone@ allow aces which the group class is always denied
+	 * through other aces.  For example, the following acl would otherwise
+	 * result in a group mask or rw:
+	 *
+	 *	group@:w::deny
+	 *	everyone@:rw::allow
+	 *
+	 * Avoid computing @gmask for acls which do not include any group class
+	 * deny aces: in such acls, the group class is never denied any
+	 * permissions from everyone@ allow aces.
+	 */
+
+restart:
+	acl->a_owner_mask = 0;
+	acl->a_group_mask = 0;
+	acl->a_other_mask = 0;
+
+	richacl_for_each_entry_reverse(ace, acl) {
+		if (richace_is_inherit_only(ace))
+			continue;
+
+		if (richace_is_owner(ace)) {
+			if (richace_is_allow(ace))
+				acl->a_owner_mask |= ace->e_mask;
+			else if (richace_is_deny(ace))
+				acl->a_owner_mask &= ~ace->e_mask;
+		} else if (richace_is_everyone(ace)) {
+			if (richace_is_allow(ace)) {
+				acl->a_owner_mask |= ace->e_mask;
+				acl->a_group_mask |= ace->e_mask & gmask;
+				acl->a_other_mask |= ace->e_mask;
+			} else if (richace_is_deny(ace)) {
+				acl->a_owner_mask &= ~ace->e_mask;
+				acl->a_group_mask &= ~ace->e_mask;
+				acl->a_other_mask &= ~ace->e_mask;
+			}
+		} else {
+			if (richace_is_allow(ace)) {
+				acl->a_owner_mask |= ace->e_mask & gmask;
+				acl->a_group_mask |= ace->e_mask & gmask;
+			} else if (richace_is_deny(ace) && gmask == ~0) {
+				gmask = richacl_group_class_allowed(acl);
+				if (likely(gmask != ~0))
+					/* should always be true */
+					goto restart;
+			}
+		}
+	}
+
+	acl->a_flags &= ~ACL4_MASKED;
+}
+EXPORT_SYMBOL_GPL(richacl_compute_max_masks);
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index fe875fe24636..14f18b597be3 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -280,5 +280,6 @@ extern int richace_is_same_identifier(const struct richace *,
 extern int richacl_masks_to_mode(const struct richacl *);
 extern unsigned int richacl_mode_to_mask(mode_t);
 extern unsigned int richacl_want_to_mask(unsigned int);
+extern void richacl_compute_max_masks(struct richacl *);
 
 #endif /* __RICHACL_H */
-- 
1.9.1

[PATCH -V1 14/22] richacl: Permission check algorithm

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

As in the standard POSIX file permission model, each process is the
owner, group, or other file class.  A process is

  - in the owner file class if it owns the file,
  - in the group file class if it is in the file's owning group or it
    matches any of the user or group entries, and
  - in the other file class otherwise.

Each file class is associated with a file mask.

A richacl grants a requested access if the NFSv4 acl in the richacl
grants the requested permissions (according to the NFSv4 permission
check algorithm) and the file mask that applies to the process includes
the requested permissions.

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/richacl_base.c       | 102 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/richacl.h |   2 +
 2 files changed, 104 insertions(+)

diff --git a/fs/richacl_base.c b/fs/richacl_base.c
index 41ff82d10d4d..baa61da691a4 100644
--- a/fs/richacl_base.c
+++ b/fs/richacl_base.c
@@ -352,3 +352,105 @@ richacl_chmod(struct richacl *acl, mode_t mode)
 	return clone;
 }
 EXPORT_SYMBOL_GPL(richacl_chmod);
+
+/**
+ * richacl_permission  -  richacl permission check algorithm
+ * @inode:	inode to check
+ * @acl:	rich acl of the inode
+ * @mask:	requested access (ACE4_* bitmask)
+ *
+ * Checks if the current process is granted @mask flags in @acl.
+ */
+int
+richacl_permission(struct inode *inode, const struct richacl *acl,
+		   unsigned int mask)
+{
+	const struct richace *ace;
+	unsigned int requested = mask, denied = 0;
+	int in_owning_group = in_group_p(inode->i_gid);
+	int in_owner_or_group_class = in_owning_group;
+
+	/*
+	 * We don't need to know which class the process is in when the acl is
+	 * not masked.
+	 */
+	if (!(acl->a_flags & ACL4_MASKED))
+		in_owner_or_group_class = 1;
+
+	/*
+	 * A process is
+	 *   - in the owner file class if it owns the file,
+	 *   - in the group file class if it is in the file's owning group or
+	 *     it matches any of the user or group entries, and
+	 *   - in the other file class otherwise.
+	 */
+
+	/*
+	 * Check if the acl grants the requested access and determine which
+	 * file class the process is in.
+	 */
+	richacl_for_each_entry(ace, acl) {
+		unsigned int ace_mask = ace->e_mask;
+
+		if (richace_is_inherit_only(ace))
+			continue;
+		if (richace_is_owner(ace)) {
+			if (!uid_eq(current_fsuid(), inode->i_uid))
+				continue;
+			goto is_owner;
+		} else if (richace_is_group(ace)) {
+			if (!in_owning_group)
+				continue;
+		} else if (richace_is_unix_id(ace)) {
+			if (ace->e_flags & ACE4_IDENTIFIER_GROUP) {
+				if (!in_group_p(make_kgid(current_user_ns(),
+							  ace->e_id)))
+					continue;
+			} else {
+				if (!uid_eq(current_fsuid(),
+					    make_kuid(current_user_ns(),
+						     ace->e_id)))
+					continue;
+			}
+		} else
+			goto is_everyone;
+
+is_owner:
+		/* The process is in the owner or group file class. */
+		in_owner_or_group_class = 1;
+
+is_everyone:
+		/* Check which mask flags the ACE allows or denies. */
+		if (richace_is_deny(ace))
+			denied |= ace_mask & mask;
+		mask &= ~ace_mask;
+
+		/*
+		 * Keep going until we know which file class
+		 * the process is in.
+		 */
+		if (!mask && in_owner_or_group_class)
+			break;
+	}
+	denied |= mask;
+
+	if (acl->a_flags & ACL4_MASKED) {
+		unsigned int file_mask;
+
+		/*
+		 * The file class a process is in determines which file mask
+		 * applies.  Check if that file mask also grants the requested
+		 * access.
+		 */
+		if (uid_eq(current_fsuid(), inode->i_uid))
+			file_mask = acl->a_owner_mask;
+		else if (in_owner_or_group_class)
+			file_mask = acl->a_group_mask;
+		else
+			file_mask = acl->a_other_mask;
+		denied |= requested & ~file_mask;
+	}
+
+	return denied ? -EACCES : 0;
+}
+EXPORT_SYMBOL_GPL(richacl_permission);
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index a1ff8a38409c..d43700a179d3 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -282,5 +282,7 @@ extern unsigned int richacl_mode_to_mask(mode_t);
 extern unsigned int richacl_want_to_mask(unsigned int);
 extern void richacl_compute_max_masks(struct richacl *);
 extern struct richacl *richacl_chmod(struct richacl *, mode_t);
+extern int richacl_permission(struct inode *, const struct richacl *,
+			      unsigned int);
 
 #endif /* __RICHACL_H */
-- 
1.9.1

[PATCH -V1 15/22] richacl: Create-time inheritance

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

When a new file is created, it can inherit an acl from its parent
directory; this is similar to how default acls work in POSIX (draft)
ACLs.

As with POSIX ACLs, if a file inherits an acl from its parent directory,
the intersection between the create mode and the permissions granted by
the inherited acl determines the file masks and file permission bits,
and the umask is ignored.

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/Makefile             |  2 +-
 fs/richacl_base.c       | 69 +++++++++++++++++++++++++++++++++++++++++++++++++
 fs/richacl_inode.c      | 59 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/richacl.h |  4 +++
 4 files changed, 133 insertions(+), 1 deletion(-)
 create mode 100644 fs/richacl_inode.c

diff --git a/fs/Makefile b/fs/Makefile
index 5b7ed5d022ec..bed5b8a623ba 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -49,7 +49,7 @@ obj-$(CONFIG_SYSCTL)		+= drop_caches.o
 
 obj-$(CONFIG_FHANDLE)		+= fhandle.o
 obj-$(CONFIG_FS_RICHACL)	+= richacl.o
-richacl-y			:= richacl_base.o
+richacl-y			:= richacl_base.o richacl_inode.o
 
 obj-y				+= quota/
 
diff --git a/fs/richacl_base.c b/fs/richacl_base.c
index baa61da691a4..29ffea8e56d7 100644
--- a/fs/richacl_base.c
+++ b/fs/richacl_base.c
@@ -454,3 +454,72 @@ is_everyone:
 	return denied ? -EACCES : 0;
 }
 EXPORT_SYMBOL_GPL(richacl_permission);
+
+/**
+ * richacl_inherit  -  compute the inherited acl of a new file
+ * @dir_acl:	acl of the containing directory
+ * @isdir:	inherit by a directory or non-directory?
+ *
+ * A directory can have acl entries which files and/or directories created
+ * inside the directory will inherit.  This function computes the acl for such
+ * a new file.  If there is no inheritable acl, it will return %NULL.
+ */
+struct richacl *
+richacl_inherit(const struct richacl *dir_acl, int isdir)
+{
+	const struct richace *dir_ace;
+	struct richacl *acl = NULL;
+	struct richace *ace;
+	int count = 0;
+
+	if (isdir) {
+		richacl_for_each_entry(dir_ace, dir_acl) {
+			if (!richace_is_inheritable(dir_ace))
+				continue;
+			count++;
+		}
+		if (!count)
+			return NULL;
+		acl = richacl_alloc(count);
+		if (!acl)
+			return ERR_PTR(-ENOMEM);
+		ace = acl->a_entries;
+		richacl_for_each_entry(dir_ace, dir_acl) {
+			if (!richace_is_inheritable(dir_ace))
+				continue;
+			memcpy(ace, dir_ace, sizeof(struct richace));
+			if (dir_ace->e_flags & ACE4_NO_PROPAGATE_INHERIT_ACE)
+				richace_clear_inheritance_flags(ace);
+			if ((dir_ace->e_flags & ACE4_FILE_INHERIT_ACE) &&
+			    !(dir_ace->e_flags & ACE4_DIRECTORY_INHERIT_ACE))
+				ace->e_flags |= ACE4_INHERIT_ONLY_ACE;
+			ace++;
+		}
+	} else {
+		richacl_for_each_entry(dir_ace, dir_acl) {
+			if (!(dir_ace->e_flags & ACE4_FILE_INHERIT_ACE))
+				continue;
+			count++;
+		}
+		if (!count)
+			return NULL;
+		acl = richacl_alloc(count);
+		if (!acl)
+			return ERR_PTR(-ENOMEM);
+		ace = acl->a_entries;
+		richacl_for_each_entry(dir_ace, dir_acl) {
+			if (!(dir_ace->e_flags & ACE4_FILE_INHERIT_ACE))
+				continue;
+			memcpy(ace, dir_ace, sizeof(struct richace));
+			richace_clear_inheritance_flags(ace);
+			/*
+			 * ACE4_DELETE_CHILD is meaningless for
+			 * non-directories, so clear it.
+			 */
+			ace->e_mask &= ~ACE4_DELETE_CHILD;
+			ace++;
+		}
+	}
+
+	return acl;
+}
diff --git a/fs/richacl_inode.c b/fs/richacl_inode.c
new file mode 100644
index 000000000000..f590fb526812
--- /dev/null
+++ b/fs/richacl_inode.c
@@ -0,0 +1,59 @@
+/*
+ * Copyright (C) 2010  Novell, Inc.
+ * Written by Andreas Gruenbacher <agruen@kernel.org>
+ *
+ * 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/module.h>
+#include <linux/fs.h>
+#include <linux/richacl.h>
+
+/**
+ * richacl_inherit_inode  -  compute inherited acl and file mode
+ * @dir_acl:	acl of the containing directory
+ * @inode:	inode of the new file (create mode in i_mode)
+ *
+ * The file permission bits in inode->i_mode must be set to the create mode.
+ * If there is an inheritable acl, the maximum permissions that the acl grants
+ * will be computed and permissions not granted by the acl will be removed from
+ * inode->i_mode.  If there is no inheritable acl, the umask will be applied
+ * instead.
+ */
+struct richacl *
+richacl_inherit_inode(const struct richacl *dir_acl, struct inode *inode)
+{
+	struct richacl *acl;
+	mode_t mask;
+
+	acl = richacl_inherit(dir_acl, S_ISDIR(inode->i_mode));
+	if (acl) {
+
+		richacl_compute_max_masks(acl);
+
+		/*
+		 * Ensure that the acl will not grant any permissions beyond
+		 * the create mode.
+		 */
+		acl->a_flags |= ACL4_MASKED;
+		acl->a_owner_mask &= richacl_mode_to_mask(inode->i_mode >> 6) |
+				     ACE4_POSIX_OWNER_ALLOWED;
+		acl->a_group_mask &= richacl_mode_to_mask(inode->i_mode >> 3);
+		acl->a_other_mask &= richacl_mode_to_mask(inode->i_mode);
+		mask = ~S_IRWXUGO | richacl_masks_to_mode(acl);
+	} else
+		mask = ~current_umask();
+
+	inode->i_mode &= mask;
+	return acl;
+}
+EXPORT_SYMBOL_GPL(richacl_inherit_inode);
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index d43700a179d3..47855ec8db4d 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -284,5 +284,9 @@ extern void richacl_compute_max_masks(struct richacl *);
 extern struct richacl *richacl_chmod(struct richacl *, mode_t);
 extern int richacl_permission(struct inode *, const struct richacl *,
 			      unsigned int);
+extern struct richacl *richacl_inherit(const struct richacl *, int);
 
+/* richacl_inode.c */
+extern struct richacl *richacl_inherit_inode(const struct richacl *,
+					     struct inode *);
 #endif /* __RICHACL_H */
-- 
1.9.1

[PATCH -V1 16/22] richacl: Check if an acl is equivalent to a file mode

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

This function is used to avoid storing richacls on disk if the acl can
be computed from the file permission bits.

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/richacl_base.c       | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/richacl.h |  1 +
 2 files changed, 55 insertions(+)

diff --git a/fs/richacl_base.c b/fs/richacl_base.c
index 29ffea8e56d7..5d777f6b8a61 100644
--- a/fs/richacl_base.c
+++ b/fs/richacl_base.c
@@ -523,3 +523,57 @@ richacl_inherit(const struct richacl *dir_acl, int isdir)
 
 	return acl;
 }
+
+/**
+ * richacl_equiv_mode  -  check if @acl is equivalent to file permission bits
+ * @mode_p:	the file mode (including the file type)
+ *
+ * If @acl can be fully represented by file permission bits, this function
+ * returns 0, and the file permission bits in @mode_p are set to the equivalent
+ * of @acl.
+ *
+ * This function is used to avoid storing richacls on disk if the acl can be
+ * computed from the file permission bits.  It allows user-space to make sure
+ * that a file has no explicit richacl set.
+ */
+int
+richacl_equiv_mode(const struct richacl *acl, mode_t *mode_p)
+{
+	const struct richace *ace = acl->a_entries;
+	unsigned int x;
+	mode_t mode;
+
+	if (acl->a_count != 1 ||
+	    acl->a_flags != ACL4_MASKED ||
+	    !richace_is_everyone(ace) ||
+	    !richace_is_allow(ace) ||
+	    ace->e_flags & ~ACE4_SPECIAL_WHO)
+		return -1;
+
+	/*
+	 * Figure out the permissions we care about: ACE4_DELETE_CHILD is
+	 * meaningless for non-directories, so we ignore it.
+	 */
+	x = ~ACE4_POSIX_ALWAYS_ALLOWED;
+	if (!S_ISDIR(*mode_p))
+		x &= ~ACE4_DELETE_CHILD;
+
+	mode = richacl_masks_to_mode(acl);
+	if ((acl->a_group_mask & x) != (richacl_mode_to_mask(mode >> 3) & x) ||
+	    (acl->a_other_mask & x) != (richacl_mode_to_mask(mode) & x))
+		return -1;
+
+	/*
+	 * Ignore permissions which the owner is always allowed.
+	 */
+	x &= ~ACE4_POSIX_OWNER_ALLOWED;
+	if ((acl->a_owner_mask & x) != (richacl_mode_to_mask(mode >> 6) & x))
+		return -1;
+
+	if ((ace->e_mask & x) != (ACE4_POSIX_MODE_ALL & x))
+		return -1;
+
+	*mode_p = (*mode_p & ~S_IRWXUGO) | mode;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(richacl_equiv_mode);
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index 47855ec8db4d..7ac6587cb166 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -285,6 +285,7 @@ extern struct richacl *richacl_chmod(struct richacl *, mode_t);
 extern int richacl_permission(struct inode *, const struct richacl *,
 			      unsigned int);
 extern struct richacl *richacl_inherit(const struct richacl *, int);
+extern int richacl_equiv_mode(const struct richacl *, mode_t *);
 
 /* richacl_inode.c */
 extern struct richacl *richacl_inherit_inode(const struct richacl *,
-- 
1.9.1

[PATCH -V1 17/22] richacl: Automatic Inheritance

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

Automatic Inheritance (AI) allows changes to the acl of a directory to
recursively propagate down to files and directories in the directory.

To implement this, the kernel keeps track of which permissions have been
inherited, and makes sure that permission propagation is turned off when
the file permission bits of a file are changed (upon create or chmod).

The actual permission propagation is implemented in user space.

AI works as follows:

 - When the ACL4_AUTO_INHERIT flag in the acl of a file is cleared, the
   file is not affected by AI.

 - When the ACL4_AUTO_INHERIT flag in the acl of a directory is set and
   a file or subdirectory is created in that directory, files created in
   the directory will have the ACL4_AUTO_INHERIT flag set, and all
   inherited aces will have the ACE4_INHERITED_ACE flag set.  This
   allows user space to distinguish between aces which have been
   inherited, and aces which have been explicitly added.

 - When the ACL4_PROTECTED acl flag in the acl of a file is set, AI will
   not modify the acl of the file.  This does not affect propagation of
   permissions from the file to its children (if the file is a
   directory).

Linux does not have a way of creating files without setting the file
permission bits, so all files created inside a directory with
ACL4_AUTO_INHERIT set will also have the ACL4_PROTECTED flag set.  This
effectively disables AI.

Protocols which support creating files without specifying permissions
can explicitly clear the ACL4_PROTECTED flag after creating a file (and
reset the file masks to "undo" applying the create mode; see
richacl_compute_max_masks()).  This is a workaround; a per-create or
per-process flag indicating to ignore the create mode when AI is in
effect would fix this problem.

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/richacl_base.c       | 10 +++++++++-
 fs/richacl_inode.c      |  7 ++++++-
 include/linux/richacl.h | 23 ++++++++++++++++++++++-
 3 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/fs/richacl_base.c b/fs/richacl_base.c
index 5d777f6b8a61..d3beaacf6b9b 100644
--- a/fs/richacl_base.c
+++ b/fs/richacl_base.c
@@ -336,7 +336,8 @@ richacl_chmod(struct richacl *acl, mode_t mode)
 	if (acl->a_owner_mask == owner_mask &&
 	    acl->a_group_mask == group_mask &&
 	    acl->a_other_mask == other_mask &&
-	    (acl->a_flags & ACL4_MASKED))
+	    (acl->a_flags & ACL4_MASKED) &&
+	    (!richacl_is_auto_inherit(acl) || richacl_is_protected(acl)))
 		return acl;
 
 	clone = richacl_clone(acl);
@@ -348,6 +349,8 @@ richacl_chmod(struct richacl *acl, mode_t mode)
 	clone->a_owner_mask = owner_mask;
 	clone->a_group_mask = group_mask;
 	clone->a_other_mask = other_mask;
+	if (richacl_is_auto_inherit(clone))
+		clone->a_flags |= ACL4_PROTECTED;
 
 	return clone;
 }
@@ -520,6 +523,11 @@ richacl_inherit(const struct richacl *dir_acl, int isdir)
 			ace++;
 		}
 	}
+	if (richacl_is_auto_inherit(dir_acl)) {
+		acl->a_flags = ACL4_AUTO_INHERIT;
+		richacl_for_each_entry(ace, acl)
+			ace->e_flags |= ACE4_INHERITED_ACE;
+	}
 
 	return acl;
 }
diff --git a/fs/richacl_inode.c b/fs/richacl_inode.c
index f590fb526812..c1297ad909dc 100644
--- a/fs/richacl_inode.c
+++ b/fs/richacl_inode.c
@@ -37,9 +37,14 @@ richacl_inherit_inode(const struct richacl *dir_acl, struct inode *inode)
 
 	acl = richacl_inherit(dir_acl, S_ISDIR(inode->i_mode));
 	if (acl) {
+		/*
+		 * We need to set ACL4_PROTECTED because we are
+		 * doing an implicit chmod
+		 */
+		if (richacl_is_auto_inherit(acl))
+			acl->a_flags |= ACL4_PROTECTED;
 
 		richacl_compute_max_masks(acl);
-
 		/*
 		 * Ensure that the acl will not grant any permissions beyond
 		 * the create mode.
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index 7ac6587cb166..88f95d78b897 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -48,10 +48,16 @@ struct richacl {
 	     _ace != _acl->a_entries - 1; \
 	     _ace--)
 
+/* a_flags values */
+#define ACL4_AUTO_INHERIT		0x01
+#define ACL4_PROTECTED			0x02
+/* #define ACL4_DEFAULTED			0x04 */
 /* Flag values defined by rich-acl */
 #define ACL4_MASKED			0x80
 
 #define ACL4_VALID_FLAGS (			\
+		ACL4_AUTO_INHERIT |		\
+		ACL4_PROTECTED |		\
 		ACL4_MASKED)
 
 /* e_type values */
@@ -68,6 +74,7 @@ struct richacl {
 /*#define ACE4_SUCCESSFUL_ACCESS_ACE_FLAG	0x0010*/
 /*#define ACE4_FAILED_ACCESS_ACE_FLAG	0x0020*/
 #define ACE4_IDENTIFIER_GROUP		0x0040
+#define ACE4_INHERITED_ACE		0x0080
 /* richacl specific flag values */
 #define ACE4_SPECIAL_WHO		0x4000
 
@@ -77,6 +84,7 @@ struct richacl {
 	ACE4_NO_PROPAGATE_INHERIT_ACE |		\
 	ACE4_INHERIT_ONLY_ACE |			\
 	ACE4_IDENTIFIER_GROUP |			\
+	ACE4_INHERITED_ACE |			\
 	ACE4_SPECIAL_WHO)
 
 /* e_mask bitflags */
@@ -183,6 +191,18 @@ richacl_put(struct richacl *acl)
 		kfree(acl);
 }
 
+static inline int
+richacl_is_auto_inherit(const struct richacl *acl)
+{
+	return acl->a_flags & ACL4_AUTO_INHERIT;
+}
+
+static inline int
+richacl_is_protected(const struct richacl *acl)
+{
+	return acl->a_flags & ACL4_PROTECTED;
+}
+
 /**
  * richace_is_owner  -  check if @ace is an OWNER@ entry
  */
@@ -253,7 +273,8 @@ richace_clear_inheritance_flags(struct richace *ace)
 	ace->e_flags &= ~(ACE4_FILE_INHERIT_ACE |
 			  ACE4_DIRECTORY_INHERIT_ACE |
 			  ACE4_NO_PROPAGATE_INHERIT_ACE |
-			  ACE4_INHERIT_ONLY_ACE);
+			  ACE4_INHERIT_ONLY_ACE |
+			  ACE4_INHERITED_ACE);
 }
 
 /**
-- 
1.9.1

[PATCH -V1 18/22] richacl: xattr mapping functions

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen@kernel.org>

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

Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/Makefile                   |   2 +-
 fs/richacl_xattr.c            | 130 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/richacl_xattr.h |  46 +++++++++++++++
 3 files changed, 177 insertions(+), 1 deletion(-)
 create mode 100644 fs/richacl_xattr.c
 create mode 100644 include/linux/richacl_xattr.h

diff --git a/fs/Makefile b/fs/Makefile
index bed5b8a623ba..6cb7e064189d 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -49,7 +49,7 @@ obj-$(CONFIG_SYSCTL)		+= drop_caches.o
 
 obj-$(CONFIG_FHANDLE)		+= fhandle.o
 obj-$(CONFIG_FS_RICHACL)	+= richacl.o
-richacl-y			:= richacl_base.o richacl_inode.o
+richacl-y			:= richacl_base.o richacl_inode.o richacl_xattr.o
 
 obj-y				+= quota/
 
diff --git a/fs/richacl_xattr.c b/fs/richacl_xattr.c
new file mode 100644
index 000000000000..e7a25e173275
--- /dev/null
+++ b/fs/richacl_xattr.c
@@ -0,0 +1,130 @@
+/*
+ * Copyright (C) 2006, 2010  Novell, Inc.
+ * Written by Andreas Gruenbacher <agruen@kernel.org>
+ *
+ * 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/kernel.h>
+#include <linux/fs.h>
+#include <linux/slab.h>
+#include <linux/module.h>
+#include <linux/richacl_xattr.h>
+
+MODULE_LICENSE("GPL");
+
+/**
+ * richacl_from_xattr  -  convert a richacl xattr into the in-memory representation
+ */
+struct richacl *
+richacl_from_xattr(const void *value, size_t size)
+{
+	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(struct richacl_xattr) ||
+	    xattr_acl->a_version != ACL4_XATTR_VERSION ||
+	    (xattr_acl->a_flags & ~ACL4_VALID_FLAGS))
+		return ERR_PTR(-EINVAL);
+
+	count = le16_to_cpu(xattr_acl->a_count);
+	if (count > ACL4_XATTR_MAX_COUNT)
+		return ERR_PTR(-EINVAL);
+
+	acl = richacl_alloc(count);
+	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 & ~ACE4_VALID_MASK)
+		goto fail_einval;
+	acl->a_group_mask = le32_to_cpu(xattr_acl->a_group_mask);
+	if (acl->a_group_mask & ~ACE4_VALID_MASK)
+		goto fail_einval;
+	acl->a_other_mask = le32_to_cpu(xattr_acl->a_other_mask);
+	if (acl->a_other_mask & ~ACE4_VALID_MASK)
+		goto fail_einval;
+
+	if (((void *)xattr_ace + count * sizeof(*xattr_ace)) > (value + size))
+		goto fail_einval;
+
+	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);
+		ace->e_id    = le32_to_cpu(xattr_ace->e_id);
+
+		if (ace->e_flags & ~ACE4_VALID_FLAGS)
+			goto fail_einval;
+		if (ace->e_type > ACE4_ACCESS_DENIED_ACE_TYPE ||
+		    (ace->e_mask & ~ACE4_VALID_MASK))
+			goto fail_einval;
+
+		xattr_ace++;
+	}
+
+	return acl;
+
+fail_einval:
+	richacl_put(acl);
+	return ERR_PTR(-EINVAL);
+}
+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 of size richacl_xattr_size(@acl) for the result
+ */
+void
+richacl_to_xattr(const struct richacl *acl, void *buffer)
+{
+	struct richacl_xattr *xattr_acl = buffer;
+	struct richace_xattr *xattr_ace;
+	const struct richace *ace;
+
+	xattr_acl->a_version = ACL4_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 &
+						 ACE4_VALID_FLAGS);
+		xattr_ace->e_mask = cpu_to_le32(ace->e_mask);
+		xattr_ace->e_id = cpu_to_le32(ace->e_id);
+		xattr_ace++;
+	}
+}
+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 000000000000..792abccce47b
--- /dev/null
+++ b/include/linux/richacl_xattr.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2006, 2010  Novell, Inc.
+ * Written by Andreas Gruenbacher <agruen@kernel.org>
+ *
+ * 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 <linux/richacl.h>
+
+#define RICHACL_XATTR "system.richacl"
+
+struct richace_xattr {
+	__le16		e_type;
+	__le16		e_flags;
+	__le32		e_mask;
+	__le32		e_id;
+};
+
+struct richacl_xattr {
+	unsigned char	a_version;
+	unsigned char	a_flags;
+	__le16		a_count;
+	__le32		a_owner_mask;
+	__le32		a_group_mask;
+	__le32		a_other_mask;
+};
+
+#define ACL4_XATTR_VERSION	0
+#define ACL4_XATTR_MAX_COUNT	1024
+
+extern struct richacl *richacl_from_xattr(const void *, size_t);
+extern size_t richacl_xattr_size(const struct richacl *acl);
+extern void richacl_to_xattr(const struct richacl *, void *);
+
+#endif /* __RICHACL_XATTR_H */
-- 
1.9.1

[PATCH -V1 20/22] vfs: Add richacl permission check

[Aneesh Kumar K.V]

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/attr.c               |  6 ++++--
 fs/namei.c              | 13 +++++++++++-
 fs/richacl_base.c       | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/fs.h      |  2 +-
 include/linux/richacl.h |  2 ++
 5 files changed, 73 insertions(+), 4 deletions(-)

diff --git a/fs/attr.c b/fs/attr.c
index e468d4f2dca8..c46f3ed82150 100644
--- a/fs/attr.c
+++ b/fs/attr.c
@@ -15,6 +15,7 @@
 #include <linux/security.h>
 #include <linux/evm.h>
 #include <linux/ima.h>
+#include <linux/richacl.h>
 
 static int richacl_change_ok(struct inode *inode, int mask)
 {
@@ -23,8 +24,9 @@ static int richacl_change_ok(struct inode *inode, int mask)
 
 	if (inode->i_op->permission)
 		return inode->i_op->permission(inode, mask);
-
-	return check_acl(inode, mask);
+	if (inode->i_op->get_richacl)
+		return check_richacl(inode, mask);
+	return -EPERM;
 }
 
 static bool inode_uid_change_ok(struct inode *inode, kuid_t ia_uid)
diff --git a/fs/namei.c b/fs/namei.c
index 06474553c08d..12010e06aedd 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -34,6 +34,7 @@
 #include <linux/device_cgroup.h>
 #include <linux/fs_struct.h>
 #include <linux/posix_acl.h>
+#include <linux/richacl.h>
 #include <asm/uaccess.h>
 
 #include "internal.h"
@@ -249,7 +250,7 @@ void putname(struct filename *name)
 }
 #endif
 
-int check_acl(struct inode *inode, int mask)
+static int check_posix_acl(struct inode *inode, int mask)
 {
 #ifdef CONFIG_FS_POSIX_ACL
 	struct posix_acl *acl;
@@ -277,6 +278,16 @@ int check_acl(struct inode *inode, int mask)
 	return -EAGAIN;
 }
 
+static int check_acl(struct inode *inode, int mask)
+{
+	if (IS_POSIXACL(inode))
+		return check_posix_acl(inode, mask);
+	else if (IS_RICHACL(inode))
+		return check_richacl(inode, mask);
+	else
+		return -EAGAIN;
+}
+
 /*
  * This does the basic permission checking
  */
diff --git a/fs/richacl_base.c b/fs/richacl_base.c
index d3beaacf6b9b..7c42290ae0c4 100644
--- a/fs/richacl_base.c
+++ b/fs/richacl_base.c
@@ -585,3 +585,57 @@ richacl_equiv_mode(const struct richacl *acl, mode_t *mode_p)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(richacl_equiv_mode);
+
+int check_richacl(struct inode *inode, int want)
+{
+#ifdef CONFIG_FS_RICHACL
+	struct richacl *acl;
+	int richacl_mask = richacl_want_to_mask(want);
+
+	if (want & MAY_NOT_BLOCK) {
+		acl = rcu_dereference(inode->i_richacl);
+		if (!acl)
+			return -EAGAIN;
+		/* no ->get_acl() calls in RCU mode... */
+		if (acl == ACL_NOT_CACHED)
+			return -ECHILD;
+		return richacl_permission(inode, acl, richacl_mask);
+	}
+	return richacl_check_acl(inode, richacl_mask);
+#endif
+	return -EAGAIN;
+}
+
+int richacl_check_acl(struct inode *inode, int richacl_mask)
+{
+
+#ifdef CONFIG_FS_RICHACL
+	struct richacl *acl;
+	acl = get_cached_richacl(inode);
+	/*
+	 * A filesystem can force a ACL callback by just never filling the
+	 * ACL cache. But normally you'd fill the cache either at inode
+	 * instantiation time, or on the first ->get_acl call.
+	 *
+	 * If the filesystem doesn't have a get_acl() function at all, we'll
+	 * just create the negative cache entry.
+	 */
+	if (acl == ACL_NOT_CACHED) {
+		if (inode->i_op->get_acl) {
+			acl = inode->i_op->get_richacl(inode);
+			if (IS_ERR(acl))
+				return PTR_ERR(acl);
+		} else {
+			set_cached_richacl(inode, NULL);
+			return -EAGAIN;
+		}
+	}
+	if (acl) {
+		int error = richacl_permission(inode, acl, richacl_mask);
+		richacl_put(acl);
+		return error;
+	}
+#endif
+	return -EAGAIN;
+}
+EXPORT_SYMBOL_GPL(richacl_check_acl);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 95df64d21e55..77459b6656ac 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1499,6 +1499,7 @@ struct inode_operations {
 	void * (*follow_link) (struct dentry *, struct nameidata *);
 	int (*permission) (struct inode *, int);
 	struct posix_acl * (*get_acl)(struct inode *, int);
+	struct richacl * (*get_richacl)(struct inode *);
 
 	int (*readlink) (struct dentry *, char __user *,int);
 	void (*put_link) (struct dentry *, struct nameidata *, void *);
@@ -2259,7 +2260,6 @@ extern sector_t bmap(struct inode *, sector_t);
 extern int notify_change(struct dentry *, struct iattr *, struct inode **);
 extern int inode_permission(struct inode *, int);
 extern int generic_permission(struct inode *, int);
-extern int check_acl(struct inode *, int);
 
 static inline bool execute_ok(struct inode *inode)
 {
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index a7db341e4ee9..17b58d470798 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -360,6 +360,8 @@ extern int richacl_permission(struct inode *, const struct richacl *,
 			      unsigned int);
 extern struct richacl *richacl_inherit(const struct richacl *, int);
 extern int richacl_equiv_mode(const struct richacl *, mode_t *);
+extern int check_richacl(struct inode *, int);
+extern int richacl_check_acl(struct inode *, int);
 
 /* richacl_inode.c */
 extern struct richacl *richacl_inherit_inode(const struct richacl *,
-- 
1.9.1

[PATCH -V1 21/22] ext4: Implement rich acl for ext4

[Aneesh Kumar K.V]

Support the richacl permission model in ext4.  The richacls are stored
in "system.richacl" xattrs.This need to be enabled by tune2fs or during
mkfs.ext4

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/Kconfig   |  15 ++++
 fs/ext4/Makefile  |   1 +
 fs/ext4/acl.c     |   7 +-
 fs/ext4/acl.h     |  12 +--
 fs/ext4/file.c    |   6 +-
 fs/ext4/ialloc.c  |   7 +-
 fs/ext4/inode.c   |  10 ++-
 fs/ext4/namei.c   |  11 ++-
 fs/ext4/richacl.c | 228 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 fs/ext4/richacl.h |  46 +++++++++++
 fs/ext4/xattr.c   |   6 ++
 fs/ext4/xattr.h   |   1 +
 12 files changed, 330 insertions(+), 20 deletions(-)
 create mode 100644 fs/ext4/richacl.c
 create mode 100644 fs/ext4/richacl.h

diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig
index efea5d5c44ce..8c821d221de6 100644
--- a/fs/ext4/Kconfig
+++ b/fs/ext4/Kconfig
@@ -73,3 +73,18 @@ config EXT4_DEBUG
 	  If you select Y here, then you will be able to turn on debugging
 	  with a command such as:
 		echo 1 > /sys/module/ext4/parameters/mballoc_debug
+
+config EXT4_FS_RICHACL
+	bool "Ext4 Rich Access Control Lists (EXPERIMENTAL)"
+	depends on EXT4_FS
+	select FS_RICHACL
+	help
+	  Rich ACLs are an implementation of NFSv4 ACLs, extended by file masks
+	  to fit into the standard POSIX file permission model.  They are
+	  designed to work seamlessly locally as well as across the NFSv4 and
+	  CIFS/SMB2 network file system protocols.
+
+	  To learn more about Rich ACL, visit
+	  http://acl.bestbits.at/richacl/
+
+	  If you don't know what Rich ACLs are, say N
diff --git a/fs/ext4/Makefile b/fs/ext4/Makefile
index 0310fec2ee3d..b9a3e2ee47c9 100644
--- a/fs/ext4/Makefile
+++ b/fs/ext4/Makefile
@@ -12,3 +12,4 @@ ext4-y	:= balloc.o bitmap.o dir.o file.o fsync.o ialloc.o inode.o page-io.o \
 
 ext4-$(CONFIG_EXT4_FS_POSIX_ACL)	+= acl.o
 ext4-$(CONFIG_EXT4_FS_SECURITY)		+= xattr_security.o
+ext4-$(CONFIG_EXT4_FS_RICHACL) 		+= richacl.o
diff --git a/fs/ext4/acl.c b/fs/ext4/acl.c
index d40c8dbbb0d6..7c508f7037fe 100644
--- a/fs/ext4/acl.c
+++ b/fs/ext4/acl.c
@@ -144,8 +144,7 @@ fail:
  *
  * inode->i_mutex: don't care
  */
-struct posix_acl *
-ext4_get_acl(struct inode *inode, int type)
+struct posix_acl *ext4_get_posix_acl(struct inode *inode, int type)
 {
 	int name_index;
 	char *value = NULL;
@@ -239,7 +238,7 @@ __ext4_set_acl(handle_t *handle, struct inode *inode, int type,
 }
 
 int
-ext4_set_acl(struct inode *inode, struct posix_acl *acl, int type)
+ext4_set_posix_acl(struct inode *inode, struct posix_acl *acl, int type)
 {
 	handle_t *handle;
 	int error, retries = 0;
@@ -264,7 +263,7 @@ retry:
  * inode->i_mutex: up (access to inode is still exclusive)
  */
 int
-ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
+ext4_init_posix_acl(handle_t *handle, struct inode *inode, struct inode *dir)
 {
 	struct posix_acl *default_acl, *acl;
 	int error;
diff --git a/fs/ext4/acl.h b/fs/ext4/acl.h
index da2c79577d72..450b4d18ad9e 100644
--- a/fs/ext4/acl.h
+++ b/fs/ext4/acl.h
@@ -54,17 +54,17 @@ static inline int ext4_acl_count(size_t size)
 #ifdef CONFIG_EXT4_FS_POSIX_ACL
 
 /* acl.c */
-struct posix_acl *ext4_get_acl(struct inode *inode, int type);
-int ext4_set_acl(struct inode *inode, struct posix_acl *acl, int type);
-extern int ext4_init_acl(handle_t *, struct inode *, struct inode *);
+struct posix_acl *ext4_get_posix_acl(struct inode *inode, int type);
+int ext4_set_posix_acl(struct inode *inode, struct posix_acl *acl, int type);
+extern int ext4_init_posix_acl(handle_t *, struct inode *, struct inode *);
 
 #else  /* CONFIG_EXT4_FS_POSIX_ACL */
 #include <linux/sched.h>
-#define ext4_get_acl NULL
-#define ext4_set_acl NULL
+#define ext4_get_posix_acl NULL
+#define ext4_set_posix_acl NULL
 
 static inline int
-ext4_init_acl(handle_t *handle, struct inode *inode, struct inode *dir)
+ext4_init_posix_acl(handle_t *handle, struct inode *inode, struct inode *dir)
 {
 	return 0;
 }
diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 063fc1538355..fddca0ed81d1 100644
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -30,6 +30,7 @@
 #include "ext4_jbd2.h"
 #include "xattr.h"
 #include "acl.h"
+#include "richacl.h"
 
 /*
  * Called when an inode is released. Note that this is different
@@ -617,8 +618,9 @@ const struct inode_operations ext4_file_inode_operations = {
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
 	.removexattr	= generic_removexattr,
-	.get_acl	= ext4_get_acl,
-	.set_acl	= ext4_set_acl,
+	.get_acl	= ext4_get_posix_acl,
+	.set_acl	= ext4_set_posix_acl,
+	.get_richacl	= ext4_get_richacl,
 	.fiemap		= ext4_fiemap,
 };
 
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 0ee59a6644e2..a05b1dcf991f 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -28,6 +28,7 @@
 #include "ext4_jbd2.h"
 #include "xattr.h"
 #include "acl.h"
+#include "richacl.h"
 
 #include <trace/events/ext4.h>
 
@@ -1013,7 +1014,11 @@ got:
 	if (err)
 		goto fail_drop;
 
-	err = ext4_init_acl(handle, inode, dir);
+	if (EXT4_IS_RICHACL(dir))
+		err = ext4_init_richacl(handle, inode, dir);
+	else
+		err = ext4_init_posix_acl(handle, inode, dir);
+
 	if (err)
 		goto fail_free_drop;
 
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index d7b7462a0e13..3c74f7a9765e 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -44,6 +44,7 @@
 #include "xattr.h"
 #include "acl.h"
 #include "truncate.h"
+#include "richacl.h"
 
 #include <trace/events/ext4.h>
 
@@ -4697,9 +4698,12 @@ int ext4_setattr(struct dentry *dentry, struct iattr *attr)
 	if (orphan && inode->i_nlink)
 		ext4_orphan_del(NULL, inode);
 
-	if (!rc && (ia_valid & ATTR_MODE))
-		rc = posix_acl_chmod(inode, inode->i_mode);
-
+	if (!rc && (ia_valid & ATTR_MODE)) {
+		if (EXT4_IS_RICHACL(inode))
+			rc = ext4_richacl_chmod(inode);
+		else
+			rc = posix_acl_chmod(inode, inode->i_mode);
+	}
 err_out:
 	ext4_std_error(inode->i_sb, error);
 	if (!error)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 1cb84f78909e..4f293601abbd 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -39,6 +39,7 @@
 
 #include "xattr.h"
 #include "acl.h"
+#include "richacl.h"
 
 #include <trace/events/ext4.h>
 /*
@@ -3441,8 +3442,9 @@ const struct inode_operations ext4_dir_inode_operations = {
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
 	.removexattr	= generic_removexattr,
-	.get_acl	= ext4_get_acl,
-	.set_acl	= ext4_set_acl,
+	.get_acl	= ext4_get_posix_acl,
+	.set_acl	= ext4_set_posix_acl,
+	.get_richacl	= ext4_get_richacl,
 	.fiemap         = ext4_fiemap,
 };
 
@@ -3452,6 +3454,7 @@ const struct inode_operations ext4_special_inode_operations = {
 	.getxattr	= generic_getxattr,
 	.listxattr	= ext4_listxattr,
 	.removexattr	= generic_removexattr,
-	.get_acl	= ext4_get_acl,
-	.set_acl	= ext4_set_acl,
+	.get_acl	= ext4_get_posix_acl,
+	.set_acl	= ext4_set_posix_acl,
+	.get_richacl	= ext4_get_richacl,
 };
diff --git a/fs/ext4/richacl.c b/fs/ext4/richacl.c
new file mode 100644
index 000000000000..bf5faf27790e
--- /dev/null
+++ b/fs/ext4/richacl.c
@@ -0,0 +1,228 @@
+/*
+ * Copyright IBM Corporation, 2010
+ * Author Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2.1 of the GNU Lesser General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/fs.h>
+#include <linux/richacl_xattr.h>
+
+#include "ext4.h"
+#include "ext4_jbd2.h"
+#include "xattr.h"
+#include "acl.h"
+#include "richacl.h"
+
+struct richacl *
+ext4_get_richacl(struct inode *inode)
+{
+	const int name_index = EXT4_XATTR_INDEX_RICHACL;
+	void *value = NULL;
+	struct richacl *acl;
+	int retval;
+
+	if (!IS_RICHACL(inode))
+		return ERR_PTR(-EOPNOTSUPP);
+	acl = get_cached_richacl(inode);
+	if (acl != ACL_NOT_CACHED)
+		return acl;
+	retval = ext4_xattr_get(inode, name_index, "", NULL, 0);
+	if (retval > 0) {
+		value = kmalloc(retval, GFP_KERNEL);
+		if (!value)
+			return ERR_PTR(-ENOMEM);
+		retval = ext4_xattr_get(inode, name_index, "", value, retval);
+	}
+	if (retval > 0) {
+		acl = richacl_from_xattr(value, retval);
+		if (acl == ERR_PTR(-EINVAL))
+			acl = ERR_PTR(-EIO);
+	} else if (retval == -ENODATA || retval == -ENOSYS)
+		acl = NULL;
+	else
+		acl = ERR_PTR(retval);
+	kfree(value);
+
+	if (!IS_ERR_OR_NULL(acl))
+		set_cached_richacl(inode, acl);
+
+	return acl;
+}
+
+static int
+ext4_set_richacl(handle_t *handle, struct inode *inode, struct richacl *acl)
+{
+	const int name_index = EXT4_XATTR_INDEX_RICHACL;
+	size_t size = 0;
+	void *value = NULL;
+	int retval;
+
+	if (acl) {
+		mode_t mode = inode->i_mode;
+		if (richacl_equiv_mode(acl, &mode) == 0) {
+			inode->i_mode = mode;
+			ext4_mark_inode_dirty(handle, inode);
+			acl = NULL;
+		}
+	}
+	if (acl) {
+		size = richacl_xattr_size(acl);
+		value = kmalloc(size, GFP_KERNEL);
+		if (!value)
+			return -ENOMEM;
+		richacl_to_xattr(acl, value);
+	}
+	if (handle)
+		retval = ext4_xattr_set_handle(handle, inode, name_index, "",
+					       value, size, 0);
+	else
+		retval = ext4_xattr_set(inode, name_index, "", value, size, 0);
+	kfree(value);
+	if (!retval)
+		set_cached_richacl(inode, acl);
+
+	return retval;
+}
+
+int
+ext4_init_richacl(handle_t *handle, struct inode *inode, struct inode *dir)
+{
+	struct richacl *dir_acl = NULL;
+
+	if (!S_ISLNK(inode->i_mode)) {
+		dir_acl = ext4_get_richacl(dir);
+		if (IS_ERR(dir_acl))
+			return PTR_ERR(dir_acl);
+	}
+	if (dir_acl) {
+		struct richacl *acl;
+		int retval;
+
+		acl = richacl_inherit_inode(dir_acl, inode);
+		richacl_put(dir_acl);
+
+		retval = PTR_ERR(acl);
+		if (acl && !IS_ERR(acl)) {
+			retval = ext4_set_richacl(handle, inode, acl);
+			richacl_put(acl);
+		}
+		return retval;
+	} else {
+		inode->i_mode &= ~current_umask();
+		return 0;
+	}
+}
+
+int
+ext4_richacl_chmod(struct inode *inode)
+{
+	struct richacl *acl;
+	int retval;
+
+	if (S_ISLNK(inode->i_mode))
+		return -EOPNOTSUPP;
+	acl = ext4_get_richacl(inode);
+	if (IS_ERR_OR_NULL(acl))
+		return PTR_ERR(acl);
+	acl = richacl_chmod(acl, inode->i_mode);
+	if (IS_ERR(acl))
+		return PTR_ERR(acl);
+	retval = ext4_set_richacl(NULL, inode, acl);
+	richacl_put(acl);
+
+	return retval;
+}
+
+static size_t
+ext4_xattr_list_richacl(struct dentry *dentry, char *list, size_t list_len,
+			const char *name, size_t name_len, int type)
+{
+	const size_t size = sizeof(RICHACL_XATTR);
+	if (!IS_RICHACL(dentry->d_inode))
+		return 0;
+	if (list && size <= list_len)
+		memcpy(list, RICHACL_XATTR, size);
+	return size;
+}
+
+static int
+ext4_xattr_get_richacl(struct dentry *dentry, const char *name, void *buffer,
+		size_t buffer_size, int type)
+{
+	struct richacl *acl;
+	size_t size;
+
+	if (strcmp(name, "") != 0)
+		return -EINVAL;
+	acl = ext4_get_richacl(dentry->d_inode);
+	if (IS_ERR(acl))
+		return PTR_ERR(acl);
+	if (acl == NULL)
+		return -ENODATA;
+	size = richacl_xattr_size(acl);
+	if (buffer) {
+		if (size > buffer_size)
+			return -ERANGE;
+		richacl_to_xattr(acl, buffer);
+	}
+	richacl_put(acl);
+
+	return size;
+}
+
+static int
+ext4_xattr_set_richacl(struct dentry *dentry, const char *name,
+		const void *value, size_t size, int flags, int type)
+{
+	handle_t *handle;
+	struct richacl *acl = NULL;
+	int retval, retries = 0;
+	struct inode *inode = dentry->d_inode;
+
+	if (!IS_RICHACL(dentry->d_inode))
+		return -EOPNOTSUPP;
+	if (S_ISLNK(inode->i_mode))
+		return -EOPNOTSUPP;
+	if (strcmp(name, "") != 0)
+		return -EINVAL;
+	if (!uid_eq(current_fsuid(), inode->i_uid) &&
+	    richacl_check_acl(inode, ACE4_WRITE_ACL) &&
+	    !capable(CAP_FOWNER))
+		return -EPERM;
+	if (value) {
+		acl = richacl_from_xattr(value, size);
+		if (IS_ERR(acl))
+			return PTR_ERR(acl);
+
+		inode->i_mode &= ~S_IRWXUGO;
+		inode->i_mode |= richacl_masks_to_mode(acl);
+	}
+
+retry:
+	handle = ext4_journal_start(inode, EXT4_HT_XATTR,
+				    EXT4_DATA_TRANS_BLOCKS(inode->i_sb));
+	if (IS_ERR(handle))
+		return PTR_ERR(handle);
+	retval = ext4_set_richacl(handle, inode, acl);
+	ext4_journal_stop(handle);
+	if (retval == -ENOSPC && ext4_should_retry_alloc(inode->i_sb, &retries))
+		goto retry;
+	richacl_put(acl);
+	return retval;
+}
+
+const struct xattr_handler ext4_richacl_xattr_handler = {
+	.prefix	= RICHACL_XATTR,
+	.list	= ext4_xattr_list_richacl,
+	.get	= ext4_xattr_get_richacl,
+	.set	= ext4_xattr_set_richacl,
+};
diff --git a/fs/ext4/richacl.h b/fs/ext4/richacl.h
new file mode 100644
index 000000000000..2577c34e2741
--- /dev/null
+++ b/fs/ext4/richacl.h
@@ -0,0 +1,46 @@
+/*
+ * Copyright IBM Corporation, 2010
+ * Author Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of version 2.1 of the GNU Lesser General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it would be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+ *
+ */
+
+#ifndef __FS_EXT4_RICHACL_H
+#define __FS_EXT4_RICHACL_H
+
+#include <linux/richacl.h>
+
+#ifdef CONFIG_EXT4_FS_RICHACL
+
+#define EXT4_IS_RICHACL(inode) IS_RICHACL(inode)
+
+extern struct richacl *ext4_get_richacl(struct inode *);
+extern int ext4_init_richacl(handle_t *, struct inode *, struct inode *);
+extern int ext4_richacl_chmod(struct inode *);
+
+#else  /* CONFIG_FS_EXT4_RICHACL */
+
+#define EXT4_IS_RICHACL(inode) (0)
+#define ext4_get_richacl   NULL
+
+static inline int
+ext4_init_richacl(handle_t *handle, struct inode *inode, struct inode *dir)
+{
+	return 0;
+}
+
+static inline int
+ext4_richacl_chmod(struct inode *inode)
+{
+	return 0;
+}
+
+#endif  /* CONFIG_FS_EXT4_RICHACL */
+#endif  /* __FS_EXT4_RICHACL_H */
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 4eec399ec807..5c47540697db 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -100,6 +100,9 @@ static const struct xattr_handler *ext4_xattr_handler_map[] = {
 #ifdef CONFIG_EXT4_FS_SECURITY
 	[EXT4_XATTR_INDEX_SECURITY]	     = &ext4_xattr_security_handler,
 #endif
+#ifdef CONFIG_EXT4_FS_RICHACL
+	[EXT4_XATTR_INDEX_RICHACL]           = &ext4_richacl_xattr_handler,
+#endif
 };
 
 const struct xattr_handler *ext4_xattr_handlers[] = {
@@ -112,6 +115,9 @@ const struct xattr_handler *ext4_xattr_handlers[] = {
 #ifdef CONFIG_EXT4_FS_SECURITY
 	&ext4_xattr_security_handler,
 #endif
+#ifdef CONFIG_EXT4_FS_RICHACL
+	&ext4_richacl_xattr_handler,
+#endif
 	NULL
 };
 
diff --git a/fs/ext4/xattr.h b/fs/ext4/xattr.h
index 29bedf5589f6..065821e814f2 100644
--- a/fs/ext4/xattr.h
+++ b/fs/ext4/xattr.h
@@ -97,6 +97,7 @@ struct ext4_xattr_ibody_find {
 extern const struct xattr_handler ext4_xattr_user_handler;
 extern const struct xattr_handler ext4_xattr_trusted_handler;
 extern const struct xattr_handler ext4_xattr_security_handler;
+extern const struct xattr_handler ext4_richacl_xattr_handler;
 
 extern ssize_t ext4_listxattr(struct dentry *, char *, size_t);
 
-- 
1.9.1

[PATCH -V1 22/22] ext4: Add Ext4 compat richacl feature flag

[Aneesh Kumar K.V]

This feature flag can be used to enable richacl on
the file system. Once enabled the "acl" mount option
will enable richacl instead of posix acl

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 fs/ext4/ext4.h  |  3 ++-
 fs/ext4/super.c | 47 ++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 66946aa62127..17ff4a1cf91e 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -976,7 +976,7 @@ struct ext4_inode_info {
 #define EXT4_MOUNT_UPDATE_JOURNAL	0x01000	/* Update the journal format */
 #define EXT4_MOUNT_NO_UID32		0x02000  /* Disable 32-bit UIDs */
 #define EXT4_MOUNT_XATTR_USER		0x04000	/* Extended user attributes */
-#define EXT4_MOUNT_POSIX_ACL		0x08000	/* POSIX Access Control Lists */
+#define EXT4_MOUNT_ACL			0x08000	/* Access Control Lists */
 #define EXT4_MOUNT_NO_AUTO_DA_ALLOC	0x10000	/* No auto delalloc mapping */
 #define EXT4_MOUNT_BARRIER		0x20000 /* Use block barriers */
 #define EXT4_MOUNT_QUOTA		0x80000 /* Some quota option set */
@@ -1505,6 +1505,7 @@ static inline void ext4_clear_state_flags(struct ext4_inode_info *ei)
 #define EXT4_FEATURE_COMPAT_EXT_ATTR		0x0008
 #define EXT4_FEATURE_COMPAT_RESIZE_INODE	0x0010
 #define EXT4_FEATURE_COMPAT_DIR_INDEX		0x0020
+#define EXT4_FEATURE_COMPAT_RICHACL		0x0200
 
 #define EXT4_FEATURE_RO_COMPAT_SPARSE_SUPER	0x0001
 #define EXT4_FEATURE_RO_COMPAT_LARGE_FILE	0x0002
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 6f9e6fadac04..2a0221652d79 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1274,6 +1274,30 @@ static ext4_fsblk_t get_sb_block(void **data)
 	return sb_block;
 }
 
+static void enable_acl(struct super_block *sb)
+{
+#if !defined(CONFIG_EXT4_FS_POSIX_ACL) && !defined(CONFIG_EXT4_FS_RICHACL)
+	return;
+#endif
+	if (EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_RICHACL)) {
+		sb->s_flags |= MS_RICHACL;
+		sb->s_flags &= ~MS_POSIXACL;
+	} else {
+		sb->s_flags |= MS_POSIXACL;
+		sb->s_flags &= ~MS_RICHACL;
+	}
+	return;
+}
+
+static void disable_acl(struct super_block *sb)
+{
+#if !defined(CONFIG_EXT4_FS_POSIX_ACL) && !defined(CONFIG_EXT4_FS_RICHACL)
+	return;
+#endif
+	sb->s_flags &= ~(MS_POSIXACL | MS_RICHACL);
+	return;
+}
+
 #define DEFAULT_JOURNAL_IOPRIO (IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, 3))
 static char deprecated_msg[] = "Mount option \"%s\" will be removed by %s\n"
 	"Contact linux-ext4@vger.kernel.org if you think we should keep it.\n";
@@ -1417,9 +1441,9 @@ static const struct mount_opts {
 	 MOPT_NO_EXT2 | MOPT_DATAJ},
 	{Opt_user_xattr, EXT4_MOUNT_XATTR_USER, MOPT_SET},
 	{Opt_nouser_xattr, EXT4_MOUNT_XATTR_USER, MOPT_CLEAR},
-#ifdef CONFIG_EXT4_FS_POSIX_ACL
-	{Opt_acl, EXT4_MOUNT_POSIX_ACL, MOPT_SET},
-	{Opt_noacl, EXT4_MOUNT_POSIX_ACL, MOPT_CLEAR},
+#if defined(CONFIG_EXT4_FS_POSIX_ACL) || defined(CONFIG_EXT4_FS_RICHACL)
+	{Opt_acl, EXT4_MOUNT_ACL, MOPT_SET},
+	{Opt_noacl, EXT4_MOUNT_ACL, MOPT_CLEAR},
 #else
 	{Opt_acl, 0, MOPT_NOSUPPORT},
 	{Opt_noacl, 0, MOPT_NOSUPPORT},
@@ -3496,8 +3520,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 		set_opt(sb, NO_UID32);
 	/* xattr user namespace & acls are now defaulted on */
 	set_opt(sb, XATTR_USER);
-#ifdef CONFIG_EXT4_FS_POSIX_ACL
-	set_opt(sb, POSIX_ACL);
+#if defined(CONFIG_EXT4_FS_POSIX_ACL) || defined(CONFIG_EXT4_FS_RICHACL)
+	set_opt(sb, ACL);
 #endif
 	if ((def_mount_opts & EXT4_DEFM_JMODE) == EXT4_DEFM_JMODE_DATA)
 		set_opt(sb, JOURNAL_DATA);
@@ -3569,8 +3593,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 			clear_opt(sb, DELALLOC);
 	}
 
-	sb->s_flags = (sb->s_flags & ~MS_POSIXACL) |
-		(test_opt(sb, POSIX_ACL) ? MS_POSIXACL : 0);
+	/*
+	 * clear ACL flags
+	 */
+	disable_acl(sb);
+	if (test_opt(sb, ACL))
+		enable_acl(sb);
 
 	if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV &&
 	    (EXT4_HAS_COMPAT_FEATURE(sb, ~0U) ||
@@ -4844,8 +4872,9 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
 	if (sbi->s_mount_flags & EXT4_MF_FS_ABORTED)
 		ext4_abort(sb, "Abort forced by user");
 
-	sb->s_flags = (sb->s_flags & ~MS_POSIXACL) |
-		(test_opt(sb, POSIX_ACL) ? MS_POSIXACL : 0);
+	disable_acl(sb);
+	if (test_opt(sb, ACL))
+		enable_acl(sb);
 
 	es = sbi->s_es;
 
-- 
1.9.1

Re: [PATCH -V1 22/22] ext4: Add Ext4 compat richacl feature flag

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 5007 bytes --]

On Apr 27, 2014, at 10:14 AM, Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
> This feature flag can be used to enable richacl on
> the file system. Once enabled the "acl" mount option
> will enable richacl instead of posix acl

I was going to complain about this patch, because re-using the "acl"
mount option to specify richacl instead of POSIX ACL would be very
confusing, since older kernels used the "acl" mount option to enable
POSIX ACLs.

Looking closer, I see that "acl" and "noacl" just means enable or disable
the ACL functionality on the filesystem.  Please fix up the commit comment.

Some more comments inline.

> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 6f9e6fadac04..2a0221652d79 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -1274,6 +1274,30 @@ static ext4_fsblk_t get_sb_block(void **data)
> 	return sb_block;
> }
> 
> +static void enable_acl(struct super_block *sb)
> +{
> +#if !defined(CONFIG_EXT4_FS_POSIX_ACL) && !defined(CONFIG_EXT4_FS_RICHACL)
> +	return;
> +#endif
> +	if (EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_RICHACL)) {
> +		sb->s_flags |= MS_RICHACL;
> +		sb->s_flags &= ~MS_POSIXACL;
> +	} else {
> +		sb->s_flags |= MS_POSIXACL;
> +		sb->s_flags &= ~MS_RICHACL;
> +	}

This should put the #ifdef around the code that is being enabled/disabled,
otherwise it just becomes dead code:

static int enable_acl(struct super_block *sb)
{
	if (EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_RICHACL)) {
#if defined(CONFIG_EXT4_FS_RICHACL)
		sb->s_flags |= MS_RICHACL;
		sb->s_flags &= ~MS_POSIXACL;
#else
		return -EOPNOTSUPP;
#endif
	} else {
#if defined(CONFIG_EXT4_FS_POSIX_ACL)
		sb->s_flags |= MS_POSIXACL;
		sb->s_flags &= ~MS_RICHACL;
#else
		return -EOPNOTSUPP;
#endif
	}
	return 0;
}

> +
> +static void disable_acl(struct super_block *sb)
> +{
> +#if !defined(CONFIG_EXT4_FS_POSIX_ACL) && !defined(CONFIG_EXT4_FS_RICHACL)
> +	return;
> +#endif
> +	sb->s_flags &= ~(MS_POSIXACL | MS_RICHACL);
> +	return;
> +}

"return" is not needed at the end of void functions. Same comment on #ifdef:

static void disable_acl(struct super_block *sb)
{
#if defined(CONFIG_EXT4_FS_POSIX_ACL) || defined(CONFIG_EXT4_FS_RICHACL)
	sb->s_flags &= ~(MS_POSIXACL | MS_RICHACL);
#endif
}


> +
> #define DEFAULT_JOURNAL_IOPRIO (IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, 3))
> static char deprecated_msg[] = "Mount option \"%s\" will be removed by %s\n"
> 	"Contact linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org if you think we should keep it.\n";
> @@ -1417,9 +1441,9 @@ static const struct mount_opts {
> 	 MOPT_NO_EXT2 | MOPT_DATAJ},
> 	{Opt_user_xattr, EXT4_MOUNT_XATTR_USER, MOPT_SET},
> 	{Opt_nouser_xattr, EXT4_MOUNT_XATTR_USER, MOPT_CLEAR},
> -#ifdef CONFIG_EXT4_FS_POSIX_ACL
> -	{Opt_acl, EXT4_MOUNT_POSIX_ACL, MOPT_SET},
> -	{Opt_noacl, EXT4_MOUNT_POSIX_ACL, MOPT_CLEAR},
> +#if defined(CONFIG_EXT4_FS_POSIX_ACL) || defined(CONFIG_EXT4_FS_RICHACL)
> +	{Opt_acl, EXT4_MOUNT_ACL, MOPT_SET},
> +	{Opt_noacl, EXT4_MOUNT_ACL, MOPT_CLEAR},
> #else
> 	{Opt_acl, 0, MOPT_NOSUPPORT},
> 	{Opt_noacl, 0, MOPT_NOSUPPORT},
> @@ -3496,8 +3520,8 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> 		set_opt(sb, NO_UID32);
> 	/* xattr user namespace & acls are now defaulted on */
> 	set_opt(sb, XATTR_USER);
> -#ifdef CONFIG_EXT4_FS_POSIX_ACL
> -	set_opt(sb, POSIX_ACL);
> +#if defined(CONFIG_EXT4_FS_POSIX_ACL) || defined(CONFIG_EXT4_FS_RICHACL)
> +	set_opt(sb, ACL);
> #endif
> 	if ((def_mount_opts & EXT4_DEFM_JMODE) == EXT4_DEFM_JMODE_DATA)
> 		set_opt(sb, JOURNAL_DATA);
> @@ -3569,8 +3593,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> 			clear_opt(sb, DELALLOC);
> 	}
> 
> -	sb->s_flags = (sb->s_flags & ~MS_POSIXACL) |
> -		(test_opt(sb, POSIX_ACL) ? MS_POSIXACL : 0);
> +	/*
> +	 * clear ACL flags
> +	 */
> +	disable_acl(sb);

Is there any expectation that the flags would be set on a newly mounted
filesystem?

> +	if (test_opt(sb, ACL))
> +		enable_acl(sb);
> 
> 	if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV &&
> 	    (EXT4_HAS_COMPAT_FEATURE(sb, ~0U) ||
> @@ -4844,8 +4872,9 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
> 	if (sbi->s_mount_flags & EXT4_MF_FS_ABORTED)
> 		ext4_abort(sb, "Abort forced by user");
> 
> -	sb->s_flags = (sb->s_flags & ~MS_POSIXACL) |
> -		(test_opt(sb, POSIX_ACL) ? MS_POSIXACL : 0);
> +	disable_acl(sb);
> +	if (test_opt(sb, ACL))
> +		enable_acl(sb);

Similarly, it seems racy to me to disable ACL support and then re-enable
it here during remount, since that might cause some concurrent operations
to fail.  It seems like enable_acl() already handles clearing the flags
correctly, so something like the following would be better:

	if (test_opt(sb, ACL))
		enable_acl(sb);
	else
		disable_acl(sb);


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH -V1 22/22] ext4: Add Ext4 compat richacl feature flag

[Aneesh Kumar K.V]

Andreas Dilger <adilger-m1MBpc4rdrD3fQ9qLvQP4Q@public.gmane.org> writes:

> On Apr 27, 2014, at 10:14 AM, Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org> wrote:
>> This feature flag can be used to enable richacl on
>> the file system. Once enabled the "acl" mount option
>> will enable richacl instead of posix acl
>
> I was going to complain about this patch, because re-using the "acl"
> mount option to specify richacl instead of POSIX ACL would be very
> confusing, since older kernels used the "acl" mount option to enable
> POSIX ACLs.
>
> Looking closer, I see that "acl" and "noacl" just means enable or disable
> the ACL functionality on the filesystem.  Please fix up the commit
> comment.

Will clarify in the commit message.

>
> Some more comments inline.
>
>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>> index 6f9e6fadac04..2a0221652d79 100644
>> --- a/fs/ext4/super.c
>> +++ b/fs/ext4/super.c
>> @@ -1274,6 +1274,30 @@ static ext4_fsblk_t get_sb_block(void **data)
>> 	return sb_block;
>> }
>> 
>> +static void enable_acl(struct super_block *sb)
>> +{
>> +#if !defined(CONFIG_EXT4_FS_POSIX_ACL) && !defined(CONFIG_EXT4_FS_RICHACL)
>> +	return;
>> +#endif
>> +	if (EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_RICHACL)) {
>> +		sb->s_flags |= MS_RICHACL;
>> +		sb->s_flags &= ~MS_POSIXACL;
>> +	} else {
>> +		sb->s_flags |= MS_POSIXACL;
>> +		sb->s_flags &= ~MS_RICHACL;
>> +	}
>
> This should put the #ifdef around the code that is being enabled/disabled,
> otherwise it just becomes dead code:
>
> static int enable_acl(struct super_block *sb)
> {
> 	if (EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_RICHACL)) {
> #if defined(CONFIG_EXT4_FS_RICHACL)
> 		sb->s_flags |= MS_RICHACL;
> 		sb->s_flags &= ~MS_POSIXACL;
> #else
> 		return -EOPNOTSUPP;
> #endif
> 	} else {
> #if defined(CONFIG_EXT4_FS_POSIX_ACL)
> 		sb->s_flags |= MS_POSIXACL;
> 		sb->s_flags &= ~MS_RICHACL;
> #else
> 		return -EOPNOTSUPP;
> #endif
> 	}
> 	return 0;
> }

That is too much #ifdef with no real benefit ?

>
>> +
>> +static void disable_acl(struct super_block *sb)
>> +{
>> +#if !defined(CONFIG_EXT4_FS_POSIX_ACL) && !defined(CONFIG_EXT4_FS_RICHACL)
>> +	return;
>> +#endif
>> +	sb->s_flags &= ~(MS_POSIXACL | MS_RICHACL);
>> +	return;
>> +}
>
> "return" is not needed at the end of void functions. Same comment on
> #ifdef:

ok

>
> static void disable_acl(struct super_block *sb)
> {
> #if defined(CONFIG_EXT4_FS_POSIX_ACL) || defined(CONFIG_EXT4_FS_RICHACL)
> 	sb->s_flags &= ~(MS_POSIXACL | MS_RICHACL);
> #endif
> }
>
>
>> +
>> #define DEFAULT_JOURNAL_IOPRIO (IOPRIO_PRIO_VALUE(IOPRIO_CLASS_BE, 3))
>> static char deprecated_msg[] = "Mount option \"%s\" will be removed by %s\n"
>> 	"Contact linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org if you think we should keep it.\n";
>> @@ -1417,9 +1441,9 @@ static const struct mount_opts {
>> 	 MOPT_NO_EXT2 | MOPT_DATAJ},
>> 	{Opt_user_xattr, EXT4_MOUNT_XATTR_USER, MOPT_SET},
>> 	{Opt_nouser_xattr, EXT4_MOUNT_XATTR_USER, MOPT_CLEAR},

....

>> 	if ((def_mount_opts & EXT4_DEFM_JMODE) == EXT4_DEFM_JMODE_DATA)
>> 		set_opt(sb, JOURNAL_DATA);
>> @@ -3569,8 +3593,12 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>> 			clear_opt(sb, DELALLOC);
>> 	}
>> 
>> -	sb->s_flags = (sb->s_flags & ~MS_POSIXACL) |
>> -		(test_opt(sb, POSIX_ACL) ? MS_POSIXACL : 0);
>> +	/*
>> +	 * clear ACL flags
>> +	 */
>> +	disable_acl(sb);
>
> Is there any expectation that the flags would be set on a newly mounted
> filesystem?
>
>> +	if (test_opt(sb, ACL))
>> +		enable_acl(sb);
>> 
>> 	if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV &&
>> 	    (EXT4_HAS_COMPAT_FEATURE(sb, ~0U) ||
>> @@ -4844,8 +4872,9 @@ static int ext4_remount(struct super_block *sb, int *flags, char *data)
>> 	if (sbi->s_mount_flags & EXT4_MF_FS_ABORTED)
>> 		ext4_abort(sb, "Abort forced by user");
>> 
>> -	sb->s_flags = (sb->s_flags & ~MS_POSIXACL) |
>> -		(test_opt(sb, POSIX_ACL) ? MS_POSIXACL : 0);
>> +	disable_acl(sb);
>> +	if (test_opt(sb, ACL))
>> +		enable_acl(sb);
>
> Similarly, it seems racy to me to disable ACL support and then re-enable
> it here during remount, since that might cause some concurrent operations
> to fail.  It seems like enable_acl() already handles clearing the flags
> correctly, so something like the following would be better:
>
> 	if (test_opt(sb, ACL))
> 		enable_acl(sb);
> 	else
> 		disable_acl(sb);
>
>

ok

-aneesh

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -V1 22/22] ext4: Add Ext4 compat richacl feature flag

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1992 bytes --]

On May 1, 2014, at 9:48 AM, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:

> Andreas Dilger <adilger@dilger.ca> writes:
> 
>> On Apr 27, 2014, at 10:14 AM, Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com> wrote:
>>> This feature flag can be used to enable richacl on
>>> the file system. Once enabled the "acl" mount option
>>> will enable richacl instead of posix acl
>> 
>>> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
>>> index 6f9e6fadac04..2a0221652d79 100644
>>> --- a/fs/ext4/super.c
>>> +++ b/fs/ext4/super.c
>>> @@ -1274,6 +1274,30 @@ static ext4_fsblk_t get_sb_block(void **data)
>>> 	return sb_block;
>>> }
>>> 
>>> +static void enable_acl(struct super_block *sb)
>>> +{
>>> +#if !defined(CONFIG_EXT4_FS_POSIX_ACL) && !defined(CONFIG_EXT4_FS_RICHACL)
>>> +	return;
>>> +#endif
>>> +	if (EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_RICHACL)) {
>>> +		sb->s_flags |= MS_RICHACL;
>>> +		sb->s_flags &= ~MS_POSIXACL;
>>> +	} else {
>>> +		sb->s_flags |= MS_POSIXACL;
>>> +		sb->s_flags &= ~MS_RICHACL;
>>> +	}
>> 
>> This should put the #ifdef around the code that is being enabled/disabled,
>> otherwise it just becomes dead code:
>> 
>> static int enable_acl(struct super_block *sb)
>> {
>> 	if (EXT4_HAS_COMPAT_FEATURE(sb, EXT4_FEATURE_COMPAT_RICHACL)) {
>> #if defined(CONFIG_EXT4_FS_RICHACL)
>> 		sb->s_flags |= MS_RICHACL;
>> 		sb->s_flags &= ~MS_POSIXACL;
>> #else
>> 		return -EOPNOTSUPP;
>> #endif
>> 	} else {
>> #if defined(CONFIG_EXT4_FS_POSIX_ACL)
>> 		sb->s_flags |= MS_POSIXACL;
>> 		sb->s_flags &= ~MS_RICHACL;
>> #else
>> 		return -EOPNOTSUPP;
>> #endif
>> 	}
>> 	return 0;
>> }
> 
> That is too much #ifdef with no real benefit ?

The benefit is that if neither CONFIG_EXT4_FS_RICHACL nor CONFIG_EXT4_FS_POSIX_ACL are defined there isn't unreachable code
after "return" at the start of the function.  Some static code
analysis tools will complain about this.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH -V1 02/22] vfs: Add IS_RICHACL() test for richacl support

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Introduce a new MS_RICHACL super-block flag and a new IS_RICHACL() test
which file systems like nfs can use.  IS_ACL() is true if IS_POSIXACL()
or IS_RICHACL() is true.

Signed-off-by: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 include/linux/fs.h      | 3 ++-
 include/uapi/linux/fs.h | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 9fb63b71a014..1a7fe920f956 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1601,6 +1601,7 @@ struct super_operations {
 #define IS_APPEND(inode)	((inode)->i_flags & S_APPEND)
 #define IS_IMMUTABLE(inode)	((inode)->i_flags & S_IMMUTABLE)
 #define IS_POSIXACL(inode)	__IS_FLG(inode, MS_POSIXACL)
+#define IS_RICHACL(inode)	__IS_FLG(inode, MS_RICHACL)
 
 #define IS_DEADDIR(inode)	((inode)->i_flags & S_DEAD)
 #define IS_NOCMTIME(inode)	((inode)->i_flags & S_NOCMTIME)
@@ -1614,7 +1615,7 @@ struct super_operations {
  * IS_ACL() tells the VFS to not apply the umask
  * and use check_acl for acl permission checks when defined.
  */
-#define IS_ACL(inode)		__IS_FLG(inode, MS_POSIXACL)
+#define IS_ACL(inode)		__IS_FLG(inode, MS_POSIXACL | MS_RICHACL)
 
 /*
  * Inode state bits.  Protected by inode->i_lock
diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h
index 1e14b9c82703..894fbc2bebb3 100644
--- a/include/uapi/linux/fs.h
+++ b/include/uapi/linux/fs.h
@@ -89,6 +89,7 @@ struct inodes_stat_t {
 #define MS_KERNMOUNT	(1<<22) /* this is a kern_mount call */
 #define MS_I_VERSION	(1<<23) /* Update inode I_version field */
 #define MS_STRICTATIME	(1<<24) /* Always perform atime updates */
+#define MS_RICHACL	(1<<25) /* Supports richacls */
 
 /* These sb flags are internal to the kernel */
 #define MS_NOSEC	(1<<28)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH -V1 13/22] richacl: Update the file masks in chmod()

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Doing a chmod() sets the file mode, which includes the file permission
bits.  When a file has a richacl, the permissions that the richacl
grants need to be limited to what the new file permission bits allow.

This is done by setting the file masks in the richacl to what the file
permission bits map to.  The richacl access check algorithm takes the
file masks into account, which ensures that the richacl cannot grant too
many permissions.

It is possible to explicitly add permissions to the file masks which go
beyond what the file permission bits can grant (like the ACE4_WRITE_ACL
permission).  The POSIX.1 standard calls this an alternate file access
control mechanism.  A subsequent chmod() would ensure that those
permissions are disabled again.

Signed-off-by: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 fs/richacl_base.c       | 40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/richacl.h |  1 +
 2 files changed, 41 insertions(+)

diff --git a/fs/richacl_base.c b/fs/richacl_base.c
index d73f1b0a9c5f..41ff82d10d4d 100644
--- a/fs/richacl_base.c
+++ b/fs/richacl_base.c
@@ -312,3 +312,43 @@ restart:
 	acl->a_flags &= ~ACL4_MASKED;
 }
 EXPORT_SYMBOL_GPL(richacl_compute_max_masks);
+
+/**
+ * richacl_chmod  -  update the file masks to reflect the new mode
+ * @mode:	new file permission bits
+ *
+ * Return a copy of @acl where the file masks have been replaced by the file
+ * masks corresponding to the file permission bits in @mode, or returns @acl
+ * itself if the file masks are already up to date.  Takes over a reference
+ * to @acl.
+ */
+struct richacl *
+richacl_chmod(struct richacl *acl, mode_t mode)
+{
+	unsigned int owner_mask, group_mask, other_mask;
+	struct richacl *clone;
+
+	owner_mask = richacl_mode_to_mask(mode >> 6) |
+		     ACE4_POSIX_OWNER_ALLOWED;
+	group_mask = richacl_mode_to_mask(mode >> 3);
+	other_mask = richacl_mode_to_mask(mode);
+
+	if (acl->a_owner_mask == owner_mask &&
+	    acl->a_group_mask == group_mask &&
+	    acl->a_other_mask == other_mask &&
+	    (acl->a_flags & ACL4_MASKED))
+		return acl;
+
+	clone = richacl_clone(acl);
+	richacl_put(acl);
+	if (!clone)
+		return ERR_PTR(-ENOMEM);
+
+	clone->a_flags |= ACL4_MASKED;
+	clone->a_owner_mask = owner_mask;
+	clone->a_group_mask = group_mask;
+	clone->a_other_mask = other_mask;
+
+	return clone;
+}
+EXPORT_SYMBOL_GPL(richacl_chmod);
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index 14f18b597be3..a1ff8a38409c 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -281,5 +281,6 @@ extern int richacl_masks_to_mode(const struct richacl *);
 extern unsigned int richacl_mode_to_mask(mode_t);
 extern unsigned int richacl_want_to_mask(unsigned int);
 extern void richacl_compute_max_masks(struct richacl *);
+extern struct richacl *richacl_chmod(struct richacl *, mode_t);
 
 #endif /* __RICHACL_H */
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH -V1 19/22] vfs: Cache richacl in struct inode

[Aneesh Kumar K.V]

From: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

Cache richacls in struct inode so that this doesn't have to be done
individually in each filesystem.

Signed-off-by: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
---
 fs/inode.c              | 25 ++++++++++++++++++-----
 include/linux/fs.h      | 12 +++++++++--
 include/linux/richacl.h | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+), 7 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index f96d2a6f88cc..b96a16d5c653 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -18,6 +18,7 @@
 #include <linux/buffer_head.h> /* for inode_has_buffers */
 #include <linux/ratelimit.h>
 #include <linux/list_lru.h>
+#include <linux/richacl.h>
 #include "internal.h"
 
 /*
@@ -185,7 +186,12 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
 	inode->i_mapping = mapping;
 	INIT_HLIST_HEAD(&inode->i_dentry);	/* buggered by rcu freeing */
 #ifdef CONFIG_FS_POSIX_ACL
-	inode->i_acl = inode->i_default_acl = ACL_NOT_CACHED;
+	if (IS_POSIXACL(inode))
+		inode->i_acl = inode->i_default_acl = ACL_NOT_CACHED;
+#endif
+#ifdef CONFIG_FS_RICHACL
+	if (IS_RICHACL(inode))
+		inode->i_richacl = ACL_NOT_CACHED;
 #endif
 
 #ifdef CONFIG_FSNOTIFY
@@ -240,10 +246,19 @@ void __destroy_inode(struct inode *inode)
 	}
 
 #ifdef CONFIG_FS_POSIX_ACL
-	if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
-		posix_acl_release(inode->i_acl);
-	if (inode->i_default_acl && inode->i_default_acl != ACL_NOT_CACHED)
-		posix_acl_release(inode->i_default_acl);
+	if (IS_POSIXACL(inode)) {
+		if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
+			posix_acl_release(inode->i_acl);
+		if (inode->i_default_acl &&
+		    inode->i_default_acl != ACL_NOT_CACHED)
+			posix_acl_release(inode->i_default_acl);
+	}
+#endif
+#ifdef CONFIG_FS_RICHACL
+	if (IS_RICHACL(inode)) {
+		if (inode->i_richacl && inode->i_richacl != ACL_NOT_CACHED)
+			richacl_put(inode->i_richacl);
+	}
 #endif
 	this_cpu_dec(nr_inodes);
 }
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 22d85798b520..95df64d21e55 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -492,6 +492,7 @@ static inline int mapping_writably_mapped(struct address_space *mapping)
 #endif
 
 struct posix_acl;
+struct richacl;
 #define ACL_NOT_CACHED ((void *)(-1))
 
 #define IOP_FASTPERM	0x0001
@@ -510,10 +511,17 @@ struct inode {
 	kgid_t			i_gid;
 	unsigned int		i_flags;
 
+	union {
 #ifdef CONFIG_FS_POSIX_ACL
-	struct posix_acl	*i_acl;
-	struct posix_acl	*i_default_acl;
+		struct {
+			struct posix_acl *i_acl;
+			struct posix_acl *i_default_acl;
+		};
 #endif
+#ifdef CONFIG_FS_RICHACL
+		struct richacl	*i_richacl;
+#endif
+	};
 
 	const struct inode_operations	*i_op;
 	struct super_block	*i_sb;
diff --git a/include/linux/richacl.h b/include/linux/richacl.h
index 88f95d78b897..a7db341e4ee9 100644
--- a/include/linux/richacl.h
+++ b/include/linux/richacl.h
@@ -191,6 +191,59 @@ richacl_put(struct richacl *acl)
 		kfree(acl);
 }
 
+#ifdef CONFIG_FS_RICHACL
+static inline struct richacl *get_cached_richacl(struct inode *inode)
+{
+	struct richacl **p, *acl;
+
+	p = &inode->i_richacl;
+	acl = ACCESS_ONCE(*p);
+	if (acl) {
+		spin_lock(&inode->i_lock);
+		acl = *p;
+		if (acl != ACL_NOT_CACHED)
+			acl = richacl_get(acl);
+		spin_unlock(&inode->i_lock);
+	}
+	return acl;
+}
+
+static inline void set_cached_richacl(struct inode *inode,
+				      struct richacl *acl)
+{
+	struct richacl *old = NULL;
+	spin_lock(&inode->i_lock);
+	old = inode->i_richacl;
+	inode->i_richacl = richacl_get(acl);
+	spin_unlock(&inode->i_lock);
+	if (old != ACL_NOT_CACHED)
+		richacl_put(old);
+}
+
+static inline void forget_cached_richacl(struct inode *inode)
+{
+	struct richacl *old = NULL;
+	spin_lock(&inode->i_lock);
+	old = inode->i_richacl;
+	inode->i_richacl = ACL_NOT_CACHED;
+	spin_unlock(&inode->i_lock);
+	if (old != ACL_NOT_CACHED)
+		richacl_put(old);
+}
+
+static inline int negative_cached_richacl(struct inode *inode)
+{
+	struct richacl **p, *acl;
+
+	p = &inode->i_richacl;
+	acl = ACCESS_ONCE(*p);
+	if (acl)
+		return 0;
+	return 1;
+}
+
+#endif
+
 static inline int
 richacl_is_auto_inherit(const struct richacl *acl)
 {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -V1 19/22] vfs: Cache richacl in struct inode

[Dave Chinner]

On Sun, Apr 27, 2014 at 09:44:50PM +0530, Aneesh Kumar K.V wrote:
> From: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> 
> Cache richacls in struct inode so that this doesn't have to be done
> individually in each filesystem.
> 
> Signed-off-by: Andreas Gruenbacher <agruen-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
> ---
>  fs/inode.c              | 25 ++++++++++++++++++-----
>  include/linux/fs.h      | 12 +++++++++--
>  include/linux/richacl.h | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 83 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index f96d2a6f88cc..b96a16d5c653 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -18,6 +18,7 @@
>  #include <linux/buffer_head.h> /* for inode_has_buffers */
>  #include <linux/ratelimit.h>
>  #include <linux/list_lru.h>
> +#include <linux/richacl.h>
>  #include "internal.h"
>  
>  /*
> @@ -185,7 +186,12 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
>  	inode->i_mapping = mapping;
>  	INIT_HLIST_HEAD(&inode->i_dentry);	/* buggered by rcu freeing */
>  #ifdef CONFIG_FS_POSIX_ACL
> -	inode->i_acl = inode->i_default_acl = ACL_NOT_CACHED;
> +	if (IS_POSIXACL(inode))
> +		inode->i_acl = inode->i_default_acl = ACL_NOT_CACHED;
> +#endif
> +#ifdef CONFIG_FS_RICHACL
> +	if (IS_RICHACL(inode))
> +		inode->i_richacl = ACL_NOT_CACHED;
>  #endif

That's just plain wrong. i think this is what Christoph didn't like.
An inode can have either a POSIX ACL or a RICH ACL, so there is no
need for multiple pointers in the inode for them.

>  
>  #ifdef CONFIG_FSNOTIFY
> @@ -240,10 +246,19 @@ void __destroy_inode(struct inode *inode)
>  	}
>  
>  #ifdef CONFIG_FS_POSIX_ACL
> -	if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
> -		posix_acl_release(inode->i_acl);
> -	if (inode->i_default_acl && inode->i_default_acl != ACL_NOT_CACHED)
> -		posix_acl_release(inode->i_default_acl);
> +	if (IS_POSIXACL(inode)) {
> +		if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
> +			posix_acl_release(inode->i_acl);
> +		if (inode->i_default_acl &&
> +		    inode->i_default_acl != ACL_NOT_CACHED)
> +			posix_acl_release(inode->i_default_acl);
> +	}
> +#endif
> +#ifdef CONFIG_FS_RICHACL
> +	if (IS_RICHACL(inode)) {
> +		if (inode->i_richacl && inode->i_richacl != ACL_NOT_CACHED)
> +			richacl_put(inode->i_richacl);
> +	}
>  #endif

	if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
		acl_release(inode->i_acl);

And all the mess of working out what acl needs releasing get taken
out of of this code.

> index 22d85798b520..95df64d21e55 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -492,6 +492,7 @@ static inline int mapping_writably_mapped(struct address_space *mapping)
>  #endif
>  
>  struct posix_acl;
> +struct richacl;
>  #define ACL_NOT_CACHED ((void *)(-1))
>  
>  #define IOP_FASTPERM	0x0001
> @@ -510,10 +511,17 @@ struct inode {
>  	kgid_t			i_gid;
>  	unsigned int		i_flags;
>  
> +	union {
>  #ifdef CONFIG_FS_POSIX_ACL
> -	struct posix_acl	*i_acl;
> -	struct posix_acl	*i_default_acl;
> +		struct {
> +			struct posix_acl *i_acl;
> +			struct posix_acl *i_default_acl;
> +		};
>  #endif
> +#ifdef CONFIG_FS_RICHACL
> +		struct richacl	*i_richacl;
> +#endif
> +	};

>  
>  	const struct inode_operations	*i_op;
>  	struct super_block	*i_sb;
> diff --git a/include/linux/richacl.h b/include/linux/richacl.h
> index 88f95d78b897..a7db341e4ee9 100644
> --- a/include/linux/richacl.h
> +++ b/include/linux/richacl.h
> @@ -191,6 +191,59 @@ richacl_put(struct richacl *acl)
>  		kfree(acl);
>  }
>  
> +#ifdef CONFIG_FS_RICHACL
> +static inline struct richacl *get_cached_richacl(struct inode *inode)
> +{
> +	struct richacl **p, *acl;
> +
> +	p = &inode->i_richacl;
> +	acl = ACCESS_ONCE(*p);
> +	if (acl) {
> +		spin_lock(&inode->i_lock);
> +		acl = *p;
> +		if (acl != ACL_NOT_CACHED)
> +			acl = richacl_get(acl);
> +		spin_unlock(&inode->i_lock);
> +	}
> +	return acl;
> +}
> +
> +static inline void set_cached_richacl(struct inode *inode,
> +				      struct richacl *acl)
> +{
> +	struct richacl *old = NULL;
> +	spin_lock(&inode->i_lock);
> +	old = inode->i_richacl;
> +	inode->i_richacl = richacl_get(acl);
> +	spin_unlock(&inode->i_lock);
> +	if (old != ACL_NOT_CACHED)
> +		richacl_put(old);
> +}
> +
> +static inline void forget_cached_richacl(struct inode *inode)
> +{
> +	struct richacl *old = NULL;
> +	spin_lock(&inode->i_lock);
> +	old = inode->i_richacl;
> +	inode->i_richacl = ACL_NOT_CACHED;
> +	spin_unlock(&inode->i_lock);
> +	if (old != ACL_NOT_CACHED)
> +		richacl_put(old);
> +}
> +
> +static inline int negative_cached_richacl(struct inode *inode)
> +{
> +	struct richacl **p, *acl;
> +
> +	p = &inode->i_richacl;
> +	acl = ACCESS_ONCE(*p);
> +	if (acl)
> +		return 0;
> +	return 1;
> +}
> +
> +#endif

This is all just copy-and-paste from the posix_acl code with the RCU
path-walk awareness removed from it. There should only be a single
RCU aware version of these functions that does not care what type of
ACL is in use.

Cheers,

Dave.
-- 
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -V1 19/22] vfs: Cache richacl in struct inode

[Matthew Wilcox]

On Tue, Apr 29, 2014 at 10:52:04AM +1000, Dave Chinner wrote:
> >  #ifdef CONFIG_FSNOTIFY
> > @@ -240,10 +246,19 @@ void __destroy_inode(struct inode *inode)
> >  	}
> >  
> >  #ifdef CONFIG_FS_POSIX_ACL
> > -	if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
> > -		posix_acl_release(inode->i_acl);
> > -	if (inode->i_default_acl && inode->i_default_acl != ACL_NOT_CACHED)
> > -		posix_acl_release(inode->i_default_acl);
> > +	if (IS_POSIXACL(inode)) {
> > +		if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
> > +			posix_acl_release(inode->i_acl);
> > +		if (inode->i_default_acl &&
> > +		    inode->i_default_acl != ACL_NOT_CACHED)
> > +			posix_acl_release(inode->i_default_acl);
> > +	}
> > +#endif
> > +#ifdef CONFIG_FS_RICHACL
> > +	if (IS_RICHACL(inode)) {
> > +		if (inode->i_richacl && inode->i_richacl != ACL_NOT_CACHED)
> > +			richacl_put(inode->i_richacl);
> > +	}
> >  #endif
> 
> 	if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
> 		acl_release(inode->i_acl);
> 
> And all the mess of working out what acl needs releasing get taken
> out of of this code.

Let's go further and have the API simply:

	inode_acl_release(inode);

with inode_acl_release looking something like:

	if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
		acl_release(inode->i_acl);
	if (inode->i_default_acl && inode->i_default_acl != ACL_NOT_CACHED)
		acl_release(inode->i_default_acl);

Re: [PATCH -V1 19/22] vfs: Cache richacl in struct inode

[Aneesh Kumar K.V]

Dave Chinner <david@fromorbit.com> writes:

> On Sun, Apr 27, 2014 at 09:44:50PM +0530, Aneesh Kumar K.V wrote:
>> From: Andreas Gruenbacher <agruen@kernel.org>
>> 
>> Cache richacls in struct inode so that this doesn't have to be done
>> individually in each filesystem.
>> 
>> Signed-off-by: Andreas Gruenbacher <agruen@kernel.org>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>>  fs/inode.c              | 25 ++++++++++++++++++-----
>>  include/linux/fs.h      | 12 +++++++++--
>>  include/linux/richacl.h | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 83 insertions(+), 7 deletions(-)
>> 
>> diff --git a/fs/inode.c b/fs/inode.c
>> index f96d2a6f88cc..b96a16d5c653 100644
>> --- a/fs/inode.c
>> +++ b/fs/inode.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/buffer_head.h> /* for inode_has_buffers */
>>  #include <linux/ratelimit.h>
>>  #include <linux/list_lru.h>
>> +#include <linux/richacl.h>
>>  #include "internal.h"
>>  
>>  /*
>> @@ -185,7 +186,12 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
>>  	inode->i_mapping = mapping;
>>  	INIT_HLIST_HEAD(&inode->i_dentry);	/* buggered by rcu freeing */
>>  #ifdef CONFIG_FS_POSIX_ACL
>> -	inode->i_acl = inode->i_default_acl = ACL_NOT_CACHED;
>> +	if (IS_POSIXACL(inode))
>> +		inode->i_acl = inode->i_default_acl = ACL_NOT_CACHED;
>> +#endif
>> +#ifdef CONFIG_FS_RICHACL
>> +	if (IS_RICHACL(inode))
>> +		inode->i_richacl = ACL_NOT_CACHED;
>>  #endif
>
> That's just plain wrong. i think this is what Christoph didn't like.
> An inode can have either a POSIX ACL or a RICH ACL, so there is no
> need for multiple pointers in the inode for them.

We don't really have multiple pointers

	union {
#ifdef CONFIG_FS_POSIX_ACL
		struct {
			struct posix_acl *i_acl;
			struct posix_acl *i_default_acl;
		};
#endif
#ifdef CONFIG_FS_RICHACL
		struct richacl	*i_richacl;
#endif
	};


But I understand what you are suggesting here. Will try to rework and
see how much easy/simpler it makes the code. 

>
>>  
>>  #ifdef CONFIG_FSNOTIFY
>> @@ -240,10 +246,19 @@ void __destroy_inode(struct inode *inode)
>>  	}
>>  
>>  #ifdef CONFIG_FS_POSIX_ACL
>> -	if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
>> -		posix_acl_release(inode->i_acl);
>> -	if (inode->i_default_acl && inode->i_default_acl != ACL_NOT_CACHED)
>> -		posix_acl_release(inode->i_default_acl);
>> +	if (IS_POSIXACL(inode)) {
>> +		if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
>> +			posix_acl_release(inode->i_acl);
>> +		if (inode->i_default_acl &&
>> +		    inode->i_default_acl != ACL_NOT_CACHED)
>> +			posix_acl_release(inode->i_default_acl);
>> +	}
>> +#endif
>> +#ifdef CONFIG_FS_RICHACL
>> +	if (IS_RICHACL(inode)) {
>> +		if (inode->i_richacl && inode->i_richacl != ACL_NOT_CACHED)
>> +			richacl_put(inode->i_richacl);
>> +	}
>>  #endif
>
> 	if (inode->i_acl && inode->i_acl != ACL_NOT_CACHED)
> 		acl_release(inode->i_acl);
>
> And all the mess of working out what acl needs releasing get taken
> out of of this code.

Understood. Will try to see how much I can simplify.

>
>> index 22d85798b520..95df64d21e55 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -492,6 +492,7 @@ static inline int mapping_writably_mapped(struct address_space *mapping)
>>  #endif
>>  
>>  struct posix_acl;
>> +struct richacl;
>>  #define ACL_NOT_CACHED ((void *)(-1))
>>  
>>  #define IOP_FASTPERM	0x0001
>> @@ -510,10 +511,17 @@ struct inode {
>>  	kgid_t			i_gid;
>>  	unsigned int		i_flags;
>>  
>> +	union {
>>  #ifdef CONFIG_FS_POSIX_ACL
>> -	struct posix_acl	*i_acl;
>> -	struct posix_acl	*i_default_acl;
>> +		struct {
>> +			struct posix_acl *i_acl;
>> +			struct posix_acl *i_default_acl;
>> +		};
>>  #endif
>> +#ifdef CONFIG_FS_RICHACL
>> +		struct richacl	*i_richacl;
>> +#endif
>> +	};
>
>>  
>>  	const struct inode_operations	*i_op;
>>  	struct super_block	*i_sb;
>> diff --git a/include/linux/richacl.h b/include/linux/richacl.h
>> index 88f95d78b897..a7db341e4ee9 100644
>> --- a/include/linux/richacl.h
>> +++ b/include/linux/richacl.h
>> @@ -191,6 +191,59 @@ richacl_put(struct richacl *acl)
>>  		kfree(acl);
>>  }
>>  
>> +#ifdef CONFIG_FS_RICHACL
>> +static inline struct richacl *get_cached_richacl(struct inode *inode)
>> +{
>> +	struct richacl **p, *acl;
>> +
>> +	p = &inode->i_richacl;
>> +	acl = ACCESS_ONCE(*p);
>> +	if (acl) {
>> +		spin_lock(&inode->i_lock);
>> +		acl = *p;
>> +		if (acl != ACL_NOT_CACHED)
>> +			acl = richacl_get(acl);
>> +		spin_unlock(&inode->i_lock);
>> +	}
>> +	return acl;
>> +}
>> +
>> +static inline void set_cached_richacl(struct inode *inode,
>> +				      struct richacl *acl)
>> +{
>> +	struct richacl *old = NULL;
>> +	spin_lock(&inode->i_lock);
>> +	old = inode->i_richacl;
>> +	inode->i_richacl = richacl_get(acl);
>> +	spin_unlock(&inode->i_lock);
>> +	if (old != ACL_NOT_CACHED)
>> +		richacl_put(old);
>> +}
>> +
>> +static inline void forget_cached_richacl(struct inode *inode)
>> +{
>> +	struct richacl *old = NULL;
>> +	spin_lock(&inode->i_lock);
>> +	old = inode->i_richacl;
>> +	inode->i_richacl = ACL_NOT_CACHED;
>> +	spin_unlock(&inode->i_lock);
>> +	if (old != ACL_NOT_CACHED)
>> +		richacl_put(old);
>> +}
>> +
>> +static inline int negative_cached_richacl(struct inode *inode)
>> +{
>> +	struct richacl **p, *acl;
>> +
>> +	p = &inode->i_richacl;
>> +	acl = ACCESS_ONCE(*p);
>> +	if (acl)
>> +		return 0;
>> +	return 1;
>> +}
>> +
>> +#endif
>
> This is all just copy-and-paste from the posix_acl code with the RCU
> path-walk awareness removed from it. There should only be a single
> RCU aware version of these functions that does not care what type of
> ACL is in use.
>

Ok.

-aneesh

Re: [PATCH -V1 00/22] New ACL format for better NFSv4 acl interoperability

[Dave Chinner]

On Sun, Apr 27, 2014 at 09:44:31PM +0530, Aneesh Kumar K.V wrote:
> Hi
> 
> As per LSF/MM summit discussion I am reposting the richacl patchset for
> upstream inclusion. The patchset includes minimal changes required to implement
> a new acl model similar to NFSv4 ACL. The acl model selection is based on
> file system feature flag. 
> 
> The following set of patches implements VFS and ext4 changes needed to implement
> a new acl model for linux. Rich ACLs are an implementation of NFSv4 ACLs,
> extended by file masks to fit into the standard POSIX file permission model.
> They are designed to work seamlessly locally as well as across the NFSv4 and
> CIFS/SMB2 network file system protocols.
> 
> A user-space utility for displaying and changing richacls is available at [1]
> (a number of examples can be found at http://acl.bestbits.at/richacl/examples.html).
> 
> [1] git://github.com/kvaneesh/richacl-tools.git master
> 
> To test richacl on ext4, create the file sytem with richacl feature flag
> (mkfs.ext4 -O richacl or  tune2fs -O richacl). With richacl feature enabled
> using mount option "acl" will switch to using richacl instead of posixacl.

No mount options, please. The ACL configuration needs to be
determined solely by the superblock feature bit - we cannot support
filesystems with mixed ACL types, and that's what this mount option
does.

> More details regarding richacl can be found at
> http://acl.bestbits.at/richacl/
> 
> Previous posting of the patchset can be found at:
> http://mid.gmane.org/1319391835-5829-1-git-send-email-aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
> "[PATCH -V8 00/26]  New ACL format for better NFSv4 acl interoperability"
> 
> The complete patchset can also be found at:
> https://github.com/kvaneesh/linux/commits/richacl-for-upstream

Where are the tests? We need comprehensive coverage in xfstests so
we can validate that it works the way it is supposed to and that we
don't break it in future, and that all filesystems behave the same
way....

Cheers,

Dave.
-- 
Dave Chinner
david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -V1 00/22] New ACL format for better NFSv4 acl interoperability

[Aneesh Kumar K.V]

Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> writes:

> On Sun, Apr 27, 2014 at 09:44:31PM +0530, Aneesh Kumar K.V wrote:
>> Hi
>> 
>> As per LSF/MM summit discussion I am reposting the richacl patchset for
>> upstream inclusion. The patchset includes minimal changes required to implement
>> a new acl model similar to NFSv4 ACL. The acl model selection is based on
>> file system feature flag. 
>> 
>> The following set of patches implements VFS and ext4 changes needed to implement
>> a new acl model for linux. Rich ACLs are an implementation of NFSv4 ACLs,
>> extended by file masks to fit into the standard POSIX file permission model.
>> They are designed to work seamlessly locally as well as across the NFSv4 and
>> CIFS/SMB2 network file system protocols.
>> 
>> A user-space utility for displaying and changing richacls is available at [1]
>> (a number of examples can be found at http://acl.bestbits.at/richacl/examples.html).
>> 
>> [1] git://github.com/kvaneesh/richacl-tools.git master
>> 
>> To test richacl on ext4, create the file sytem with richacl feature flag
>> (mkfs.ext4 -O richacl or  tune2fs -O richacl). With richacl feature enabled
>> using mount option "acl" will switch to using richacl instead of posixacl.
>
> No mount options, please. The ACL configuration needs to be
> determined solely by the superblock feature bit - we cannot support
> filesystems with mixed ACL types, and that's what this mount option
> does.

For ext4 since acls are enabled by default we really don't need to
speciy -o acl in mount. What i meant by above is that using "acl/noacl" mount
option will now enabe/disable POSIX or RICHacl based on the superblock
feature bit. 

>
>> More details regarding richacl can be found at
>> http://acl.bestbits.at/richacl/
>> 
>> Previous posting of the patchset can be found at:
>> http://mid.gmane.org/1319391835-5829-1-git-send-email-aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
>> "[PATCH -V8 00/26]  New ACL format for better NFSv4 acl interoperability"
>> 
>> The complete patchset can also be found at:
>> https://github.com/kvaneesh/linux/commits/richacl-for-upstream
>
> Where are the tests? We need comprehensive coverage in xfstests so
> we can validate that it works the way it is supposed to and that we
> don't break it in future, and that all filesystems behave the same
> way....
>

https://github.com/kvaneesh/richacl-tools/tree/master/test


-aneesh

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -V1 00/22] New ACL format for better NFSv4 acl interoperability

[Dave Chinner]

On Mon, Apr 28, 2014 at 10:54:52AM +0530, Aneesh Kumar K.V wrote:
> Dave Chinner <david@fromorbit.com> writes:
> > On Sun, Apr 27, 2014 at 09:44:31PM +0530, Aneesh Kumar K.V wrote:
> >> Hi
> >> 
> >> As per LSF/MM summit discussion I am reposting the richacl patchset for
> >> upstream inclusion. The patchset includes minimal changes required to implement
> >> a new acl model similar to NFSv4 ACL. The acl model selection is based on
> >> file system feature flag. 
> >> 
> >> The following set of patches implements VFS and ext4 changes needed to implement
> >> a new acl model for linux. Rich ACLs are an implementation of NFSv4 ACLs,
> >> extended by file masks to fit into the standard POSIX file permission model.
> >> They are designed to work seamlessly locally as well as across the NFSv4 and
> >> CIFS/SMB2 network file system protocols.
> >> 
> >> A user-space utility for displaying and changing richacls is available at [1]
> >> (a number of examples can be found at http://acl.bestbits.at/richacl/examples.html).
....
> >> More details regarding richacl can be found at
> >> http://acl.bestbits.at/richacl/
> >> 
> >> Previous posting of the patchset can be found at:
> >> http://mid.gmane.org/1319391835-5829-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com
> >> "[PATCH -V8 00/26]  New ACL format for better NFSv4 acl interoperability"
> >> 
> >> The complete patchset can also be found at:
> >> https://github.com/kvaneesh/linux/commits/richacl-for-upstream
> >
> > Where are the tests? We need comprehensive coverage in xfstests so
> > we can validate that it works the way it is supposed to and that we
> > don't break it in future, and that all filesystems behave the same
> > way....
> >
> 
> https://github.com/kvaneesh/richacl-tools/tree/master/test

FYI, I doubt very much that anyone will run a stand-alone richacls
test suite regularly.  Can you please work to integrate this into
xfstests so it becomes a regular part of a filesystem developer's
daily workflow?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH -V1 00/22] New ACL format for better NFSv4 acl interoperability

[Aneesh Kumar K.V]

Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> writes:

> On Mon, Apr 28, 2014 at 10:54:52AM +0530, Aneesh Kumar K.V wrote:
>> Dave Chinner <david-FqsqvQoI3Ljby3iVrkZq2A@public.gmane.org> writes:
>> > On Sun, Apr 27, 2014 at 09:44:31PM +0530, Aneesh Kumar K.V wrote:
>> >> Hi
>> >> 
>> >> As per LSF/MM summit discussion I am reposting the richacl patchset for
>> >> upstream inclusion. The patchset includes minimal changes required to implement
>> >> a new acl model similar to NFSv4 ACL. The acl model selection is based on
>> >> file system feature flag. 
>> >> 
>> >> The following set of patches implements VFS and ext4 changes needed to implement
>> >> a new acl model for linux. Rich ACLs are an implementation of NFSv4 ACLs,
>> >> extended by file masks to fit into the standard POSIX file permission model.
>> >> They are designed to work seamlessly locally as well as across the NFSv4 and
>> >> CIFS/SMB2 network file system protocols.
>> >> 
>> >> A user-space utility for displaying and changing richacls is available at [1]
>> >> (a number of examples can be found at http://acl.bestbits.at/richacl/examples.html).
> ....
>> >> More details regarding richacl can be found at
>> >> http://acl.bestbits.at/richacl/
>> >> 
>> >> Previous posting of the patchset can be found at:
>> >> http://mid.gmane.org/1319391835-5829-1-git-send-email-aneesh.kumar-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org
>> >> "[PATCH -V8 00/26]  New ACL format for better NFSv4 acl interoperability"
>> >> 
>> >> The complete patchset can also be found at:
>> >> https://github.com/kvaneesh/linux/commits/richacl-for-upstream
>> >
>> > Where are the tests? We need comprehensive coverage in xfstests so
>> > we can validate that it works the way it is supposed to and that we
>> > don't break it in future, and that all filesystems behave the same
>> > way....
>> >
>> 
>> https://github.com/kvaneesh/richacl-tools/tree/master/test
>
> FYI, I doubt very much that anyone will run a stand-alone richacls
> test suite regularly.  Can you please work to integrate this into
> xfstests so it becomes a regular part of a filesystem developer's
> daily workflow?
>

Will update xfstest based on the progress made with this patchset.

-aenesh

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -V1 00/22] New ACL format for better NFSv4 acl interoperability

[Christoph Hellwig]

This doesn't address any or the previous points:

 - common implementation instead of the godawful boilerplate code
   (and we even fixed most of this for Posix ACL by now, so even less
   reason to do the same crap again!)
 - common data structure with Posix ACLs

And of course no real explanation why we need the braindead access/deny
scheme at how it will get properly integrated with the system.

So in this for a clear NAK.

Re: [PATCH -V1 00/22] New ACL format for better NFSv4 acl interoperability

[Aneesh Kumar K.V]

Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> writes:

> This doesn't address any or the previous points:
>
>  - common implementation instead of the godawful boilerplate code
>    (and we even fixed most of this for Posix ACL by now, so even less
>    reason to do the same crap again!)

We already do that with richacl. Richacl already have most of the
details implemented in common code. Comparing to recent posix acl
changes we could still simplify chmod and xattr bits. I will do that
in the next update. 

>  - common data structure with Posix ACLs
>

Can you explain this ?. Why do we want to do that ? 


> And of course no real explanation why we need the braindead access/deny
> scheme at how it will get properly integrated with the system.
>
> So in this for a clear NAK.

-aneesh

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -V1 00/22] New ACL format for better NFSv4 acl interoperability

[Christoph Hellwig]

On Mon, Apr 28, 2014 at 11:24:08AM +0530, Aneesh Kumar K.V wrote:
> We already do that with richacl. Richacl already have most of the
> details implemented in common code. Comparing to recent posix acl
> changes we could still simplify chmod and xattr bits. I will do that
> in the next update. 

There's still tons of duplication.  There should be no code in the
filesystem except for a few callouts for the inode init and chmod path,
and the attr set/get should also be mostly in a library.  If you need
to add more than 20 lines to the filesystem you did something wrong.

> 
> >  - common data structure with Posix ACLs
> >
> 
> Can you explain this ?. Why do we want to do that ? 

One acl structure implementing the Posix and NFSv4 acls and instead
of a big pile of duplicate infrastructure.

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH -V1 00/22] New ACL format for better NFSv4 acl interoperability

[J. Bruce Fields]

On Sun, Apr 27, 2014 at 09:44:31PM +0530, Aneesh Kumar K.V wrote:
> As per LSF/MM summit discussion I am reposting the richacl patchset for
> upstream inclusion. The patchset includes minimal changes required to implement
> a new acl model similar to NFSv4 ACL. The acl model selection is based on
> file system feature flag. 

It'd also be nice to see the nfsd bits, as that (along with samba) is
one of the main users.

(And the richacl->NFSv4 acl mapping is tricky thanks to the mask bits.)

--b.

> 
> The following set of patches implements VFS and ext4 changes needed to implement
> a new acl model for linux. Rich ACLs are an implementation of NFSv4 ACLs,
> extended by file masks to fit into the standard POSIX file permission model.
> They are designed to work seamlessly locally as well as across the NFSv4 and
> CIFS/SMB2 network file system protocols.
> 
> A user-space utility for displaying and changing richacls is available at [1]
> (a number of examples can be found at http://acl.bestbits.at/richacl/examples.html).
> 
> [1] git://github.com/kvaneesh/richacl-tools.git master
> 
> To test richacl on ext4, create the file sytem with richacl feature flag
> (mkfs.ext4 -O richacl or  tune2fs -O richacl). With richacl feature enabled
> using mount option "acl" will switch to using richacl instead of posixacl.
> 
> More details regarding richacl can be found at
> http://acl.bestbits.at/richacl/
> 
> Previous posting of the patchset can be found at:
> http://mid.gmane.org/1319391835-5829-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com
> "[PATCH -V8 00/26]  New ACL format for better NFSv4 acl interoperability"
> 
> The complete patchset can also be found at:
> https://github.com/kvaneesh/linux/commits/richacl-for-upstream
> 
> Since we are trying to get the changes merged upstream after a long time, I am
> posting this as V1 again. I Also dropped the Acked-by tag from
> David Howells <dhowells@redhat.com> and J. Bruce Fields <bfields@redhat.com>.
> Please let me know if I can add them back again.
> 
> -aneesh
>