fs: optimize acl_permission_check()

generic_permission() turned out to be costlier than expected. The reason
is that acl_permission_check() performs owner checks that involve costly
pointer dereferences.

There's already code that skips expensive group checks if the group and
other permission bits are the same wrt to the mask that is checked. This
logic can be extended to also shortcut permission checking in
acl_permission_check().

If the inode doesn't have POSIX ACLs than ownership doesn't matter. If
there are no missing UGO permissions the permission check can be
shortcut.

Acked-by: Al Viro <viro@zeniv.linux.org.uk>
Link: https://lore.kernel.org/all/CAHk-=wgXEoAOFRkDg+grxs+p1U+QjWXLixRGmYEfd=vG+OBuFw@mail.gmail.com
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/fs/namei.c b/fs/namei.c
index 4a4a22a08ac20df0e2cbe4b242d47eb4bc630faa..05a8d544fb351ed5f3de1f199a159e2060761b01 100644 (file)
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -326,6 +326,25 @@ static int check_acl(struct mnt_idmap *idmap,
        return -EAGAIN;
 }
 
+/*
+ * Very quick optimistic "we know we have no ACL's" check.
+ *
+ * Note that this is purely for ACL_TYPE_ACCESS, and purely
+ * for the "we have cached that there are no ACLs" case.
+ *
+ * If this returns true, we know there are no ACLs. But if
+ * it returns false, we might still not have ACLs (it could
+ * be the is_uncached_acl() case).
+ */
+static inline bool no_acl_inode(struct inode *inode)
+{
+#ifdef CONFIG_FS_POSIX_ACL
+       return likely(!READ_ONCE(inode->i_acl));
+#else
+       return true;
+#endif
+}
+
 /**
  * acl_permission_check - perform basic UNIX permission checking
  * @idmap:     idmap of the mount the inode was found from
@@ -348,6 +367,28 @@ static int acl_permission_check(struct mnt_idmap *idmap,
        unsigned int mode = inode->i_mode;
        vfsuid_t vfsuid;
 
+       /*
+        * Common cheap case: everybody has the requested
+        * rights, and there are no ACLs to check. No need
+        * to do any owner/group checks in that case.
+        *
+        *  - 'mask&7' is the requested permission bit set
+        *  - multiplying by 0111 spreads them out to all of ugo
+        *  - '& ~mode' looks for missing inode permission bits
+        *  - the '!' is for "no missing permissions"
+        *
+        * After that, we just need to check that there are no
+        * ACL's on the inode - do the 'IS_POSIXACL()' check last
+        * because it will dereference the ->i_sb pointer and we
+        * want to avoid that if at all possible.
+        */
+       if (!((mask & 7) * 0111 & ~mode)) {
+               if (no_acl_inode(inode))
+                       return 0;
+               if (!IS_POSIXACL(inode))
+                       return 0;
+       }
+
        /* Are we the owner? If so, ACL's don't matter */
        vfsuid = i_uid_into_vfsuid(idmap, inode);
        if (likely(vfsuid_eq_kuid(vfsuid, current_fsuid()))) {