Linux kernel -stable discussions
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] fs: make sure to call fsnotify_inoderemove() only once when inode is dead" failed to apply to 6.6-stable tree
@ 2026-11-07 10:28 Sasha Levin
  2026-11-10  3:29 ` [PATCH 6.6.y] fs: make sure to call fsnotify_inoderemove() only once when inode is dead Sasha Levin
  0 siblings, 1 reply; 5+ messages in thread
From: Sasha Levin @ 2026-11-07 10:28 UTC (permalink / raw)
  To: amir73il, 4ncienth, jack, brauner; +Cc: stable

The patch below does not apply to the 6.6-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
git checkout FETCH_HEAD
git cherry-pick -x 669bb5af15d41a21359d00a441490f87a88eb52a
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '20261107052836-failed-6.6-669bb5af15d4@kernel.org' --subject-prefix 'PATCH 6.6.y' 'HEAD^..'

Thanks,
Sasha

------------------ original commit in Linus's tree ------------------

From 669bb5af15d41a21359d00a441490f87a88eb52a Mon Sep 17 00:00:00 2001
From: Amir Goldstein <amir73il@gmail.com>
Date: Wed, 2 Sep 2026 18:49:25 +0200
Subject: [PATCH] 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.
Also do not allow open_by_handle() to add new aliases to a "dead" inode.

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.

Reported-and-tested-by: Daehyeon Ko <4ncienth@gmail.com>
Closes: https://lore.kernel.org/linux-fsdevel/20260827045007.3831259-1-4ncienth@gmail.com/
CC: stable@vger.kernel.org
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20260902164924.1333135-2-jack@suse.cz
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/dcache.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index 1b1a81f10da67..8f17db2724b38 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -450,6 +450,17 @@ static void dentry_free(struct dentry *dentry)
 		call_rcu(&dentry->d_rcu, __d_free);
 }
 
+/*
+ * If inode is unlinked and doesn't have any aliases (i.e., all fds pointing to
+ * it are closed), it is pretty much dead. Except that file handle lookup could
+ * still revive it which causes issues to fsnotify. So once inode reaches this
+ * state we make sure to block creating any new aliases.
+ */
+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 +470,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 +481,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);
@@ -2237,7 +2250,12 @@ static struct dentry *__d_obtain_alias(struct inode *inode, bool disconnected)
 
 	sb = inode->i_sb;
 
-	res = d_find_any_alias(inode); /* existing alias? */
+	spin_lock(&inode->i_lock);
+	if (!inode_notify_dead(inode))
+		res = __d_find_any_alias(inode);	/* existing alias? */
+	else
+		res = ERR_PTR(-ESTALE);
+	spin_unlock(&inode->i_lock);
 	if (res)
 		goto out;
 
@@ -2249,7 +2267,10 @@ static struct dentry *__d_obtain_alias(struct inode *inode, bool disconnected)
 
 	security_d_instantiate(new, inode);
 	spin_lock(&inode->i_lock);
-	res = __d_find_any_alias(inode); /* recheck under lock */
+	if (!inode_notify_dead(inode))
+		res = __d_find_any_alias(inode);	/* recheck under lock */
+	else
+		res = ERR_PTR(-ESTALE);
 	if (likely(!res)) { /* still no alias, attach a disconnected dentry */
 		unsigned add_flags = d_flags_for_inode(inode);
 
-- 
2.53.0





^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 6.6.y] fs: make sure to call fsnotify_inoderemove() only once when inode is dead
  2026-11-07 10:28 FAILED: patch "[PATCH] fs: make sure to call fsnotify_inoderemove() only once when inode is dead" failed to apply to 6.6-stable tree Sasha Levin
@ 2026-11-10  3:29 ` Sasha Levin
  2026-11-10  8:10   ` Amir Goldstein
  0 siblings, 1 reply; 5+ messages in thread
From: Sasha Levin @ 2026-11-10  3:29 UTC (permalink / raw)
  To: stable
  Cc: Amir Goldstein, Daehyeon Ko, Jan Kara,
	Christian Brauner (Amutable), Sasha Levin

From: Amir Goldstein <amir73il@gmail.com>

[ Upstream commit 669bb5af15d41a21359d00a441490f87a88eb52a ]

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.
Also do not allow open_by_handle() to add new aliases to a "dead" inode.

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.

Reported-and-tested-by: Daehyeon Ko <4ncienth@gmail.com>
Closes: https://lore.kernel.org/linux-fsdevel/20260827045007.3831259-1-4ncienth@gmail.com/
CC: stable@vger.kernel.org
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20260902164924.1333135-2-jack@suse.cz
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
[ sashal: Moved the second inode-dead check into the existing __d_instantiate_anon() helper to accommodate separate alias instantiation. ]
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/dcache.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index e1f4a9e61e589..2b9546b381f7e 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -390,14 +390,16 @@ 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);
 	hlist_del_init(&dentry->d_u.d_alias);
 	raw_write_seqcount_end(&dentry->d_seq);
+	notify_dead = !inode->i_nlink && hlist_empty(&inode->i_dentry);
 	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);
@@ -2097,7 +2099,10 @@ static struct dentry *__d_instantiate_anon(struct dentry *dentry,
 
 	security_d_instantiate(dentry, inode);
 	spin_lock(&inode->i_lock);
-	res = __d_find_any_alias(inode);
+	if (!inode->i_nlink && hlist_empty(&inode->i_dentry))
+		res = ERR_PTR(-ESTALE);
+	else
+		res = __d_find_any_alias(inode);
 	if (res) {
 		spin_unlock(&inode->i_lock);
 		dput(dentry);
@@ -2144,7 +2149,12 @@ static struct dentry *__d_obtain_alias(struct inode *inode, bool disconnected)
 	if (IS_ERR(inode))
 		return ERR_CAST(inode);
 
-	res = d_find_any_alias(inode);
+	spin_lock(&inode->i_lock);
+	if (!inode->i_nlink && hlist_empty(&inode->i_dentry))
+		res = ERR_PTR(-ESTALE);
+	else
+		res = __d_find_any_alias(inode);
+	spin_unlock(&inode->i_lock);
 	if (res)
 		goto out_iput;
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 6.6.y] fs: make sure to call fsnotify_inoderemove() only once when inode is dead
  2026-11-10  3:29 ` [PATCH 6.6.y] fs: make sure to call fsnotify_inoderemove() only once when inode is dead Sasha Levin
@ 2026-11-10  8:10   ` Amir Goldstein
  2026-11-10 15:24     ` Sasha Levin
  0 siblings, 1 reply; 5+ messages in thread
From: Amir Goldstein @ 2026-11-10  8:10 UTC (permalink / raw)
  To: Sasha Levin; +Cc: stable, Daehyeon Ko, Jan Kara, Christian Brauner (Amutable)

On Tue, Nov 10, 2026 at 4:29 AM Sasha Levin <sashal@kernel.org> wrote:
>
> From: Amir Goldstein <amir73il@gmail.com>
>
> [ Upstream commit 669bb5af15d41a21359d00a441490f87a88eb52a ]
>
> 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.
> Also do not allow open_by_handle() to add new aliases to a "dead" inode.
>
> 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.
>
> Reported-and-tested-by: Daehyeon Ko <4ncienth@gmail.com>
> Closes: https://lore.kernel.org/linux-fsdevel/20260827045007.3831259-1-4ncienth@gmail.com/
> CC: stable@vger.kernel.org
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> Signed-off-by: Jan Kara <jack@suse.cz>
> Link: https://patch.msgid.link/20260902164924.1333135-2-jack@suse.cz
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
> [ sashal: Moved the second inode-dead check into the existing __d_instantiate_anon() helper to accommodate separate alias instantiation. ]
> Signed-off-by: Sasha Levin <sashal@kernel.org>

Sasha,

Please drop this patch from all stable kernels or apply only together
with the followup fix.

This commit introduces a regression which was fixed in upstream
shortly after:

commit a522dd871024a0f065bfa7638afdb929607f58fc
Author: Amir Goldstein <amir73il@gmail.com>
Date:   Fri Oct 30 16:55:52 2026 +0100

    fs: don't fail __d_obtain_alias() for unlinked inodes
...
    Christian Brauner <brauner@kernel.org> says:

    Amir's v3 replaced 669bb5af15d4 as a whole. I turned it into an
    incremental fix on top of mainline and wrote a new commit message.
    Jan's Reviewed-by was given on v3 and the resulting code is identical.

    Fixes: 669bb5af15d4 ("fs: make sure to call fsnotify_inoderemove()
only once when inode is dead")
    Cc: stable@vger.kernel.org
    Reported-by: Daehyeon Ko <4ncienth@gmail.com>
    Signed-off-by: Amir Goldstein <amir73il@gmail.com>
    Link: https://patch.msgid.link/CAOQ4uxg7Qt436SiKhKEJKOjtCjeB_XyTjazYF7HMyQAGGcgx2A@mail.gmail.com
    Reviewed-by: Jan Kara <jack@suse.cz>
    Link: https://patch.msgid.link/20261030-work-dcache-obtain_alias-v1-1-a1f7e44536d2@kernel.org
    Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

I don't like the idea of selecting patches to stable which already have fixes
themselves without applying the fixes of fixes together.

Please instruct your bots not to do that (at least for my subsystems)
because the risk outweighs the benefits IMO for the general case.
If bots are able to apply the fix and not able to apply the fixes to fix
along with them this requires maintainer intervention.

Sure obviously often fixes that hit stable will need fixes of their own
and often those fixes of fixes are not known at the time of applying the
fixes to stable.

But this is not the case here, the fixes to fixes followed to mainline very
shortly after the fixes and are available for your bots to detect and examine.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 6.6.y] fs: make sure to call fsnotify_inoderemove() only once when inode is dead
  2026-11-10  8:10   ` Amir Goldstein
@ 2026-11-10 15:24     ` Sasha Levin
  2026-11-10 15:41       ` Amir Goldstein
  0 siblings, 1 reply; 5+ messages in thread
From: Sasha Levin @ 2026-11-10 15:24 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Sasha Levin, stable, Daehyeon Ko, Jan Kara,
	Christian Brauner (Amutable)

On Tue, Nov 10, 2026 at 09:10:17AM +0100, Amir Goldstein wrote:
> Please drop this patch from all stable kernels or apply only together
> with the followup fix.

For 7.3, 6.18 and 6.12 the backport tooling picked a522dd871024 together with
669bb5af15d4. Both were added to those queues in the same commit, a522dd871024
sits after 669bb5af15d4 in each series, and both are in the current 7.3.4-rc1,
6.18.63-rc1 and 6.12.117-rc1.

The 6.6.y backport you replied to was a manual one after the patch failed to
apply. a522dd871024 won't apply to it as-is, so I'm holding the 6.6.y backport
until there is a matching backport of a522dd871024.

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 6.6.y] fs: make sure to call fsnotify_inoderemove() only once when inode is dead
  2026-11-10 15:24     ` Sasha Levin
@ 2026-11-10 15:41       ` Amir Goldstein
  0 siblings, 0 replies; 5+ messages in thread
From: Amir Goldstein @ 2026-11-10 15:41 UTC (permalink / raw)
  To: Sasha Levin; +Cc: stable, Daehyeon Ko, Jan Kara, Christian Brauner (Amutable)

On Tue, Nov 10, 2026 at 4:24 PM Sasha Levin <sashal@kernel.org> wrote:
>
> On Tue, Nov 10, 2026 at 09:10:17AM +0100, Amir Goldstein wrote:
> > Please drop this patch from all stable kernels or apply only together
> > with the followup fix.
>
> For 7.3, 6.18 and 6.12 the backport tooling picked a522dd871024 together with
> 669bb5af15d4. Both were added to those queues in the same commit, a522dd871024
> sits after 669bb5af15d4 in each series, and both are in the current 7.3.4-rc1,
> 6.18.63-rc1 and 6.12.117-rc1.
>
> The 6.6.y backport you replied to was a manual one after the patch failed to
> apply.

OK so the problem was a missing upstream check in the manual apply process,
which is what I was trying to point out.

> a522dd871024 won't apply to it as-is, so I'm holding the 6.6.y backport
> until there is a matching backport of a522dd871024.

FWIW IMO it is not so important to invest in this specific backport.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-10 15:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-07 10:28 FAILED: patch "[PATCH] fs: make sure to call fsnotify_inoderemove() only once when inode is dead" failed to apply to 6.6-stable tree Sasha Levin
2026-11-10  3:29 ` [PATCH 6.6.y] fs: make sure to call fsnotify_inoderemove() only once when inode is dead Sasha Levin
2026-11-10  8:10   ` Amir Goldstein
2026-11-10 15:24     ` Sasha Levin
2026-11-10 15:41       ` Amir Goldstein

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox