linux-kernel - [PATCH] fs: RESOLVE_CACHED final path component fix

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20231109190844.2044940-1-agruenba@redhat.com>
Date:   Thu,  9 Nov 2023 20:08:44 +0100
From:   Andreas Gruenbacher <agruenba@...hat.com>
To:     Jens Axboe <axboe@...nel.dk>
Cc:     Andreas Gruenbacher <agruenba@...hat.com>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Christian Brauner <brauner@...nel.org>,
        Abhi Das <adas@...hat.com>, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: [PATCH] fs: RESOLVE_CACHED final path component fix

Jens,

since your commit 99668f618062, applications can request cached lookups
with the RESOLVE_CACHED openat2() flag.  When adding support for that in
gfs2, we found that this causes the ->permission inode operation to be
called with the MAY_NOT_BLOCK flag set for directories along the path,
which is good, but the ->permission check on the final path component is
missing that flag.  The filesystem will then sleep when it needs to read
in the ACL, for example.

This doesn't look like the intended RESOLVE_CACHED behavior.

The file permission checks in path_openat() happen as follows:

(1) link_path_walk() -> may_lookup() -> inode_permission() is called for
each but the final path component. If the LOOKUP_RCU nameidata flag is
set, may_lookup() passes the MAY_NOT_BLOCK flag on to
inode_permission(), which passes it on to the permission inode
operation.

(2) do_open() -> may_open() -> inode_permission() is called for the
final path component. The MAY_* flags passed to inode_permission() are
computed by build_open_flags(), outside of do_open(), and passed down
from there. The MAY_NOT_BLOCK flag doesn't get set.

I think we can fix this in build_open_flags(), by setting the
MAY_NOT_BLOCK flag when a RESOLVE_CACHED lookup is requested, right
where RESOLVE_CACHED is mapped to LOOKUP_CACHED as well.

Fixes: 99668f618062 ("fs: expose LOOKUP_CACHED through openat2() RESOLVE_CACHED")
Signed-off-by: Andreas Gruenbacher <agruenba@...hat.com>

diff --git a/fs/open.c b/fs/open.c
index 98f6601fbac6..61311c9845bd 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1340,6 +1340,7 @@ inline int build_open_flags(const struct open_how *how, struct open_flags *op)
 		if (flags & (O_TRUNC | O_CREAT | __O_TMPFILE))
 			return -EAGAIN;
 		lookup_flags |= LOOKUP_CACHED;
+		op->acc_mode |= MAY_NOT_BLOCK;
 	}

 	op->lookup_flags = lookup_flags;