linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: NeilBrown <neilb@ownmail.net>
To: "Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Christian Brauner" <brauner@kernel.org>,
	"Val Packett" <val@packett.cool>
Cc: "Amir Goldstein" <amir73il@gmail.com>, "Jan Kara" <jack@suse.cz>,
	linux-fsdevel@vger.kernel.org, "Jeff Layton" <jlayton@kernel.org>,
	"Chris Mason" <clm@fb.com>, "David Sterba" <dsterba@suse.com>,
	"David Howells" <dhowells@redhat.com>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Tyler Hicks" <code@tyhicks.com>,
	"Miklos Szeredi" <miklos@szeredi.hu>,
	"Chuck Lever" <chuck.lever@oracle.com>,
	"Olga Kornievskaia" <okorniev@redhat.com>,
	"Dai Ngo" <Dai.Ngo@oracle.com>,
	"Namjae Jeon" <linkinjeon@kernel.org>,
	"Steve French" <smfrench@gmail.com>,
	"Sergey Senozhatsky" <senozhatsky@chromium.org>,
	"Carlos Maiolino" <cem@kernel.org>,
	"John Johansen" <john.johansen@canonical.com>,
	"Paul Moore" <paul@paul-moore.com>,
	"James Morris" <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	"Stephen Smalley" <stephen.smalley.work@gmail.com>,
	"Ondrej Mosnacek" <omosnace@redhat.com>,
	"Mateusz Guzik" <mjguzik@gmail.com>,
	"Lorenzo Stoakes" <lorenzo.stoakes@oracle.com>,
	"Stefan Berger" <stefanb@linux.ibm.com>,
	"Darrick J. Wong" <djwong@kernel.org>,
	linux-kernel@vger.kernel.org, netfs@lists.linux.dev,
	ecryptfs@vger.kernel.org, linux-nfs@vger.kernel.org,
	linux-unionfs@vger.kernel.org, linux-cifs@vger.kernel.org,
	linux-xfs@vger.kernel.org, linux-security-module@vger.kernel.org,
	selinux@vger.kernel.org
Subject: [PATCH] fuse: fix conversion of fuse_reverse_inval_entry() to start_removing()
Date: Mon, 01 Dec 2025 09:06:18 +1100	[thread overview]
Message-ID: <176454037897.634289.3566631742434963788@noble.neil.brown.name> (raw)
In-Reply-To: <6713ea38-b583-4c86-b74a-bea55652851d@packett.cool>


From: NeilBrown <neil@brown.name>

The recent conversion of fuse_reverse_inval_entry() to use
start_removing() was wrong.
As Val Packett points out the original code did not call ->lookup
while the new code does.  This can lead to a deadlock.

Rather than using full_name_hash() and d_lookup() as the old code
did, we can use try_lookup_noperm() which combines these.  Then
the result can be given to start_removing_dentry() to get the required
locks for removal.  We then double check that the name hasn't
changed.

As 'dir' needs to be used several times now, we load the dput() until
the end, and initialise to NULL so dput() is always safe.

Reported-by: Val Packett <val@packett.cool>
Closes: https://lore.kernel.org/all/6713ea38-b583-4c86-b74a-bea55652851d@packett.cool
Fixes: c9ba789dad15 ("VFS: introduce start_creating_noperm() and start_removing_noperm()")
Signed-off-by: NeilBrown <neil@brown.name>
---
 fs/fuse/dir.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index a0d5b302bcc2..8384fa96cf53 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1390,8 +1390,8 @@ int fuse_reverse_inval_entry(struct fuse_conn *fc, u64 parent_nodeid,
 {
 	int err = -ENOTDIR;
 	struct inode *parent;
-	struct dentry *dir;
-	struct dentry *entry;
+	struct dentry *dir = NULL;
+	struct dentry *entry = NULL;
 
 	parent = fuse_ilookup(fc, parent_nodeid, NULL);
 	if (!parent)
@@ -1404,11 +1404,19 @@ int fuse_reverse_inval_entry(struct fuse_conn *fc, u64 parent_nodeid,
 	dir = d_find_alias(parent);
 	if (!dir)
 		goto put_parent;
-
-	entry = start_removing_noperm(dir, name);
-	dput(dir);
-	if (IS_ERR(entry))
-		goto put_parent;
+	while (!entry) {
+		struct dentry *child = try_lookup_noperm(name, dir);
+		if (!child || IS_ERR(child))
+			goto put_parent;
+		entry = start_removing_dentry(dir, child);
+		dput(child);
+		if (IS_ERR(entry))
+			goto put_parent;
+		if (!d_same_name(entry, dir, name)) {
+			end_removing(entry);
+			entry = NULL;
+		}
+	}
 
 	fuse_dir_changed(parent);
 	if (!(flags & FUSE_EXPIRE_ONLY))
@@ -1446,6 +1454,7 @@ int fuse_reverse_inval_entry(struct fuse_conn *fc, u64 parent_nodeid,
 
 	end_removing(entry);
  put_parent:
+	dput(dir);
 	iput(parent);
 	return err;
 }
-- 
2.50.0.107.gf914562f5916.dirty


  parent reply	other threads:[~2025-11-30 22:06 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-13  0:18 [PATCH v6 00/15] Create and use APIs to centralise locking for directory ops NeilBrown
2025-11-13  0:18 ` [PATCH v6 01/15] debugfs: rename end_creating() to debugfs_end_creating() NeilBrown
2025-11-13  0:18 ` [PATCH v6 02/15] VFS: introduce start_dirop() and end_dirop() NeilBrown
2025-11-13  0:18 ` [PATCH v6 03/15] VFS: tidy up do_unlinkat() NeilBrown
2025-11-13  0:18 ` [PATCH v6 04/15] VFS/nfsd/cachefiles/ovl: add start_creating() and end_creating() NeilBrown
2025-11-13  0:18 ` [PATCH v6 05/15] VFS/nfsd/cachefiles/ovl: introduce start_removing() and end_removing() NeilBrown
2025-11-13  0:18 ` [PATCH v6 06/15] VFS: introduce start_creating_noperm() and start_removing_noperm() NeilBrown
2025-11-30  0:01   ` Val Packett
2025-11-30  0:19     ` Al Viro
2025-11-30 22:06     ` NeilBrown [this message]
2025-12-01  8:22       ` [PATCH] fuse: fix conversion of fuse_reverse_inval_entry() to start_removing() Amir Goldstein
2025-12-01  8:33         ` Al Viro
2025-12-01 14:03           ` Miklos Szeredi
2025-12-01 17:08             ` Al Viro
2025-12-02  8:46               ` Miklos Szeredi
2025-12-05 13:09             ` Christian Brauner
2025-12-15 14:19               ` Christian Brauner
2025-12-01  8:50         ` NeilBrown
2025-12-01  8:56           ` Al Viro
2025-11-13  0:18 ` [PATCH v6 07/15] smb/server: use end_removing_noperm for for target of smb2_create_link() NeilBrown
2025-11-13  0:18 ` [PATCH v6 08/15] VFS: introduce start_removing_dentry() NeilBrown
2025-11-13  0:18 ` [PATCH v6 09/15] VFS: add start_creating_killable() and start_removing_killable() NeilBrown
2025-11-13  0:18 ` [PATCH v6 10/15] VFS/nfsd/ovl: introduce start_renaming() and end_renaming() NeilBrown
2025-11-13  0:18 ` [PATCH v6 11/15] VFS/ovl/smb: introduce start_renaming_dentry() NeilBrown
2025-11-13  0:18 ` [PATCH v6 12/15] Add start_renaming_two_dentries() NeilBrown
2025-11-17 23:04   ` Paul Moore
2025-11-13  0:18 ` [PATCH v6 13/15] ecryptfs: use new start_creating/start_removing APIs NeilBrown
2025-11-13  0:18 ` [PATCH v6 14/15] VFS: change vfs_mkdir() to unlock on failure NeilBrown
2025-11-13  0:18 ` [PATCH v6 15/15] VFS: introduce end_creating_keep() NeilBrown
2025-11-14 12:24 ` [PATCH v6 00/15] Create and use APIs to centralise locking for directory ops Christian Brauner
2025-11-14 14:05   ` Jeff Layton
2025-11-14 14:23   ` Christian Brauner
2025-11-14 14:52     ` Jeff Layton
2025-11-14 22:00       ` Christian Brauner
2025-11-27 11:11     ` NeilBrown
2025-11-27 11:06   ` NeilBrown
2025-11-14 12:27 ` Christian Brauner

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=176454037897.634289.3566631742434963788@noble.neil.brown.name \
    --to=neilb@ownmail.net \
    --cc=Dai.Ngo@oracle.com \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=cem@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=clm@fb.com \
    --cc=code@tyhicks.com \
    --cc=dakr@kernel.org \
    --cc=dhowells@redhat.com \
    --cc=djwong@kernel.org \
    --cc=dsterba@suse.com \
    --cc=ecryptfs@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jack@suse.cz \
    --cc=jlayton@kernel.org \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=miklos@szeredi.hu \
    --cc=mjguzik@gmail.com \
    --cc=neil@brown.name \
    --cc=netfs@lists.linux.dev \
    --cc=okorniev@redhat.com \
    --cc=omosnace@redhat.com \
    --cc=paul@paul-moore.com \
    --cc=rafael@kernel.org \
    --cc=selinux@vger.kernel.org \
    --cc=senozhatsky@chromium.org \
    --cc=serge@hallyn.com \
    --cc=smfrench@gmail.com \
    --cc=stefanb@linux.ibm.com \
    --cc=stephen.smalley.work@gmail.com \
    --cc=val@packett.cool \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).