Re: [BUG] fanotify: destroy/add race leaves a mark on a detached connector

[Amir Goldstein]

On Mon, Oct 12, 2026 at 1:48 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Thu, Sep 3, 2026 at 3:07 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> >
> > On Wed, Sep 2, 2026 at 11:51 PM Daehyeon Ko <4ncienth@xxxxxxxxx> wrote:
> > >
> > > Hi Amir,
> > >
> > > I was able to make the handle-reopen sequence reliable on ext4 and drafted
> > > an LTP test for it.
> > >
> > > The test keeps 120 auxiliary inode marks on the victim to widen the interval
> > > after the first FAN_DELETE_SELF, pins the unlink and handle-reopen workers to
> > > different CPUs, and has the reopen worker read the event queue nonblocking
> > > before calling open_by_handle_at().
> > >
> > > I ran the exact final test for 100 iterations in each of two fresh 2-vCPU
> > > KASAN boots per kernel:
> > >
> > >   - unmodified v7.2: 200/200 iterations reopened the unlinked inode, and
> > >     closing the reopened fd generated a second FAN_DELETE_SELF;
> > >   - v7.2 with your patch: 200/200 iterations returned ESTALE from
> > >     open_by_handle_at(), with no duplicate FAN_DELETE_SELF.
> > >
> > > The test is based on LTP master commit
> > > 12724413534a6d4160ff9694ba6f09daa4ccb6bd. The attached patch adds
> > > fanotify26 and the corresponding build and runtest entries.
> >
> > Very nice!
> >
> > I think we should wait for Christian to pick up the fix or suggest an
> > alternative, but after that please post this test to LTP.
> >
> > Thanks for following up on this!
>
> Christian,
>
> Please apply this attached patch in-place of 669bb5af15d41 ("fs: make
> sure to call fsnotify_inoderemove() only once when inode is dead") on
> your vfs-7.4.misc branch.
>
> The difference from v1 [1] is removal of the obtain_alias parts, which are
> wrong for network filesystems.
>
> The LTP test provided by Daehyeon now fails because it uses open_by_handle_at()
> for the reproducer, but a similar race could be created with hardlinks
> release/unlink race and this case should be fixed by this v2, meaning that
> FS_DELETE_SELF is reported when all hardlinks are unlinked and all
> open files to all hardlinks are closed.

Sorry I forgot to update the comment.
Attached v3 with the comment change.

Thanks,
Amir.

>
> [1] https://lore.kernel.org/linux-fsdevel/CAOQ4uxjbEY12XMmfCXTPTpASmECCXcGBQR+bS93k=4vgqzFDZA@xxxxxxxxxxxxxx/
From ff91e13847a5dbee4e864a47be5bf4bcc8199b74 Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir73il@xxxxxxxxx>
Date: Wed, 2 Sep 2026 18:49:25 +0200
Subject: [PATCH v3] fs: make sure to call fsnotify_inoderemove() only once
 when inode is dead

Daehyeon reported a race wherein fsnotify_removeinode() can be called
when inode still has a live alias.  This can lead to several undesired
outcomes such as crashes in fsnotify code or avoidance of inode mark
notifications.

Check that inode is really "dead", meaning no nlink and no more aliases
before calling fsnotify_inoderemove() for final cleanup of inode marks.

Ideally, we'd call fsnotify_inoderemove() once last inode reference is
dropped to make things simpler. Alas fsnotify inode marks currently hold
inode references and thus this doesn't work. Once we teach fsnotify to
avoid holding inode references, these games with dead inodes can go as
well.

Note that a new alias could be created by open_by_handle() to an inode
that has already reported FS_DELETE_SELF.  This case will be handled by
a different patch.

Reported-by: Daehyeon Ko <4ncienth@xxxxxxxxx>
Closes: https://lore.kernel.org/linux-fsdevel/20260827045007.3831259-1-4ncienth@xxxxxxxxx/
CC: stable@xxxxxxxxxxxxxxx
Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
Link: https://patch.msgid.link/20260902164924.1333135-2-jack@xxxxxxx
---
 fs/dcache.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index b89c8480b655b..86a3a00929de1 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -450,6 +450,16 @@ static void dentry_free(struct dentry *dentry)
 		call_rcu(&dentry->d_rcu, __d_free);
 }
 
+/*
+ * When inode is unlinked and doesn't have any aliases (i.e., all fds pointing
+ * to it are closed) we report FS_DELETE_SELF to fsnotify and cleanup all inode
+ * marks.
+ */
+static bool inode_notify_dead(struct inode *inode)
+{
+	return !inode->i_nlink && hlist_empty(&inode->i_dentry);
+}
+
 /*
  * Release the dentry's inode, using the filesystem
  * d_iput() operation if defined.
@@ -459,6 +469,7 @@ static void dentry_unlink_inode(struct dentry * dentry)
 	__releases(dentry->d_inode->i_lock)
 {
 	struct inode *inode = dentry->d_inode;
+	bool notify_dead;
 
 	raw_write_seqcount_begin(&dentry->d_seq);
 	__d_clear_type_and_inode(dentry);
@@ -469,9 +480,10 @@ static void dentry_unlink_inode(struct dentry * dentry)
 	 */
 	dentry->waiters = NULL;
 	raw_write_seqcount_end(&dentry->d_seq);
+	notify_dead = inode_notify_dead(inode);
 	spin_unlock(&dentry->d_lock);
 	spin_unlock(&inode->i_lock);
-	if (!inode->i_nlink)
+	if (notify_dead)
 		fsnotify_inoderemove(inode);
 	if (dentry->d_op && dentry->d_op->d_iput)
 		dentry->d_op->d_iput(dentry, inode);
-- 
2.55.0