Skip to contentSkip to conversation

Add POSIX Access Control Lists to ext2/3

PATCH In review [email protected] 4 participants 7 messages Oct 15, 2002 – Oct 17, 2002 VFS

lore
Share & tools
lore
#0 lore raw

PATCH 1/5 Add POSIX Access Control Lists to ext2/3

The following five patches add Posix ACL support to the 2.5 kernel. The first three patches are generic, and will be useful to other filesystems who also have ACL support on deck (in particular, the JFS and XFS folks). The last two patches add ACL support to the ext2 and ext3 filesystems. These patches are safe, in that the code paths remain largely untouched if they the compile-time option is not enabled, and even if ACL support is compiled in, the filesystem must be explicitly mounted a mount-option to enable ACL support.

These pataches require that the extended attribute patches be applied first.

- Ted

This patch (as well as the following two) implements core ACL support. It implements a common interface which is used to provide ACL support for the ext2/3, XFS and JFS filesystems, and was developed in cooperation with developers from those teams. User mode tools which support this interface may be found at http://acl.bestbits.at

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
#

# fs/Config.help            |   13 +
# fs/Config.in              |    2 
# fs/Makefile               |    1 
# fs/posix_acl.c            |  446 ++++++++++++++++++++++++++++++++++++++++++++++
# include/linux/fs.h        |    5 
# include/linux/posix_acl.h |   87 ++++++++

# 6 files changed, 554 insertions(+)
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/10/15 [email protected] 1.854
# Port of 0.8.50 acl patch to 2.5
#
# This patch (as well as the following two) implements core ACL support. It implements

# a common interface which is used to provide ACL support for the ext2/3, XFS and JFS
# filesystems, and was developed in cooperation with developers from those teams.
# User mode tools which support this interface may be found at http://acl.bestbits.at
# --------------------------------------------
#
diff -Nru a/fs/Config.help b/fs/Config.help
--- a/fs/Config.help	Tue Oct 15 16:58:57 2002
+++ b/fs/Config.help	Tue Oct 15 16:58:57 2002
@@ -1,3 +1,16 @@
+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 the Posix ACLs for
+  Linux website <http://acl.bestbits.at/>.
+
+  If you plan to use Access Control Lists, you may also need the
+  getfacl and setfacl utilities, along with some additional patches
+  from the website.
+
+  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 -Nru a/fs/Config.in b/fs/Config.in
--- a/fs/Config.in	Tue Oct 15 16:58:57 2002
+++ b/fs/Config.in	Tue Oct 15 16:58:57 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

fs/Makefile +1 -0
diff -Nru a/fs/Makefile b/fs/Makefile
@@ -31,6 +31,7 @@
 obj-$(CONFIG_BINFMT_ELF)	+= binfmt_elf.o
 
 obj-$(CONFIG_FS_MBCACHE)	+= mbcache.o
+obj-$(CONFIG_FS_POSIX_ACL)	+= posix_acl.o
 
 obj-$(CONFIG_QUOTA)		+= dquot.o
 obj-$(CONFIG_QFMT_V1)		+= quota_v1.o
fs/posix_acl.c new file +446 -0

446 lines not on this page · raw message

include/linux/fs.h +5 -0
diff -Nru a/include/linux/fs.h b/include/linux/fs.h
@@ -765,6 +765,9 @@
 	unsigned long (*get_unmapped_area)(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
 };
 
+/* posix_acl.h */
+struct posix_acl;
+
 struct inode_operations {
 	int (*create) (struct inode *,struct dentry *,int);
 	struct dentry * (*lookup) (struct inode *,struct dentry *);
@@ -786,6 +789,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 *);
+	struct posix_acl *(*get_posix_acl) (struct inode *, int);
+	int (*set_posix_acl) (struct inode *, int, struct posix_acl *);
 };
 
 struct seq_file;
include/linux/posix_acl.h new file +87 -0

87 lines not on this page · raw message

#1 lore raw
Quoted 10 lines · On Tue, Oct 15, 2002 at 06:20:55PM -0400, [email protected] wrote:

The following five patches add Posix ACL support to the 2.5 kernel. The first three patches are generic, and will be useful to other filesystems who also have ACL support on deck (in particular, the JFS and XFS folks). The last two patches add ACL support to the ext2 and ext3 filesystems. These patches are safe, in that the code paths remain largely untouched if they the compile-time option is not enabled, and even if ACL support is compiled in, the filesystem must be explicitly mounted a mount-option to enable ACL support.

Quoted 2 lines

These pataches require that the extended attribute patches be applied first.

Linux, please domn't apply these ACL patches until the inode method mess is sorted out.

Ted, please either go _always_ through the {get,set}_posix_acl methods or never. Currently XFS doesn't know and doesn't want to know about the so called "egenric ACL representation" used by ext2/ext3. With theses methods we'd have to add it to XFS which is fine for me as long as it the representation generally used for working with ACLs. That would mean we'd have to add new syscall or at least VFS-level hooks to the xattr code.

Or just declare the xattr representation the default one, remove the get_posix_acl/set_posix_acl code from this code and fix up the nfsd patch (that doesn't seem to be submitted yet) up to us the xattr representation.

#2 lore raw
Quoted 6 lines · Christoph Hellwig

Ted, please either go _always_ through the {get,set}_posix_acl methods or never. Currently XFS doesn't know and doesn't want to know about the so called "egenric ACL representation" used by ext2/ext3. With theses methods we'd have to add it to XFS which is fine for me as long as it the representation generally used for working with ACLs. That would mean we'd have to add new syscall or at least VFS-level hooks to the xattr code.

Fine. I'll just yank the {get,set}_posix_acl methods for now. The inode methods were only needed for the NFS code (see Andreas' comments about the xattr interfaces being problematical for VFS support).

However, the reality is that at this point, we probably won't have time to get support in for the NFS server ACL before feature freeze, and changing the interface to ACL's (never mind the headaches of trying to agree to a new syscall interface at this late date), given the deployed userspace tools, just doesn't seem to be realistic.

So if you or Andreas has time to work on NFS server support for ACL's in the next two days, great, but I'm going to just pass on this for now. Later on, we can figure out what changes are necessary to all of the various filesystems so that nfsd can get what information it needs.

- Ted

#3 lore raw
Quoted 13 lines · Theodore Ts'o
Quoted 7 lines · Christoph Hellwig

Ted, please either go _always_ through the {get,set}_posix_acl methods or never. Currently XFS doesn't know and doesn't want to know about the so called "egenric ACL representation" used by ext2/ext3. With theses methods we'd have to add it to XFS which is fine for me as long as it the representation generally used for working with ACLs. That would mean we'd have to add new syscall or at least VFS-level hooks to the xattr code.

Fine. I'll just yank the {get,set}_posix_acl methods for now. The inode methods were only needed for the NFS code (see Andreas' comments about the xattr interfaces being problematical for VFS support).

Quoted 5 lines

However, the reality is that at this point, we probably won't have time to get support in for the NFS server ACL before feature freeze, and changing the interface to ACL's (never mind the headaches of trying to agree to a new syscall interface at this late date), given the deployed userspace tools, just doesn't seem to be realistic.

The pain of not having the NFS ACL hack is only moderate; it only affects interoperability of older systems with a feature that wasn't there before, and even then the effects aren't dramatic. We could live without it for a while, but I'll see if I code that up in time too.

--Andreas.

#4 lore raw
In reply to Theodore Ts'o

Mornin' all,

Quoted 9 lines · Theodore Ts'o
Quoted 6 lines · Christoph Hellwig

Ted, please either go _always_ through the {get,set}_posix_acl methods or never. Currently XFS doesn't know and doesn't want to know about the so called "egenric ACL representation" used by ext2/ext3. With theses methods we'd have to add it to XFS which is fine for me as long as it the representation generally used for working with ACLs. That would mean we'd have to add new syscall or at least VFS-level hooks to the xattr code.

Fine. I'll just yank the {get,set}_posix_acl methods for now. The

Please remove them permanently, IMO they are broken by design.

They are an optimisization for the one special case (posix acls), and manage to pollute the VFS for that one special case - a much cleaner solution is to optimise the code sitting behind the xattr inode calls and preferably in ways that all of the ext2/3 EAs can benefit (users attrs, capabilities, MAC labels, etc), & not just ACLs.

Yes, this means nfsd has to do some more work to alloc some space and convert from syscall format to native before doing its thing with the attributes, but thats in the noise compared to going to disk and fetching the EA (which is what we really want to optimise here) and can also be wrapped in Andreas' lib code so that the nfsd changes are minimal.

Quoted 5 lines

However, the reality is that at this point, we probably won't have time to get support in for the NFS server ACL before feature freeze, and changing the interface to ACL's (never mind the headaches of trying to agree to a new syscall interface at this late date), given the deployed userspace tools, just doesn't seem to be realistic.

No additional syscalls are needed. This would also be short sighted
- we don't yet support any other system EAs (as I've said to Andreas on several occassions, capabilities look like a really good candidate since they also must be read in kernel space on exec and read/written from userspace), so any interface changes now based on a fairly weak optimisation for the single system attribute that the patches support today (posix acls) is premature at best.

Quoted 3 lines

now. Later on, we can figure out what changes are necessary to all of the various filesystems so that nfsd can get what information it needs.

No changes are needed to other filesystems, these new "generic posix acl" inode_operations just need to go away, permanently - nfsd can get everything it needs using the existing interfaces. Requests for some data showing any performance improvement at all with these VFS extensions haven't produced anything to date, let alone a compelling case for extending the VFS just for posix acls.

Just my AUS2c.

cheers.

Signature · 1 line
Nathan
#5 lore raw

Hello Nathan,

Quoted 12 lines · Nathan Scott

Mornin' all,

Quoted 9 lines · Theodore Ts'o
Quoted 7 lines · Christoph Hellwig

Ted, please either go _always_ through the {get,set}_posix_acl methods or never. Currently XFS doesn't know and doesn't want to know about the so called "egenric ACL representation" used by ext2/ext3. With theses methods we'd have to add it to XFS which is fine for me as long as it the representation generally used for working with ACLs. That would mean we'd have to add new syscall or at least VFS-level hooks to the xattr code.

Quoted 3 lines
Quoted 1 line

Fine. I'll just yank the {get,set}_posix_acl methods for now. The

Please remove them permanently, IMO they are broken by design.

That's done, despite that I disagree with your line of argument (see below).

Quoted 6 lines

They are an optimisization for the one special case (posix acls), and manage to pollute the VFS for that one special case - a much cleaner solution is to optimise the code sitting behind the xattr inode calls and preferably in ways that all of the ext2/3 EAs can benefit (users attrs, capabilities, MAC labels, etc), & not just ACLs.

Nathan, you are ignoring the design of the inode operations: They are on purpose passing extended attributes by value. This is good as a kernel/user space interface, but inappropriate for passing around references to kernel internal structures. There is no further optimizing to be done behind that interface.

In the current design no filesystem independent part of the kernel would benefit from a faster interface except that one NFS hack. Since that appears to be no serious performance bottleneck and the hack will eventually go away using ->getxattr() seems acceptable.

As soon as any filesystem independent part of the kernel needs an interface more efficient that pass-by-value we will again have exactly the same problem.

Quoted 6 lines

Yes, this means nfsd has to do some more work to alloc some space and convert from syscall format to native before doing its thing with the attributes, but thats in the noise compared to going to disk and fetching the EA (which is what we really want to optimise here) and can also be wrapped in Andreas' lib code so that the nfsd changes are minimal.

I'm not sure if you have looked at how the [gs]et_posix_acl() operations are implemented for ext2/ext3/reiserfs/xfs. They are caching the ACLs per inode instead of repeatedly copying around the xattr values; permission() calls therefore are very efficient. XFS is the only exception; it always goes through the xattr interface and does not cache ACLs. I think XFS should also use caching, at least for ACL_TYPE_ACCESS ACLs.

Going to disk and fetching EAs only causes disk accesses once; afterwards the block is cached.

Quoted 6 lines
Quoted 5 lines

However, the reality is that at this point, we probably won't have time to get support in for the NFS server ACL before feature freeze, and changing the interface to ACL's (never mind the headaches of trying to agree to a new syscall interface at this late date), given the deployed userspace tools, just doesn't seem to be realistic.

Quoted 7 lines

No additional syscalls are needed. This would also be short sighted
- we don't yet support any other system EAs (as I've said to Andreas on several occassions, capabilities look like a really good candidate since they also must be read in kernel space on exec and read/written from userspace), so any interface changes now based on a fairly weak optimisation for the single system attribute that the patches support today (posix acls) is premature at best.

File system capabilities are in a different league that ACLs. They only need to be read once for each exec(), so going through the xattr interface is no problem at all. (In comparison, all ACLs along the path to a file are accessed whenever that file is accessed.)

Cheers,
Andreas.

#6 lore raw
Quoted 1 line · Andreas Gruenbacher

Hello Nathan,

hi there,

Quoted 7 lines · Nathan Scott
Quoted 2 lines

They are an optimisization for the one special case (posix acls), and manage to pollute the VFS for that one special case ...

...
As soon as any filesystem independent part of the kernel needs an interface more efficient that pass-by-value we will again have exactly the same problem.

My point is simply that a proposal to extend the VFS in this way needs to be accompanied by a compelling argument showing the performance bump that its providing.

Quoted 2 lines

Going to disk and fetching EAs only causes disk accesses once; afterwards the block is cached.

Good - this is true for both XFS and ext2/3 then. So, we are talking about using ref counting vs. copying for any in-kernel users of attrs, and you're saying there is some significant overheads with copying and I'm saying show me what kind of overheads we're talking about, please.

cheers.

Signature · 1 line
Nathan