From: Andreas Gruenbacher Subject: Re: [PATCH] generic acl support for ->permission Date: Wed, 1 Sep 2004 21:20:52 +0200 Sender: linux-fsdevel-owner@vger.kernel.org Message-ID: <200409012120.53027.agruen@suse.de> References: <20040901152439.GA28950@lst.de> Mime-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_UEiNB04+bVQ3MGZ" Cc: linux-fsdevel@vger.kernel.org, shaggy@austin.ibm.com, jeffm@suse.com Return-path: Received: from cantor.suse.de ([195.135.220.2]:31193 "EHLO Cantor.suse.de") by vger.kernel.org with ESMTP id S269968AbUICWxw (ORCPT ); Fri, 3 Sep 2004 18:53:52 -0400 To: Christoph Hellwig In-Reply-To: <20040901152439.GA28950@lst.de> Content-Disposition: inline List-Id: linux-fsdevel.vger.kernel.org --Boundary-00=_UEiNB04+bVQ3MGZ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Hello Christoph, as far as I can see what you propose is very similar to the approach taken in the attached patch, with one more indirection. This looks nicer to me, at the cost of "polluting" iops instead of vfs_permission. JFS will still need Dave's patch, obviously. The attached patch only covers ext3; not sure if it even compiles. Cheers, -- Andreas Gruenbacher SUSE Labs, SUSE LINUX AG --Boundary-00=_UEiNB04+bVQ3MGZ Content-Type: text/x-diff; charset="iso-8859-1"; name="other.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="other.diff" Index: linux-2.6.9-rc1-bk8/fs/ext3/file.c =================================================================== --- linux-2.6.9-rc1-bk8.orig/fs/ext3/file.c +++ linux-2.6.9-rc1-bk8/fs/ext3/file.c @@ -136,6 +136,6 @@ struct inode_operations ext3_file_inode_ .getxattr = ext3_getxattr, .listxattr = ext3_listxattr, .removexattr = ext3_removexattr, - .permission = ext3_permission, + .check_posix_acl = ext3_check_posix_acl, }; Index: linux-2.6.9-rc1-bk8/fs/ext3/acl.h =================================================================== --- linux-2.6.9-rc1-bk8.orig/fs/ext3/acl.h +++ linux-2.6.9-rc1-bk8/fs/ext3/acl.h @@ -59,7 +59,7 @@ static inline int ext3_acl_count(size_t #define EXT3_ACL_NOT_CACHED ((void *)-1) /* acl.c */ -extern int ext3_permission (struct inode *, int, struct nameidata *); +extern int ext3_check_posix_acl (struct inode *, int); extern int ext3_acl_chmod (struct inode *); extern int ext3_init_acl (handle_t *, struct inode *, struct inode *); @@ -68,7 +68,7 @@ extern void exit_ext3_acl(void); #else /* CONFIG_EXT3_FS_POSIX_ACL */ #include -#define ext3_permission NULL +#define ext3_check_posix_acl NULL static inline int ext3_acl_chmod(struct inode *inode) Index: linux-2.6.9-rc1-bk8/fs/namei.c =================================================================== --- linux-2.6.9-rc1-bk8.orig/fs/namei.c +++ linux-2.6.9-rc1-bk8/fs/namei.c @@ -180,8 +180,19 @@ int vfs_permission(struct inode * inode, if (current->fsuid == inode->i_uid) mode >>= 6; - else if (in_group_p(inode->i_gid)) - mode >>= 3; + else { + if (IS_POSIXACL(inode) && (mode & S_IRWXG) && + inode->i_op->check_posix_acl) { + int error = inode->i_op->check_posix_acl(inode, mask); + if (error == -EACCES) + goto check_capabilities; + else if (error != -EAGAIN) + return error; + } + + if (in_group_p(inode->i_gid)) + mode >>= 3; + } /* * If the DACs are ok we don't need any capability check. @@ -189,6 +200,7 @@ int vfs_permission(struct inode * inode, if (((mode & mask & (MAY_READ|MAY_WRITE|MAY_EXEC)) == mask)) return 0; +check_capabilities: /* * Read/write DACs are always overridable. * Executable DACs are overridable if at least one exec bit is set. @@ -208,7 +220,7 @@ int vfs_permission(struct inode * inode, return -EACCES; } -int permission(struct inode * inode,int mask, struct nameidata *nd) +int permission(struct inode * inode, int mask, struct nameidata *nd) { int retval; int submask; Index: linux-2.6.9-rc1-bk8/include/linux/fs.h =================================================================== --- linux-2.6.9-rc1-bk8.orig/include/linux/fs.h +++ linux-2.6.9-rc1-bk8/include/linux/fs.h @@ -930,6 +930,7 @@ struct inode_operations { void (*put_link) (struct dentry *, struct nameidata *); void (*truncate) (struct inode *); int (*permission) (struct inode *, int, struct nameidata *); + int (*check_posix_acl) (struct inode *, int); int (*setattr) (struct dentry *, struct iattr *); int (*getattr) (struct vfsmount *mnt, struct dentry *, struct kstat *); int (*setxattr) (struct dentry *, const char *,const void *,size_t,int); Index: linux-2.6.9-rc1-bk8/fs/ext3/acl.c =================================================================== --- linux-2.6.9-rc1-bk8.orig/fs/ext3/acl.c +++ linux-2.6.9-rc1-bk8/fs/ext3/acl.c @@ -285,60 +285,17 @@ ext3_set_acl(handle_t *handle, struct in return error; } -/* - * Inode operation permission(). - * - * inode->i_sem: don't care - */ int -ext3_permission(struct inode *inode, int mask, struct nameidata *nd) +ext3_check_posix_acl(struct inode *inode, int mask) { - int mode = inode->i_mode; + struct posix_acl *acl = ext3_get_acl(inode, ACL_TYPE_ACCESS); + int error = -EAGAIN; - /* Nobody gets write access to a read-only fs */ - if ((mask & MAY_WRITE) && IS_RDONLY(inode) && - (S_ISREG(mode) || S_ISDIR(mode) || S_ISLNK(mode))) - return -EROFS; - /* Nobody gets write access to an immutable file */ - if ((mask & MAY_WRITE) && IS_IMMUTABLE(inode)) - return -EACCES; - if (current->fsuid == inode->i_uid) { - mode >>= 6; - } else if (test_opt(inode->i_sb, POSIX_ACL)) { - struct posix_acl *acl; - - /* The access ACL cannot grant access if the group class - permission bits don't contain all requested permissions. */ - if (((mode >> 3) & mask & S_IRWXO) != mask) - goto check_groups; - acl = ext3_get_acl(inode, ACL_TYPE_ACCESS); - if (acl) { - int error = posix_acl_permission(inode, acl, mask); - posix_acl_release(acl); - if (error == -EACCES) - goto check_capabilities; - return error; - } else - goto check_groups; - } else { -check_groups: - if (in_group_p(inode->i_gid)) - mode >>= 3; + if (acl) { + error = posix_acl_permission(inode, acl, mask); + posix_acl_release(acl); } - if ((mode & mask & S_IRWXO) == mask) - return 0; - -check_capabilities: - /* Allowed to override Discretionary Access Control? */ - if (!(mask & MAY_EXEC) || - (inode->i_mode & S_IXUGO) || S_ISDIR(inode->i_mode)) - if (capable(CAP_DAC_OVERRIDE)) - return 0; - /* Read and search granted if capable(CAP_DAC_READ_SEARCH) */ - if (capable(CAP_DAC_READ_SEARCH) && ((mask == MAY_READ) || - (S_ISDIR(inode->i_mode) && !(mask & MAY_WRITE)))) - return 0; - return -EACCES; + return error; } /* Index: linux-2.6.9-rc1-bk8/fs/ext3/namei.c =================================================================== --- linux-2.6.9-rc1-bk8.orig/fs/ext3/namei.c +++ linux-2.6.9-rc1-bk8/fs/ext3/namei.c @@ -2359,7 +2359,7 @@ struct inode_operations ext3_dir_inode_o .getxattr = ext3_getxattr, .listxattr = ext3_listxattr, .removexattr = ext3_removexattr, - .permission = ext3_permission, + .check_posix_acl = ext3_check_posix_acl, }; struct inode_operations ext3_special_inode_operations = { @@ -2368,5 +2368,5 @@ struct inode_operations ext3_special_ino .getxattr = ext3_getxattr, .listxattr = ext3_listxattr, .removexattr = ext3_removexattr, - .permission = ext3_permission, + .check_posix_acl = ext3_check_posix_acl, }; --Boundary-00=_UEiNB04+bVQ3MGZ--