[RFC] POSIX ACL kernel infrastructure

[RFC] POSIX ACL kernel infrastructure

[Andreas Gruenbacher]

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

Hello,

I want to propose adding some infrastructure to the VFS for POSIX ACL 
ssupport. There are already implementations for XFS and Ext2/Ext3; ReiserFS 
is to follow soon.

The design is as follows: POSIX ACLs are passed between the kernel and user 
space as Extended Attributes. The patches I've mentioned are doing exactly 
that. The filesystems are responsible for handling ACLs; they are not limited 
to POSIX ACLs.
   Filesystems may choose to cache/manipulate ACLs in the kernel internal 
format that this patch implements, or differently. In any case they should 
implement the get_posix_acl and set_posix_acl inode operations if mapping 
between native filesystem ACLs and POSIX ACLs makes sense.
   Other parts of the kernel (such as nfsd) can use the get_posix_acl and 
set_posix_acl inode operations to manipulate POSIX ACLs. This is currently 
required for nfsd, since nfsd must mask off permissions depending on the 
protocol and client version (a module parameter controls this behavior).

(This patch also requires the ENOTSUP and MS_NOUMASK issues to be resolved.
I've posted messages on these topics to linux-kernel before, and I will send 
copies to linux-fsdevel.)

Regards,
Andreas.

------------------------------------------------------------------
 Andreas Gruenbacher                                SuSE Linux AG
 mailto:agruen@suse.de                     Deutschherrnstr. 15-19
 http://www.suse.de/                   D-90429 Nuernberg, Germany

[-- Attachment #2: linux-2.5.30-posix-acl-0.diff.gz --]
[-- Type: application/x-gzip, Size: 4842 bytes --]

Re: [RFC] POSIX ACL kernel infrastructure

[Christoph Hellwig]

On Sun, Aug 04, 2002 at 03:46:35PM +0200, Andreas Gruenbacher wrote:
> Hello,
> 
> I want to propose adding some infrastructure to the VFS for POSIX ACL 
> ssupport. There are already implementations for XFS and Ext2/Ext3; ReiserFS 
> is to follow soon.

Could you please resend without mime and gzip?

> 
> ------------------------------------------------------------------
>  Andreas Gruenbacher                                SuSE Linux AG
>  mailto:agruen@suse.de                     Deutschherrnstr. 15-19
>  http://www.suse.de/                   D-90429 Nuernberg, Germany


---end quoted text---

Re: [RFC] POSIX ACL kernel infrastructure

[Andreas Gruenbacher]

On Sunday 04 August 2002 16:04, you wrote:
> On Sun, Aug 04, 2002 at 03:46:35PM +0200, Andreas Gruenbacher wrote:
> > Hello,
> >
> > I want to propose adding some infrastructure to the VFS for POSIX ACL
> > ssupport. There are already implementations for XFS and Ext2/Ext3;
> > ReiserFS is to follow soon.
>
> Could you please resend without mime and gzip?

Well, sorry the patch is rather large uncompressed.

Cheers,
Andreas.

diff -Nur --exclude-from=linux.excl linux-2.5.30/fs/Config.help 
linux-2.5.30.patch/fs/Config.help
--- linux-2.5.30/fs/Config.help	Thu Aug  1 23:16:33 2002
+++ linux-2.5.30.patch/fs/Config.help	Sun Aug  4 13:38:57 2002
@@ -1,3 +1,20 @@
+CONFIG_FS_POSIX_ACL
+  POSIX Access Control Lists (ACLs) support permissions for users and
+  groups beyond the owner/group/world scheme.
+
+  To learn more about Access Control Lists, visit
+  <http://acl.bestbits.at/>.
+
+  You need an additional filesystem patch to get POSIX ACL support
+  on one of the built-in filesystems; such patches are available at
+  http://acl.bestbits.at/ for ext2 and ext3, and at
+  http://oss.sgi.com/projects/xfs/ for XFS.
+  
+  The getfacl and setfacl utilities for manipulating POSIX ACLs are also
+  available at these web sites.
+
+  If you don't know what Access Control Lists are, say N.
+
 CONFIG_QUOTA
   If you say Y here, you will be able to set per user limits for disk
   usage (also called disk quotas). Currently, it works for the
diff -Nur --exclude-from=linux.excl linux-2.5.30/fs/Config.in 
linux-2.5.30.patch/fs/Config.in
--- linux-2.5.30/fs/Config.in	Thu Aug  1 23:16:29 2002
+++ linux-2.5.30.patch/fs/Config.in	Sun Aug  4 13:24:12 2002
@@ -4,6 +4,8 @@
 mainmenu_option next_comment
 comment 'File systems'
 
+bool 'POSIX Access Control Lists' CONFIG_FS_POSIX_ACL
+
 bool 'Quota support' CONFIG_QUOTA
 dep_tristate '  Old quota format support' CONFIG_QFMT_V1 $CONFIG_QUOTA
 dep_tristate '  Quota format v2 support' CONFIG_QFMT_V2 $CONFIG_QUOTA
diff -Nur --exclude-from=linux.excl linux-2.5.30/fs/Makefile 
linux-2.5.30.patch/fs/Makefile
--- linux-2.5.30/fs/Makefile	Thu Aug  1 23:16:03 2002
+++ linux-2.5.30.patch/fs/Makefile	Sun Aug  4 13:28:38 2002
@@ -7,7 +7,8 @@
 
 O_TARGET := fs.o
 
-export-objs :=	open.o dcache.o buffer.o bio.o inode.o dquot.o mpage.o
+export-objs :=	open.o dcache.o buffer.o bio.o inode.o dquot.o mpage.o \
+		posix_acl.o
 
 obj-y :=	open.o read_write.o devices.o file_table.o buffer.o \
 		bio.o super.o block_dev.o char_dev.o stat.o exec.o pipe.o \
@@ -36,6 +37,8 @@
 obj-$(CONFIG_QFMT_V2)		+= quota_v2.o
 obj-$(CONFIG_QUOTACTL)		+= quota.o
 
+obj-$(CONFIG_FS_POSIX_ACL)	+= posix_acl.o
+
 obj-$(CONFIG_PROC_FS)		+= proc/
 obj-y				+= partitions/
 obj-y				+= driverfs/
diff -Nur --exclude-from=linux.excl linux-2.5.30/fs/posix_acl.c 
linux-2.5.30.patch/fs/posix_acl.c
--- linux-2.5.30/fs/posix_acl.c	Thu Jan  1 01:00:00 1970
+++ linux-2.5.30.patch/fs/posix_acl.c	Sun Aug  4 14:52:42 2002
@@ -0,0 +1,496 @@
+/*
+ * linux/fs/posix_acl.c
+ *
+ *  Copyright (C) 2001 by Andreas Gruenbacher <a.gruenbacher@computer.org>
+ *
+ *  Fixes from William Schumacher incorporated on 15 March 2001.
+ *     (Reported by Charles Bertsch, <CBertsch@microtest.com>).
+ */
+
+/*
+ *  This file contains generic functions for manipulating
+ *  POSIX 1003.1e draft standard 17 ACLs.
+ */
+
+#include <linux/version.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <asm/atomic.h>
+#include <linux/fs.h>
+#include <linux/posix_acl.h>
+#include <linux/module.h>
+
+#include <linux/smp_lock.h>
+#include <linux/errno.h>
+
+MODULE_AUTHOR("Andreas Gruenbacher <a.gruenbacher@computer.org>");
+MODULE_DESCRIPTION("Generic Posix Access Control List (ACL) Manipulation");
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0)
+MODULE_LICENSE("GPL");
+#endif
+
+EXPORT_SYMBOL(posix_acl_alloc);
+EXPORT_SYMBOL(posix_acl_dup);
+EXPORT_SYMBOL(posix_acl_clone);
+EXPORT_SYMBOL(posix_acl_release);
+EXPORT_SYMBOL(posix_acl_valid);
+EXPORT_SYMBOL(posix_acl_equiv_mode);
+EXPORT_SYMBOL(posix_acl_from_mode);
+EXPORT_SYMBOL(posix_acl_create_masq);
+EXPORT_SYMBOL(posix_acl_chmod_masq);
+EXPORT_SYMBOL(posix_acl_masq_nfs_mode);
+EXPORT_SYMBOL(posix_acl_permission);
+
+EXPORT_SYMBOL(get_posix_acl);
+EXPORT_SYMBOL(set_posix_acl);
+
+/*
+ * Allocate a new ACL with the specified number of entries.
+ */
+posix_acl_t *
+posix_acl_alloc(int count)
+{
+	const size_t size = sizeof(posix_acl_t) +
+	                    count * sizeof(posix_acl_entry_t);
+	posix_acl_t *acl = kmalloc(size, GFP_KERNEL);
+	if (acl) {
+		atomic_set(&acl->a_refcount, 1);
+		acl->a_count = count;
+	}
+	return acl;
+}
+
+/*
+ * Duplicate an ACL handle.
+ */
+posix_acl_t *
+posix_acl_dup(posix_acl_t *acl)
+{
+	if (acl)
+		atomic_inc(&acl->a_refcount);
+	return acl;
+}
+
+/*
+ * Clone an ACL.
+ */
+posix_acl_t *
+posix_acl_clone(const posix_acl_t *acl)
+{
+	posix_acl_t *clone = NULL;
+
+	if (acl) {
+		int size = sizeof(posix_acl_t) + acl->a_count *
+		           sizeof(posix_acl_entry_t);
+		clone = kmalloc(size, GFP_KERNEL);
+		if (clone) {
+			memcpy(clone, acl, size);
+			atomic_set(&clone->a_refcount, 1);
+		}
+	}
+	return clone;
+}
+
+/*
+ * Free an ACL handle.
+ */
+void
+posix_acl_release(posix_acl_t *acl)
+{
+	if (acl && atomic_dec_and_test(&acl->a_refcount))
+		kfree(acl);
+}
+
+/*
+ * Check if an acl is valid. Returns 0 if it is, or -E... otherwise.
+ */
+int
+posix_acl_valid(const posix_acl_t *acl)
+{
+	const posix_acl_entry_t *pa, *pe;
+	int state = ACL_USER_OBJ;
+	unsigned int id = 0;  /* keep gcc happy */
+	int needs_mask = 0;
+
+	FOREACH_ACL_ENTRY(pa, acl, pe) {
+		if (pa->e_perm & ~(ACL_READ|ACL_WRITE|ACL_EXECUTE))
+			return -EINVAL;
+		switch (pa->e_tag) {
+			case ACL_USER_OBJ:
+				if (state == ACL_USER_OBJ) {
+					id = 0;
+					state = ACL_USER;
+					break;
+				}
+				return -EINVAL;
+
+			case ACL_USER:
+				if (state != ACL_USER)
+					return -EINVAL;
+				if (pa->e_id == ACL_UNDEFINED_ID ||
+				    pa->e_id < id)
+					return -EINVAL;
+				id = pa->e_id + 1;
+				needs_mask = 1;
+				break;
+
+			case ACL_GROUP_OBJ:
+				if (state == ACL_USER) {
+					id = 0;
+					state = ACL_GROUP;
+					break;
+				}
+				return -EINVAL;
+
+			case ACL_GROUP:
+				if (state != ACL_GROUP)
+					return -EINVAL;
+				if (pa->e_id == ACL_UNDEFINED_ID ||
+				    pa->e_id < id)
+					return -EINVAL;
+				id = pa->e_id + 1;
+				needs_mask = 1;
+				break;
+
+			case ACL_MASK:
+				if (state != ACL_GROUP)
+					return -EINVAL;
+				state = ACL_OTHER;
+				break;
+
+			case ACL_OTHER:
+				if (state == ACL_OTHER ||
+				    (state == ACL_GROUP && !needs_mask)) {
+					state = 0;
+					break;
+				}
+				return -EINVAL;
+
+			default:
+				return -EINVAL;
+		}
+	}
+	if (state == 0)
+		return 0;
+	return -EINVAL;
+}
+
+/*
+ * Returns 0 if the acl can be exactly represented in the traditional
+ * file mode permission bits, or else 1. Returns -E... on error.
+ */
+int
+posix_acl_equiv_mode(const posix_acl_t *acl, mode_t *mode_p)
+{
+	const posix_acl_entry_t *pa, *pe;
+	mode_t mode = 0;
+	int not_equiv = 0;
+
+	FOREACH_ACL_ENTRY(pa, acl, pe) {
+		switch (pa->e_tag) {
+			case ACL_USER_OBJ:
+				mode |= (pa->e_perm & S_IRWXO) << 6;
+				break;
+			case ACL_GROUP_OBJ:
+				mode |= (pa->e_perm & S_IRWXO) << 3;
+				break;
+			case ACL_OTHER:
+				mode |= pa->e_perm & S_IRWXO;
+				break;
+			case ACL_MASK:
+				mode = (mode & ~S_IRWXG) |
+				       ((pa->e_perm & S_IRWXO) << 3);
+				not_equiv = 1;
+				break;
+			case ACL_USER:
+			case ACL_GROUP:
+				not_equiv = 1;
+				break;
+			default:
+				return -EINVAL;
+		}
+	}
+        if (mode_p)
+                *mode_p = (*mode_p & ~S_IRWXUGO) | mode;
+        return not_equiv;
+}
+
+/*
+ * Create an ACL representing the file mode permission bits of an inode.
+ */
+posix_acl_t *
+posix_acl_from_mode(mode_t mode)
+{
+	posix_acl_t *acl = posix_acl_alloc(3);
+	if (!acl)
+		return ERR_PTR(-ENOMEM);
+
+	acl->a_entries[0].e_tag  = ACL_USER_OBJ;
+	acl->a_entries[0].e_id   = ACL_UNDEFINED_ID;
+	acl->a_entries[0].e_perm = (mode & S_IRWXU) >> 6;
+
+	acl->a_entries[1].e_tag  = ACL_GROUP_OBJ;
+	acl->a_entries[1].e_id   = ACL_UNDEFINED_ID;
+	acl->a_entries[1].e_perm = (mode & S_IRWXG) >> 3;
+
+	acl->a_entries[2].e_tag  = ACL_OTHER;
+	acl->a_entries[2].e_id   = ACL_UNDEFINED_ID;
+	acl->a_entries[2].e_perm = (mode & S_IRWXO);
+	return acl;
+}
+
+/*
+ * Return 0 if current is granted want access to the inode
+ * by the acl. Returns -E... otherwise.
+ */
+int
+posix_acl_permission(struct inode *inode, const posix_acl_t *acl, int want)
+{
+	const posix_acl_entry_t *pa, *pe, *mask_obj;
+	int found = 0;
+
+	FOREACH_ACL_ENTRY(pa, acl, pe) {
+                switch(pa->e_tag) {
+                        case ACL_USER_OBJ:
+				/* (May have been checked already) */
+                                if (inode->i_uid == current->fsuid)
+                                        goto check_perm;
+                                break;
+                        case ACL_USER:
+                                if (pa->e_id == current->fsuid)
+                                        goto mask;
+				break;
+                        case ACL_GROUP_OBJ:
+                                if (in_group_p(inode->i_gid)) {
+					found = 1;
+					if ((pa->e_perm & want) == want)
+						goto mask;
+                                }
+				break;
+                        case ACL_GROUP:
+                                if (in_group_p(pa->e_id)) {
+					found = 1;
+					if ((pa->e_perm & want) == want)
+						goto mask;
+                                }
+                                break;
+                        case ACL_MASK:
+                                break;
+                        case ACL_OTHER:
+				if (found)
+					return -EACCES;
+				else
+					goto check_perm;
+			default:
+				return -EIO;
+                }
+        }
+	return -EIO;
+
+mask:
+	for (mask_obj = pa+1; mask_obj != pe; mask_obj++) {
+		if (mask_obj->e_tag == ACL_MASK) {
+			if ((pa->e_perm & mask_obj->e_perm & want) == want)
+				return 0;
+			return -EACCES;
+		}
+	}
+
+check_perm:
+	if ((pa->e_perm & want) == want)
+		return 0;
+	return -EACCES;
+}
+
+/*
+ * Modify acl when creating a new inode. The caller must ensure the acl is
+ * only referenced once.
+ *
+ * mode_p initially must contain the mode parameter to the open() / creat()
+ * system calls. All permissions that are not granted by the acl are removed.
+ * The permissions in the acl are changed to reflect the mode_p parameter.
+ */
+int
+posix_acl_create_masq(posix_acl_t *acl, mode_t *mode_p)
+{
+	posix_acl_entry_t *pa, *pe;
+	posix_acl_entry_t *group_obj = NULL, *mask_obj = NULL;
+	mode_t mode = *mode_p;
+	int not_equiv = 0;
+
+	/* assert(atomic_read(acl->a_refcount) == 1); */
+
+	FOREACH_ACL_ENTRY(pa, acl, pe) {
+                switch(pa->e_tag) {
+                        case ACL_USER_OBJ:
+				pa->e_perm &= (mode >> 6) | ~S_IRWXO;
+				mode &= (pa->e_perm << 6) | ~S_IRWXU;
+				break;
+
+			case ACL_USER:
+			case ACL_GROUP:
+				not_equiv = 1;
+				break;
+
+                        case ACL_GROUP_OBJ:
+				group_obj = pa;
+                                break;
+
+                        case ACL_OTHER:
+				pa->e_perm &= mode | ~S_IRWXO;
+				mode &= pa->e_perm | ~S_IRWXO;
+                                break;
+
+                        case ACL_MASK:
+				mask_obj = pa;
+				not_equiv = 1;
+                                break;
+
+			default:
+				return -EIO;
+                }
+        }
+
+	if (mask_obj) {
+		mask_obj->e_perm &= (mode >> 3) | ~S_IRWXO;
+		mode &= (mask_obj->e_perm << 3) | ~S_IRWXG;
+	} else {
+		if (!group_obj)
+			return -EIO;
+		group_obj->e_perm &= (mode >> 3) | ~S_IRWXO;
+		mode &= (group_obj->e_perm << 3) | ~S_IRWXG;
+	}
+
+	*mode_p = (*mode_p & ~S_IRWXUGO) | mode;
+        return not_equiv;
+}
+
+/*
+ * Modify the ACL for the chmod syscall.
+ */
+int
+posix_acl_chmod_masq(posix_acl_t *acl, mode_t mode)
+{
+	posix_acl_entry_t *group_obj = NULL, *mask_obj = NULL;
+	posix_acl_entry_t *pa, *pe;
+
+	/* assert(atomic_read(acl->a_refcount) == 1); */
+
+	FOREACH_ACL_ENTRY(pa, acl, pe) {
+		switch(pa->e_tag) {
+			case ACL_USER_OBJ:
+				pa->e_perm = (mode & S_IRWXU) >> 6;
+				break;
+
+			case ACL_USER:
+			case ACL_GROUP:
+				break;
+
+			case ACL_GROUP_OBJ:
+				group_obj = pa;
+				break;
+
+			case ACL_MASK:
+				mask_obj = pa;
+				break;
+
+			case ACL_OTHER:
+				pa->e_perm = (mode & S_IRWXO);
+				break;
+
+			default:
+				return -EIO;
+		}
+	}
+
+	if (mask_obj) {
+		mask_obj->e_perm = (mode & S_IRWXG) >> 3;
+	} else {
+		if (!group_obj)
+			return -EIO;
+		group_obj->e_perm = (mode & S_IRWXG) >> 3;
+	}
+
+	return 0;
+}
+
+/*
+ * Adjust the mode parameter so that NFSv2 grants nobody permissions
+ * that may not be granted by the ACL. This is necessary because NFSv2
+ * may compute access permissions on the client side, and may serve cached
+ * data whenever it assumes access would be granted.  Since ACLs may also
+ * be used to deny access to specific users, the minimal permissions
+ * for secure operation over NFSv2 are very restrictive. Permissions
+ * granted to users via Access Control Lists will not be effective over
+ * NFSv2.
+ *
+ * Privilege escalation can only happen for read operations, as writes are
+ * always carried out on the NFS server, where the proper access checks are
+ * implemented.
+ */
+int
+posix_acl_masq_nfs_mode(posix_acl_t *acl, mode_t *mode_p)
+{
+	posix_acl_entry_t *pa, *pe; int min_perm = S_IRWXO;
+
+	FOREACH_ACL_ENTRY(pa, acl, pe) {
+                switch(pa->e_tag) {
+			case ACL_USER_OBJ:
+				break;
+
+			case ACL_USER:
+			case ACL_GROUP_OBJ:
+			case ACL_GROUP:
+			case ACL_MASK:
+			case ACL_OTHER:
+				min_perm &= pa->e_perm;
+				break;
+
+			default:
+				return -EIO;
+		}
+	}
+	*mode_p = (*mode_p & ~(S_IRWXG|S_IRWXO)) | (min_perm << 3) | min_perm;
+
+	return 0;
+}
+
+/*
+ * Get the POSIX ACL of an inode.
+ */
+posix_acl_t *
+get_posix_acl(struct inode *inode, int type)
+{
+	posix_acl_t *acl;
+
+	if (!inode->i_op || !inode->i_op->get_posix_acl)
+		return ERR_PTR(-ENOTSUP);
+
+	down(&inode->i_sem);
+	lock_kernel();  /* goes away in 2.5.x */
+	acl = inode->i_op->get_posix_acl(inode, type);
+	unlock_kernel();  /* goes away in 2.5.x */
+	up(&inode->i_sem);
+
+	return acl;
+}
+
+/*
+ * Set the POSIX ACL of an inode.
+ */
+int
+set_posix_acl(struct inode *inode, int type, posix_acl_t *acl)
+{
+	int error;
+
+	if (!inode->i_op || !inode->i_op->set_posix_acl)
+		return -ENOTSUP;
+
+	down(&inode->i_sem);
+	lock_kernel();  /* goes away in 2.5.x */
+	error = inode->i_op->set_posix_acl(inode, type, acl);
+	unlock_kernel();  /* goes away in 2.5.x */
+	up(&inode->i_sem);
+
+	return error;
+}
diff -Nur --exclude-from=linux.excl linux-2.5.30/include/linux/errno.h 
linux-2.5.30.patch/include/linux/errno.h
--- linux-2.5.30/include/linux/errno.h	Sun Aug  4 14:59:40 2002
+++ linux-2.5.30.patch/include/linux/errno.h	Sun Aug  4 14:59:26 2002
@@ -11,6 +11,8 @@
 #define ERESTARTNOHAND	514	/* restart if no handler.. */
 #define ENOIOCTLCMD	515	/* No ioctl command */
 
+#define ENOTSUP		-1	/* UNRESOLVED ISSUE -- see other proposal */
+
 /* Defined for the NFSv3 protocol */
 #define EBADHANDLE	521	/* Illegal NFS file handle */
 #define ENOTSYNC	522	/* Update synchronization mismatch */
diff -Nur --exclude-from=linux.excl linux-2.5.30/include/linux/fs.h 
linux-2.5.30.patch/include/linux/fs.h
--- linux-2.5.30/include/linux/fs.h	Thu Aug  1 23:16:15 2002
+++ linux-2.5.30.patch/include/linux/fs.h	Sun Aug  4 13:29:31 2002
@@ -23,6 +23,7 @@
 #include <linux/string.h>
 #include <linux/radix-tree.h>
 #include <linux/bitops.h>
+#include <linux/posix_acl.h>
 
 #include <asm/atomic.h>
 
@@ -787,6 +788,8 @@
 	ssize_t (*getxattr) (struct dentry *, const char *, void *, size_t);
 	ssize_t (*listxattr) (struct dentry *, char *, size_t);
 	int (*removexattr) (struct dentry *, const char *);
+	posix_acl_t *(*get_posix_acl) (struct inode *, int);
+	int (*set_posix_acl) (struct inode *, int, posix_acl_t *);
 };
 
 struct seq_file;
diff -Nur --exclude-from=linux.excl linux-2.5.30/include/linux/posix_acl.h 
linux-2.5.30.patch/include/linux/posix_acl.h
--- linux-2.5.30/include/linux/posix_acl.h	Thu Jan  1 01:00:00 1970
+++ linux-2.5.30.patch/include/linux/posix_acl.h	Sun Aug  4 13:24:12 2002
@@ -0,0 +1,69 @@
+/*
+  File: linux/posix_acl.h
+
+  (C) 2000 Andreas Gruenbacher, <a.gruenbacher@computer.org>
+*/
+
+
+#ifndef __LINUX_POSIX_ACL_H
+#define __LINUX_POSIX_ACL_H
+
+
+#define ACL_UNDEFINED_ID	(-1)
+
+/* a_type field in acl_user_posix_entry_t */
+#define ACL_TYPE_ACCESS		(0x8000)
+#define ACL_TYPE_DEFAULT	(0x4000)
+
+/* e_tag entry in posix_acl_entry_t */
+#define ACL_USER_OBJ		(0x01)
+#define ACL_USER		(0x02)
+#define ACL_GROUP_OBJ		(0x04)
+#define ACL_GROUP		(0x08)
+#define ACL_MASK		(0x10)
+#define ACL_OTHER		(0x20)
+
+/* permissions in the e_perm field */
+#define ACL_READ		(0x04)
+#define ACL_WRITE		(0x02)
+#define ACL_EXECUTE		(0x01)
+//#define ACL_ADD		(0x08)
+//#define ACL_DELETE		(0x10)
+
+#ifdef __KERNEL__
+
+typedef struct {
+	short			e_tag;
+	unsigned short		e_perm;
+	unsigned int		e_id;
+} posix_acl_entry_t;
+
+typedef struct {
+	atomic_t		a_refcount;
+	unsigned int		a_count;
+	posix_acl_entry_t	a_entries[0];
+} posix_acl_t;
+
+#define FOREACH_ACL_ENTRY(pa, acl, pe) \
+	for(pa=(acl)->a_entries, pe=pa+(acl)->a_count; pa<pe; pa++)
+
+/* pxacl.c */
+
+extern posix_acl_t *posix_acl_alloc(int);
+extern posix_acl_t *posix_acl_dup(posix_acl_t *);
+extern posix_acl_t *posix_acl_clone(const posix_acl_t *);
+extern void posix_acl_release(posix_acl_t *);
+extern int posix_acl_valid(const posix_acl_t *);
+extern int posix_acl_permission(struct inode *, const posix_acl_t *, int);
+extern posix_acl_t *posix_acl_from_mode(mode_t);
+extern int posix_acl_equiv_mode(const posix_acl_t *, mode_t *);
+extern int posix_acl_create_masq(posix_acl_t *, mode_t *);
+extern int posix_acl_chmod_masq(posix_acl_t *, mode_t);
+extern int posix_acl_masq_nfs_mode(posix_acl_t *, mode_t *);
+
+extern posix_acl_t *get_posix_acl(struct inode *, int);
+extern int set_posix_acl(struct inode *, int, posix_acl_t *);
+#endif  /* __KERNEL__ */
+
+
+#endif  /* __LINUX_POSIX_ACL_H */

Re: [RFC] POSIX ACL kernel infrastructure

[Christoph Hellwig]

On Sun, Aug 04, 2002 at 04:14:47PM +0200, Andreas Gruenbacher wrote:
> +MODULE_AUTHOR("Andreas Gruenbacher <a.gruenbacher@computer.org>");
> +MODULE_DESCRIPTION("Generic Posix Access Control List (ACL) Manipulation");
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0)
> +MODULE_LICENSE("GPL");
> +#endif

MODULE_LICENSE was new in 2.4.9 or 2.4.10, but certainly not present in
2.4.1..  I don't think kernel version checks in core code are a good idea
though.  especially if you aim for inclusion.

> +posix_acl_t *
> +posix_acl_alloc(int count)
> +{
> +	const size_t size = sizeof(posix_acl_t) +
> +	                    count * sizeof(posix_acl_entry_t);
> +	posix_acl_t *acl = kmalloc(size, GFP_KERNEL);
> +	if (acl) {
> +		atomic_set(&acl->a_refcount, 1);
> +		acl->a_count = count;
> +	}
> +	return acl;

are you sure this is never called from filsystem transactions?
passing a gfp flag down seems like a good idea to me.

> +/*
> + * Duplicate an ACL handle.
> + */
> +posix_acl_t *
> +posix_acl_dup(posix_acl_t *acl)
> +{
> +	if (acl)
> +		atomic_inc(&acl->a_refcount);
> +	return acl;
> +}

Make this an inline in a header?  can acl really be NULL?

> +/*
> + * Get the POSIX ACL of an inode.
> + */
> +posix_acl_t *
> +get_posix_acl(struct inode *inode, int type)
> +{
> +	posix_acl_t *acl;
> +
> +	if (!inode->i_op || !inode->i_op->get_posix_acl)
> +		return ERR_PTR(-ENOTSUP);

inode->i_op is never NULL.

> +	down(&inode->i_sem);
> +	lock_kernel();  /* goes away in 2.5.x */

this patch _is_ for 2.5, isn't it?

> linux-2.5.30.patch/include/linux/fs.h
> --- linux-2.5.30/include/linux/fs.h	Thu Aug  1 23:16:15 2002
> +++ linux-2.5.30.patch/include/linux/fs.h	Sun Aug  4 13:29:31 2002
> @@ -23,6 +23,7 @@
>  #include <linux/string.h>
>  #include <linux/radix-tree.h>
>  #include <linux/bitops.h>
> +#include <linux/posix_acl.h>
>  
>  #include <asm/atomic.h>
>  
> @@ -787,6 +788,8 @@
>  	ssize_t (*getxattr) (struct dentry *, const char *, void *, size_t);
>  	ssize_t (*listxattr) (struct dentry *, char *, size_t);
>  	int (*removexattr) (struct dentry *, const char *);
> +	posix_acl_t *(*get_posix_acl) (struct inode *, int);
> +	int (*set_posix_acl) (struct inode *, int, posix_acl_t *);

If you had followed Documentation/CodingSyle and use struct osix_acl
instead of posix_acl_t we wouldn't have to bloat fs.h with yet another
indirect header..

Also what exactly are get_posix_acl/set_posix_acl for?  We have wrappers
for them in fs/posix_acl.c, but even in your 2.4 patch only get_posix_acl
is ever used.  Shouldn't we always set/get posix ACLs through the xattr
inode operations?
 
> +#ifdef __KERNEL__

why the __KERNEL__?  

> +/* pxacl.c */

Shouldn't this be posix_acl.c?

Re: [RFC] POSIX ACL kernel infrastructure

[Andreas Gruenbacher]

On Sunday 04 August 2002 16:33, Christoph Hellwig wrote:
> On Sun, Aug 04, 2002 at 04:14:47PM +0200, Andreas Gruenbacher wrote:
> > +MODULE_AUTHOR("Andreas Gruenbacher <a.gruenbacher@computer.org>");
> > +MODULE_DESCRIPTION("Generic Posix Access Control List (ACL)
> > Manipulation"); +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0)
> > +MODULE_LICENSE("GPL");
> > +#endif
>
> MODULE_LICENSE was new in 2.4.9 or 2.4.10, but certainly not present in
> 2.4.1..  I don't think kernel version checks in core code are a good idea
> though.  especially if you aim for inclusion.

It was 2.4.10; I've canged that. I wanted to use identical code from 2.2 to 
2.4; the non-2.5 parts can be stripped off.

> > +posix_acl_t *
> > +posix_acl_alloc(int count)
> > +{
> > +	const size_t size = sizeof(posix_acl_t) +
> > +	                    count * sizeof(posix_acl_entry_t);
> > +	posix_acl_t *acl = kmalloc(size, GFP_KERNEL);
> > +	if (acl) {
> > +		atomic_set(&acl->a_refcount, 1);
> > +		acl->a_count = count;
> > +	}
> > +	return acl;
>
> are you sure this is never called from filesystem transactions?
> passing a gfp flag down seems like a good idea to me.

Probably, yes.

> > +/*
> > + * Duplicate an ACL handle.
> > + */
> > +posix_acl_t *
> > +posix_acl_dup(posix_acl_t *acl)
> > +{
> > +	if (acl)
> > +		atomic_inc(&acl->a_refcount);
> > +	return acl;
> > +}
>
> Make this an inline in a header?  can acl really be NULL?

Yes it can, meaning `there are only the file permission bits'.

> > +/*
> > + * Get the POSIX ACL of an inode.
> > + */
> > +posix_acl_t *
> > +get_posix_acl(struct inode *inode, int type)
> > +{
> > +	posix_acl_t *acl;
> > +
> > +	if (!inode->i_op || !inode->i_op->get_posix_acl)
> > +		return ERR_PTR(-ENOTSUP);
>
> inode->i_op is never NULL.

Changed.

> > +	down(&inode->i_sem);
> > +	lock_kernel();  /* goes away in 2.5.x */
>
> this patch _is_ for 2.5, isn't it?

Removed.

> > linux-2.5.30.patch/include/linux/fs.h
> > --- linux-2.5.30/include/linux/fs.h	Thu Aug  1 23:16:15 2002
> > +++ linux-2.5.30.patch/include/linux/fs.h	Sun Aug  4 13:29:31 2002
> > @@ -23,6 +23,7 @@
> >  #include <linux/string.h>
> >  #include <linux/radix-tree.h>
> >  #include <linux/bitops.h>
> > +#include <linux/posix_acl.h>
> >
> >  #include <asm/atomic.h>
> >
> > @@ -787,6 +788,8 @@
> >  	ssize_t (*getxattr) (struct dentry *, const char *, void *, size_t);
> >  	ssize_t (*listxattr) (struct dentry *, char *, size_t);
> >  	int (*removexattr) (struct dentry *, const char *);
> > +	posix_acl_t *(*get_posix_acl) (struct inode *, int);
> > +	int (*set_posix_acl) (struct inode *, int, posix_acl_t *);
>
> If you had followed Documentation/CodingSyle and use struct posix_acl
> instead of posix_acl_t we wouldn't have to bloat fs.h with yet another
> indirect header..

Using `struct posix_acl' everywhere is so much more messy. Is an exception 
justified here? The core kernel code has typedefs all over the place.

> Also what exactly are get_posix_acl/set_posix_acl for?  We have wrappers
> for them in fs/posix_acl.c, but even in your 2.4 patch only get_posix_acl
> is ever used.  Shouldn't we always set/get posix ACLs through the xattr
> inode operations?

>From user space, yes. The get_posix_acl operation is currently used in nfsd; 
going via the xattr operations would be too expensive here. The set_posix_acl 
operation is indeed not used so far; I think it makes sense to add it for 
completeness' sake.

>
> > +#ifdef __KERNEL__
>
> why the __KERNEL__?

Not needed anymore.

> > +/* pxacl.c */
>
> Shouldn't this be posix_acl.c?

Yes.

I have put up an updated version at 
<http://acl.bestbits.at/pre/2.5/linux-2.5.30-posix-acl-1.diff>.

Regards,
Andreas.

------------------------------------------------------------------
 Andreas Gruenbacher                                SuSE Linux AG
 mailto:agruen@suse.de                     Deutschherrnstr. 15-19
 http://www.suse.de/                   D-90429 Nuernberg, Germany

Re: [RFC] POSIX ACL kernel infrastructure

[Christoph Hellwig]

On Mon, Aug 05, 2002 at 02:11:33PM +0200, Andreas Gruenbacher wrote:
> It was 2.4.10; I've canged that. I wanted to use identical code from 2.2 to 
> 2.4; the non-2.5 parts can be stripped off.

Then strip them off :)  especially as you added even more ugly checks around
the lock_kernel/unlock_kernel invocations..

> Using `struct posix_acl' everywhere is so much more messy.

Why?

> Is an exception 
> justified here? The core kernel code has typedefs all over the place.

Ask Linus..  You should at least provide an alternate struct posix_acl
definition so fs.h doesn't have to pull in posix_acl.h but can use a
struct forward declaration.

> >From user space, yes. The get_posix_acl operation is currently used in nfsd; 
> going via the xattr operations would be too expensive here. The set_posix_acl 
> operation is indeed not used so far; I think it makes sense to add it for 
> completeness' sake.

I'm not fully convienced.  At least add the locking rules to Documentation/
filesystems/Locking.  Documentation/filesystems/porting should also get a
an entry about the ACL support.

Re: [RFC] POSIX ACL kernel infrastructure

[Nathan Scott]

hi Andreas,

On Mon, Aug 05, 2002 at 01:28:32PM +0100, Christoph Hellwig wrote:
> On Mon, Aug 05, 2002 at 02:11:33PM +0200, Andreas Gruenbacher wrote:
> 
> > >From user space, yes. The get_posix_acl operation is currently used in nfsd; 
> > going via the xattr operations would be too expensive here. The set_posix_acl 
> > operation is indeed not used so far; I think it makes sense to add it for 
> > completeness' sake.
> 
> I'm not fully convienced.

Nor am I (I don't buy either the too expensive argument or the
need for unused ops).  It also begins to add unnecessary stuff
to the VFS inode ops - when capabilities, MAC labels, and misc
other attributes come along, we don't want to be adding in ops
for each of them too (esp. considering many filesystems do not
support extended attributes at all).  IMO getxattr() should be
all we ever need here.

MS_POSIXACL from your earlier mail is a Good Thing, I think -
I'll switch XFS over to use that when I get some time (instead
of the inode flag we are currently using).

cheers.

-- 
Nathan

Re: [RFC] POSIX ACL kernel infrastructure

[Andreas Gruenbacher]

On Friday 09 August 2002 04:02, Nathan Scott wrote:
> hi Andreas,
>
> On Mon, Aug 05, 2002 at 01:28:32PM +0100, Christoph Hellwig wrote:
> > On Mon, Aug 05, 2002 at 02:11:33PM +0200, Andreas Gruenbacher wrote:
> > > >From user space, yes. The get_posix_acl operation is currently used in
> > > > nfsd;
> > >
> > > going via the xattr operations would be too expensive here. The
> > > set_posix_acl operation is indeed not used so far; I think it makes
> > > sense to add it for completeness' sake.
> >
> > I'm not fully convienced.
>
> Nor am I (I don't buy either the too expensive argument or the
> need for unused ops).

The kernel NFS daemon introduces a number of interesting problems in the 
kernel that no other component requires; the ACL permission masking hack is 
one of them. The hack affects NFSv2 and NFSv3 

It is really important that encode_fattr and encode_fattr3, the functions that 
include the hack, are efficient; they are used in all the essential NFS RPCs.

The two implementation choices are:

	* Use the get_posix_acl inode operation

	On Ext2, Ext3, ReiserFS the inode operation passes a handle to the
	ACL to the caller (no copying). The handle is in the cache	after the
	first access, the cache is very effective. XFS currently seems not to
	implement the *_posix_acl inode operations. That's fine; just the
	NFS hack won't be able to mask permissions.

	* Go via the xattr ACL format

	The xattr interface uses pass-by-value by design. So this requires
	allocating a buffer, conversion from the file system into the xattr
	format, parsing the xattr format. The xattr format is in 
 (with possible reverse endianness conversion).

The get_posix_acl approach has the added benefit that other parts of the 
kernel interested in manipulating ACLs can do so easily and efficiently 
(though I don't see which ones that could be right now). If set_posix_acl 
really _must_ be removed I won't bang my head against a wall.

Is this sufficient?

> It also begins to add unnecessary stuff
> to the VFS inode ops - when capabilities, MAC labels, and misc
> other attributes come along, we don't want to be adding in ops
> for each of them too (esp. considering many filesystems do not
> support extended attributes at all). IMO getxattr() should be
> all we ever need here.

Those file systems not supporting some of the options are not affected. We 
only need to add stuff to the VFS for things that other parts of the kernel 
need to access independently of the real filesystem, in those cases where the 
accesses need to be faster than the xattr interface can be.

> MS_POSIXACL from your earlier mail is a Good Thing, I think -
> I'll switch XFS over to use that when I get some time (instead
> of the inode flag we are currently using).

Fine, I will also use that, instead of MS_NOUMASK. MS_POSIXACL is nice to have 
in the nfsd hack as well. Having it is not extremely clean design, and I will 
hate myself for giving in the day another mechanism requires the umask to be 
skipped which is not POSIX ACLs, though.

Cheers,
Andreas.

------------------------------------------------------------------
 Andreas Gruenbacher                                SuSE Linux AG
 mailto:agruen@suse.de                     Deutschherrnstr. 15-19
 http://www.suse.de/                   D-90429 Nuernberg, Germany

Re: [RFC] POSIX ACL kernel infrastructure

[Christoph Hellwig]

On Fri, Aug 09, 2002 at 12:53:55PM +0200, Andreas Gruenbacher wrote:
> The kernel NFS daemon introduces a number of interesting problems in the 
> kernel that no other component requires; the ACL permission masking hack is 
> one of them. The hack affects NFSv2 and NFSv3 
> 
> It is really important that encode_fattr and encode_fattr3, the functions that 
> include the hack, are efficient; they are used in all the essential NFS RPCs.

IF it's so essential why are you doing another IOP call then instead of adding
inkernel A retrieving to ->getattr and struct kstat?  encode_fattr* has to
call it anyway and that's the logical place for ACLs..

Re: [RFC] POSIX ACL kernel infrastructure

[Andreas Gruenbacher]

On Friday 09 August 2002 13:15, Christoph Hellwig wrote:
> On Fri, Aug 09, 2002 at 12:53:55PM +0200, Andreas Gruenbacher wrote:
> > The kernel NFS daemon introduces a number of interesting problems in the
> > kernel that no other component requires; the ACL permission masking hack
> > is one of them. The hack affects NFSv2 and NFSv3
> >
> > It is really important that encode_fattr and encode_fattr3, the functions
> > that include the hack, are efficient; they are used in all the essential
> > NFS RPCs.
>
> IF it's so essential why are you doing another IOP call then instead of
> adding inkernel A retrieving to ->getattr and struct kstat?  encode_fattr*
> has to call it anyway and that's the logical place for ACLs..

I read this as `Why are you not returning the permissions nfsd should see in 
struct, which the inode->getattr IOP fills?'. Please correct me if this is 
not what you wanted to say.

The usual callers of getattr should see the unmasked permissions; it's only 
nfsd that needs this special treatment, and only on filesystems that have 
ACLs enabled. The getattr IOP is used by others as well.

If you had adding an extra field to struct kstat in mind, this just seems too 
intrusive to me for solving the nfsd problem. The problem will go away in the 
future when all NFSv3 clients are using the ACCESS RPC, anyway. It's just a 
hack right now.

-Andreas.

------------------------------------------------------------------
 Andreas Gruenbacher                                SuSE Linux AG
 mailto:agruen@suse.de                     Deutschherrnstr. 15-19
 http://www.suse.de/                   D-90429 Nuernberg, Germany

Re: [RFC] POSIX ACL kernel infrastructure

[Christoph Hellwig]

On Fri, Aug 09, 2002 at 02:22:49PM +0200, Andreas Gruenbacher wrote:
> I read this as `Why are you not returning the permissions nfsd should see in 
> struct, which the inode->getattr IOP fills?'. Please correct me if this is 
> not what you wanted to say.

I meant adding an acl field to struct kstat that is supposed to be filled
by ->getattr.

> The usual callers of getattr should see the unmasked permissions; it's only 
> nfsd that needs this special treatment, and only on filesystems that have 
> ACLs enabled. The getattr IOP is used by others as well.

So why the is a new IOP better in that respect? 

> If you had adding an extra field to struct kstat in mind, this just seems too 
> intrusive to me for solving the nfsd problem. The problem will go away in the 
> future when all NFSv3 clients are using the ACCESS RPC, anyway. It's just a 
> hack right now.

So you want to add two inode operations for a hack that is only needed to
support old NFS clients?  Do you have any profiling data that shows major
overhead when going through the xattr layer?

Re: [RFC] POSIX ACL kernel infrastructure

[Andreas Gruenbacher]

On Friday 09 August 2002 14:32, Christoph Hellwig wrote:
> On Fri, Aug 09, 2002 at 02:22:49PM +0200, Andreas Gruenbacher wrote:
> > I read this as `Why are you not returning the permissions nfsd should see
> > in struct, which the inode->getattr IOP fills?'. Please correct me if
> > this is not what you wanted to say.
>
> I meant adding an acl field to struct kstat that is supposed to be filled
> by ->getattr.

I see. That's problematic on most filesystems, because retrieving the ACL from 
disk is pretty expensive, and often is not needed. For things like stat() it 
is not needed, for example.

> > The usual callers of getattr should see the unmasked permissions; it's
> > only nfsd that needs this special treatment, and only on filesystems that
> > have ACLs enabled. The getattr IOP is used by others as well.
>
> So why is the a new IOP better in that respect?

Only nfsd needs to pay the runtime cost.

> > If you had adding an extra field to struct kstat in mind, this just seems
> > too intrusive to me for solving the nfsd problem. The problem will go
> > away in the future when all NFSv3 clients are using the ACCESS RPC,
> > anyway. It's just a hack right now.
>
> So you want to add two inode operations for a hack that is only needed to
> support old NFS clients?

It will be needed for NFSv2 and NFSv3, until all NFSv3 clients support the 
ACCESS RPC properly. The ACCESS RPC patch for the client side in the current 
kernel still has some performance issues. The hack won't go away anytime 
soon, but a lot fewer people will be affected by the underlying problem in a 
year or so.

I also think the inode operations are a good thing to keep independent of the 
nfsd issue.

> Do you have any profiling data that shows major overhead when going through 
the xattr layer?

No, this would be tricky to measure. The interface requires copying around 
ACLs, and endianness conversions on big endian machines; it's simply 
inappropriate for that task.

--Andreas.

------------------------------------------------------------------
 Andreas Gruenbacher                                SuSE Linux AG
 mailto:agruen@suse.de                     Deutschherrnstr. 15-19
 http://www.suse.de/                   D-90429 Nuernberg, Germany