All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: NeilBrown <neil@brown.name>
Cc: 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: Sat, 13 Sep 2025 22:28:15 +0100	[thread overview]
Message-ID: <20250913212815.GE39973@ZenIV> (raw)
In-Reply-To: <20250908090557.GJ31600@ZenIV>

On Mon, Sep 08, 2025 at 10:05:57AM +0100, Al Viro wrote:

> > Fudging some type-state with C may well be useful but I suspect it is at
> > most part of a solution.  Simplification, documentation, run-time checks
> > might also be important parts.  As the type-state flag-day is a big
> > thing, maybe it shouldn't be first.
> 
> All of that requires being able to answer questions about what's there in
> the existing filesystems.  Which is pretty much the same problem as
> those audits, obviously.  And static annotations are way easier to
> reason about.

Speaking of annoyances, d_exact_alias() is gone, and good riddance, but there's
another fun issue in the same area - environment for d_splice_alias() call *and*
for one of those d_drop()-just-in-case.

The call tree is still the same:
_nfs4_open_and_get_state()
	<- _nfs4_do_open()
		<- nfs4_do_open()
			<- nfs4_atomic_open()
				== nfs_rpc_ops:open_context
					<- nfs_atomic_open()
						== ->atomic_open
					<- nfs4_file_open()
						== ->open
			<- nfs4_proc_create()
				== nfs_rpc_ops:create
					<- nfs_do_create()
						<- nfs_create()
							== ->create
						<- nfs_atomic_open_v23(), with O_CREAT
							== ->atomic_open
							# won't reach nfs4 stuff?

->create() and ->atomic_open() have the parent held at least shared;
->open() does not, but the chunk in question is hit only if dentry
is negative, which won't happen in case of ->open().

Additional complication comes from the possibility for _nfs4_open_and_get_state()
to fail after that d_splice_alias().  In that case we have _nfs4_do_open()
return an error; its caller is inside a do-while loop in nfs4_do_open() and
I think we can't end up going around the loop after such late failure (the
only error possible after that is -EACCES/-NFS4ERR_ACCESS and that's not one
of those that can lead to more iterations.

	However, looking at that late failure, that's the only call of
nfs4_opendata_access(), and that function seems to expect the possibility
of state->inode being a directory; can that really happen?

	Because if it can, we have a problem:
                alias = d_splice_alias(igrab(state->inode), dentry);
                /* d_splice_alias() can't fail here - it's a non-directory */
                if (alias) {
                        dput(ctx->dentry);
                        ctx->dentry = dentry = alias;
                }
very much *can* fail if it's reached with state->inode being a directory -
we can get ERR_PTR() out of that d_splice_alias() and that will oops at

        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?)

Sigh...

  parent reply	other threads:[~2025-09-13 21:28 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                   ` Al Viro [this message]
2025-09-14  1:05                     ` [RFC] a possible way of reducing the PITA of ->d_name audits NeilBrown
2025-09-14  1:37                       ` Al Viro
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=20250913212815.GE39973@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 \
    /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.