summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLinus Torvalds <torvalds@linux-foundation.org>2020-06-05 13:40:45 -0700
committerLinus Torvalds <torvalds@linux-foundation.org>2020-06-08 11:04:19 -0700
commit5fc475b749c72e0b3f3991b33a90d302f52ae746 (patch)
tree0b4c46c6b95411925ed464b135cee6ab8c864481
parentaf7b4801030c07637840191c69eb666917e4135d (diff)
vfs: do not do group lookup when not necessary
Rasmus Villemoes points out that the 'in_group_p()' tests can be a noticeable expense, and often completely unnecessary. A common situation is that the 'group' bits are the same as the 'other' bits wrt the permissions we want to test. So rewrite 'acl_permission_check()' to not bother checking for group ownership when the permission check doesn't care. For example, if we're asking for read permissions, and both 'group' and 'other' allow reading, there's really no reason to check if we're part of the group or not: either way, we'll allow it. Rasmus says: "On a bog-standard Ubuntu 20.04 install, a workload consisting of compiling lots of userspace programs (i.e., calling lots of short-lived programs that all need to get their shared libs mapped in, and the compilers poking around looking for system headers - lots of /usr/lib, /usr/bin, /usr/include/ accesses) puts in_group_p around 0.1% according to perf top. System-installed files are almost always 0755 (directories and binaries) or 0644, so in most cases, we can avoid the binary search and the cost of pulling the cred->groups array and in_group_p() .text into the cpu cache" Reported-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--fs/namei.c44
1 files changed, 29 insertions, 15 deletions
diff --git a/fs/namei.c b/fs/namei.c
index d81f73ff1a8b..e74a7849e9b5 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -288,37 +288,51 @@ static int check_acl(struct inode *inode, int mask)
}
/*
- * This does the basic permission checking
+ * This does the basic UNIX permission checking.
+ *
+ * Note that the POSIX ACL check cares about the MAY_NOT_BLOCK bit,
+ * for RCU walking.
*/
static int acl_permission_check(struct inode *inode, int mask)
{
unsigned int mode = inode->i_mode;
- if (likely(uid_eq(current_fsuid(), inode->i_uid)))
+ /* Are we the owner? If so, ACL's don't matter */
+ if (likely(uid_eq(current_fsuid(), inode->i_uid))) {
+ mask &= 7;
mode >>= 6;
- else {
- if (IS_POSIXACL(inode) && (mode & S_IRWXG)) {
- int error = check_acl(inode, mask);
- if (error != -EAGAIN)
- return error;
- }
+ return (mask & ~mode) ? -EACCES : 0;
+ }
- if (in_group_p(inode->i_gid))
- mode >>= 3;
+ /* Do we have ACL's? */
+ if (IS_POSIXACL(inode) && (mode & S_IRWXG)) {
+ int error = check_acl(inode, mask);
+ if (error != -EAGAIN)
+ return error;
}
+ /* Only RWX matters for group/other mode bits */
+ mask &= 7;
+
/*
- * If the DACs are ok we don't need any capability check.
+ * Are the group permissions different from
+ * the other permissions in the bits we care
+ * about? Need to check group ownership if so.
*/
- if ((mask & ~mode & (MAY_READ | MAY_WRITE | MAY_EXEC)) == 0)
- return 0;
- return -EACCES;
+ if (mask & (mode ^ (mode >> 3))) {
+ if (in_group_p(inode->i_gid))
+ mode >>= 3;
+ }
+
+ /* Bits in 'mode' clear that we require? */
+ return (mask & ~mode) ? -EACCES : 0;
}
/**
* 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, ...)
+ * @mask: right to check for (%MAY_READ, %MAY_WRITE, %MAY_EXEC,
+ * %MAY_NOT_BLOCK ...)
*
* Used to check for read/write/execute permissions on a file.
* We use "fsuid" for this, letting us set arbitrary permissions