From e8e66ed25b5cbeebed69c475f6c108e52078a3b3 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 28 Aug 2009 10:05:33 -0700 Subject: Do not call 'ima_path_check()' for each path component Not only is that a supremely timing-critical path, but it's hopefully some day going to be lockless for the common case, and ima can't do that. Plus the integrity code doesn't even care about non-regular files, so it was always a total waste of time and effort. Acked-by: Serge Hallyn Acked-by: Mimi Zohar Signed-off-by: Linus Torvalds --- fs/namei.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'fs/namei.c') diff --git a/fs/namei.c b/fs/namei.c index 1f13751693a5..a005d8b7afad 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -856,9 +856,6 @@ static int __link_path_walk(const char *name, struct nameidata *nd) if (err == -EAGAIN) err = inode_permission(nd->path.dentry->d_inode, MAY_EXEC); - if (!err) - err = ima_path_check(&nd->path, MAY_EXEC, - IMA_COUNT_UPDATE); if (err) break; -- cgit v1.2.3 From b7a437b08a44a3ed7e3a052eb39d2c5f618b603b Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 28 Aug 2009 10:50:37 -0700 Subject: Simplify exec_permission_lite() logic Instead of returning EAGAIN and having the caller do something special for that case, just do the special case directly. Reviewed-by: James Morris Acked-by: Serge Hallyn Signed-off-by: Linus Torvalds --- fs/namei.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'fs/namei.c') diff --git a/fs/namei.c b/fs/namei.c index a005d8b7afad..8c3580610eec 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -435,7 +435,7 @@ static int exec_permission_lite(struct inode *inode) umode_t mode = inode->i_mode; if (inode->i_op->permission) - return -EAGAIN; + return inode_permission(inode, MAY_EXEC); if (current_fsuid() == inode->i_uid) mode >>= 6; @@ -853,9 +853,6 @@ static int __link_path_walk(const char *name, struct nameidata *nd) nd->flags |= LOOKUP_CONTINUE; err = exec_permission_lite(inode); - if (err == -EAGAIN) - err = inode_permission(nd->path.dentry->d_inode, - MAY_EXEC); if (err) break; -- cgit v1.2.3 From f1ac9f6bfea6f21e8ab6dbbe46879d62a6fba8c0 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 28 Aug 2009 10:53:56 -0700 Subject: Simplify exec_permission_lite() further This function is only called for path components that are already known to be directories (they have a '->lookup' method). So don't bother doing that whole S_ISDIR() testing, the whole point of the 'lite()' version is that we know that we are looking at a directory component, and that we're only checking name lookup permission. Reviewed-by: James Morris Acked-by: Serge Hallyn Signed-off-by: Linus Torvalds --- fs/namei.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'fs/namei.c') diff --git a/fs/namei.c b/fs/namei.c index 8c3580610eec..929f535fb225 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -445,13 +445,7 @@ static int exec_permission_lite(struct inode *inode) if (mode & MAY_EXEC) goto ok; - if ((inode->i_mode & S_IXUGO) && capable(CAP_DAC_OVERRIDE)) - goto ok; - - if (S_ISDIR(inode->i_mode) && capable(CAP_DAC_OVERRIDE)) - goto ok; - - if (S_ISDIR(inode->i_mode) && capable(CAP_DAC_READ_SEARCH)) + if (capable(CAP_DAC_OVERRIDE) || capable(CAP_DAC_READ_SEARCH)) goto ok; return -EACCES; -- cgit v1.2.3 From cb9179ead0aa0e3b7b4087cdba59baf16bbeef6d Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 28 Aug 2009 11:08:31 -0700 Subject: Simplify exec_permission_lite(), part 3 Don't call down to the generic inode_permission() function just to call the inode-specific permission function - just do it directly. The generic inode_permission() code does things like checking MAY_WRITE and devcgroup_inode_permission(), neither of which are relevant for the light pathname walk permission checks (we always do just MAY_EXEC, and the inode is never a special device). Reviewed-by: James Morris Acked-by: Serge Hallyn Signed-off-by: Linus Torvalds --- fs/namei.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'fs/namei.c') diff --git a/fs/namei.c b/fs/namei.c index 929f535fb225..e645e3070360 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -434,8 +434,12 @@ static int exec_permission_lite(struct inode *inode) { umode_t mode = inode->i_mode; - if (inode->i_op->permission) - return inode_permission(inode, MAY_EXEC); + if (inode->i_op->permission) { + int ret = inode->i_op->permission(inode, MAY_EXEC); + if (!ret) + goto ok; + return ret; + } if (current_fsuid() == inode->i_uid) mode >>= 6; -- cgit v1.2.3 From 5909ccaa300a4a834ffa275327af4df0b9cb5295 Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Fri, 28 Aug 2009 11:51:25 -0700 Subject: Make 'check_acl()' a first-class filesystem op This is stage one in flattening out the callchains for the common permission testing. Rather than have most filesystem implement their own inode->i_op->permission function that just calls back down to the VFS layers 'generic_permission()' with the per-filesystem ACL checking function, the filesystem can just expose its 'check_acl' function directly, and let the VFS layer do everything for it. This is all just preparatory - no filesystem actually enables this yet. Reviewed-by: James Morris Acked-by: Serge Hallyn Signed-off-by: Linus Torvalds --- fs/namei.c | 62 ++++++++++++++++++++++++++++++------------------------ include/linux/fs.h | 1 + 2 files changed, 36 insertions(+), 27 deletions(-) (limited to 'fs/namei.c') diff --git a/fs/namei.c b/fs/namei.c index e645e3070360..ed27bb205b7e 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -169,19 +169,10 @@ void putname(const char *name) EXPORT_SYMBOL(putname); #endif - -/** - * generic_permission - check for access rights on a Posix-like filesystem - * @inode: inode to check access rights for - * @mask: right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC) - * @check_acl: optional callback to check for Posix ACLs - * - * Used to check for read/write/execute permissions on a file. - * We use "fsuid" for this, letting us set arbitrary permissions - * for filesystem access without changing the "normal" uids which - * are used for other things.. +/* + * This does basic POSIX ACL permission checking */ -int generic_permission(struct inode *inode, int mask, +static int acl_permission_check(struct inode *inode, int mask, int (*check_acl)(struct inode *inode, int mask)) { umode_t mode = inode->i_mode; @@ -193,9 +184,7 @@ int generic_permission(struct inode *inode, int mask, else { if (IS_POSIXACL(inode) && (mode & S_IRWXG) && check_acl) { int error = check_acl(inode, mask); - if (error == -EACCES) - goto check_capabilities; - else if (error != -EAGAIN) + if (error != -EAGAIN) return error; } @@ -208,8 +197,32 @@ int generic_permission(struct inode *inode, int mask, */ if ((mask & ~mode) == 0) return 0; + return -EACCES; +} + +/** + * generic_permission - check for access rights on a Posix-like filesystem + * @inode: inode to check access rights for + * @mask: right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC) + * @check_acl: optional callback to check for Posix ACLs + * + * Used to check for read/write/execute permissions on a file. + * We use "fsuid" for this, letting us set arbitrary permissions + * for filesystem access without changing the "normal" uids which + * are used for other things.. + */ +int generic_permission(struct inode *inode, int mask, + int (*check_acl)(struct inode *inode, int mask)) +{ + int ret; + + /* + * Do the basic POSIX ACL permission checks. + */ + ret = acl_permission_check(inode, mask, check_acl); + if (ret != -EACCES) + return ret; - check_capabilities: /* * Read/write DACs are always overridable. * Executable DACs are overridable if at least one exec bit is set. @@ -262,7 +275,7 @@ int inode_permission(struct inode *inode, int mask) if (inode->i_op->permission) retval = inode->i_op->permission(inode, mask); else - retval = generic_permission(inode, mask, NULL); + retval = generic_permission(inode, mask, inode->i_op->check_acl); if (retval) return retval; @@ -432,27 +445,22 @@ static struct dentry * cached_lookup(struct dentry * parent, struct qstr * name, */ static int exec_permission_lite(struct inode *inode) { - umode_t mode = inode->i_mode; + int ret; if (inode->i_op->permission) { - int ret = inode->i_op->permission(inode, MAY_EXEC); + ret = inode->i_op->permission(inode, MAY_EXEC); if (!ret) goto ok; return ret; } - - if (current_fsuid() == inode->i_uid) - mode >>= 6; - else if (in_group_p(inode->i_gid)) - mode >>= 3; - - if (mode & MAY_EXEC) + ret = acl_permission_check(inode, MAY_EXEC, inode->i_op->check_acl); + if (!ret) goto ok; if (capable(CAP_DAC_OVERRIDE) || capable(CAP_DAC_READ_SEARCH)) goto ok; - return -EACCES; + return ret; ok: return security_inode_permission(inode, MAY_EXEC); } diff --git a/include/linux/fs.h b/include/linux/fs.h index 73e9b643e455..c1f993515f51 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -1528,6 +1528,7 @@ struct inode_operations { void (*put_link) (struct dentry *, struct nameidata *, void *); void (*truncate) (struct inode *); int (*permission) (struct inode *, int); + int (*check_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); -- cgit v1.2.3