From: Al Viro <viro@zeniv.linux.org.uk>
To: NeilBrown <neil@brown.name>
Cc: Trond Myklebust <trondmy@kernel.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Christian Brauner <brauner@kernel.org>,
linux-fsdevel@vger.kernel.org, Jan Kara <jack@suse.cz>
Subject: Re: [RFC] a possible way of reducing the PITA of ->d_name audits
Date: Sun, 14 Sep 2025 02:37:30 +0100 [thread overview]
Message-ID: <20250914013730.GF39973@ZenIV> (raw)
In-Reply-To: <175781190836.1696783.10753790171717564249@noble.neil.brown.name>
On Sun, Sep 14, 2025 at 11:05:08AM +1000, NeilBrown wrote:
> READDIR without establishing and "open" state.
>
> Why do you think nfs4_opendata_access() expects the possibilty of a
> directory?
static int nfs4_opendata_access(const struct cred *cred,
struct nfs4_opendata *opendata,
struct nfs4_state *state, fmode_t fmode)
{
struct nfs_access_entry cache;
u32 mask, flags;
/* access call failed or for some reason the server doesn't
* support any access modes -- defer access call until later */
if (opendata->o_res.access_supported == 0)
return 0;
mask = 0;
if (fmode & FMODE_EXEC) {
/* ONLY check for exec rights */
if (S_ISDIR(state->inode->i_mode))
^^^^^^^
How else would you describe this?
mask = NFS4_ACCESS_LOOKUP;
else
mask = NFS4_ACCESS_EXECUTE;
> > if (d_inode(dentry) == state->inode)
> > nfs_inode_attach_open_context(ctx);
> > shortly afterwards (incidentally, what is that check about? It can only
> > fail in case of nfs4_file_open(); should we have open(2) succeed in such
> > situation?)
>
> I don't know what is going on here.
> Based on the commit that introduced this code
>
> Commit: c45ffdd26961 ("NFSv4: Close another NFSv4 recovery race")
>
> there is presumably some race that can cause the test to fail.
> Maybe Trond (cc:ed) could help explain.
AFAICS, it can happen if you are there from nfs4_file_open(), hit
_nfs4_opendata_to_nfs4_state(opendata), find ->rpc_done to be true
in there, hit nfs4_opendata_find_nfs4_state(), have it call
nfs4_opendata_get_inode() and run into a server without
NFS_CAP_ATOMIC_OPEN_V1. Then you get ->o_arg.claim set to
NFS4_OPEN_CLAIM_NULL and hit this:
inode = nfs_fhget(data->dir->d_sb, &data->o_res.fh,
&data->f_attr);
finding not the same inode as your dentry has attached to it.
So the test might end up not being true, at least from my reading of
that code.
What I don't understand is the reasons for not failing immediately
with EOPENSTALE in that case.
TBH, I would be a lot more comfortable if the "attach inode to dentry"
logics in there had been taken several levels up the call chains - analysis
would be much easier that way...
next prev parent reply other threads:[~2025-09-14 1:37 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-07 20:32 [RFC] a possible way of reducing the PITA of ->d_name audits Al Viro
2025-09-07 21:51 ` Linus Torvalds
2025-09-08 0:06 ` Al Viro
2025-09-08 0:47 ` Linus Torvalds
2025-09-08 2:51 ` Al Viro
2025-09-08 3:57 ` Al Viro
2025-09-08 4:50 ` NeilBrown
2025-09-08 5:19 ` Al Viro
2025-09-08 6:25 ` NeilBrown
2025-09-08 9:05 ` Al Viro
2025-09-10 2:45 ` NeilBrown
2025-09-10 7:24 ` Al Viro
2025-09-10 22:52 ` NeilBrown
2025-09-12 5:49 ` ->atomic_open() fun (was Re: [RFC] a possible way of reducing the PITA of ->d_name audits) Al Viro
2025-09-12 8:23 ` Miklos Szeredi
2025-09-12 18:29 ` Al Viro
2025-09-12 19:22 ` Miklos Szeredi
2025-09-12 20:36 ` Al Viro
2025-09-12 20:50 ` Al Viro
2025-09-13 3:36 ` NeilBrown
2025-09-13 5:07 ` Al Viro
2025-09-13 5:50 ` NeilBrown
2025-09-14 19:01 ` Miklos Szeredi
2025-09-14 19:50 ` Al Viro
2025-09-14 20:05 ` Miklos Szeredi
2025-09-15 8:54 ` Bernd Schubert
2025-09-12 18:55 ` Al Viro
2025-09-12 18:59 ` [PATCH 1/9] allow finish_no_open(file, ERR_PTR(-E...)) Al Viro
2025-09-12 18:59 ` [PATCH 2/9] 9p: simplify v9fs_vfs_atomic_open() Al Viro
2025-09-12 18:59 ` [PATCH 3/9] 9p: simplify v9fs_vfs_atomic_open_dotl() Al Viro
2025-09-12 18:59 ` [PATCH 4/9] simplify cifs_atomic_open() Al Viro
2025-09-12 18:59 ` [PATCH 5/9] simplify vboxsf_dir_atomic_open() Al Viro
2025-09-12 18:59 ` [PATCH 6/9] simplify nfs_atomic_open_v23() Al Viro
2025-09-12 18:59 ` [PATCH 7/9] simplify fuse_atomic_open() Al Viro
2025-09-12 18:59 ` [PATCH 8/9] simplify gfs2_atomic_open() Al Viro
2025-09-12 18:59 ` [PATCH 9/9] slightly simplify nfs_atomic_open() Al Viro
2025-09-12 22:23 ` [PATCH 1/9] allow finish_no_open(file, ERR_PTR(-E...)) Linus Torvalds
2025-09-13 3:34 ` NeilBrown
2025-09-13 21:28 ` [RFC] a possible way of reducing the PITA of ->d_name audits Al Viro
2025-09-14 1:05 ` NeilBrown
2025-09-14 1:37 ` Al Viro [this message]
2025-09-14 5:56 ` Al Viro
2025-09-14 23:07 ` NeilBrown
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=20250914013730.GF39973@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=brauner@kernel.org \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=neil@brown.name \
--cc=torvalds@linux-foundation.org \
--cc=trondmy@kernel.org \
/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.