All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andreas Gruenbacher <agruenba@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: Andreas Gruenbacher <agruenba@redhat.com>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	Abhi Das <adas@redhat.com>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] fs: RESOLVE_CACHED final path component fix
Date: Thu,  9 Nov 2023 20:08:44 +0100	[thread overview]
Message-ID: <20231109190844.2044940-1-agruenba@redhat.com> (raw)

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@redhat.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;


             reply	other threads:[~2023-11-09 19:09 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-09 19:08 Andreas Gruenbacher [this message]
2023-11-09 22:00 ` [PATCH] fs: RESOLVE_CACHED final path component fix Al Viro
2023-11-09 22:11   ` Al Viro
2023-11-09 22:12   ` Andreas Grünbacher
2023-11-09 22:22     ` Al Viro
2023-12-12 19:37       ` Abhi Das

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231109190844.2044940-1-agruenba@redhat.com \
    --to=agruenba@redhat.com \
    --cc=adas@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.