All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mateusz Guzik <mjguzik@gmail.com>
To: brauner@kernel.org
Cc: viro@zeniv.linux.org.uk, jack@suse.cz,
	linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	Mateusz Guzik <mjguzik@gmail.com>
Subject: [PATCH v3 5/7] fs: handle hypothetical filesystems which use I_DONTCACHE and drop the lock in ->drop_inode
Date: Sun, 29 Mar 2026 19:20:00 +0200	[thread overview]
Message-ID: <20260329172002.3557801-6-mjguzik@gmail.com> (raw)
In-Reply-To: <20260329172002.3557801-1-mjguzik@gmail.com>

f2fs and ntfs play games where they transitiong the refcount 0->1 and release
the inode spinlock, allowing other threads to grab a ref of their own.
They also return 0 in that case, making this problem harmless.

Should they start using the I_DONTCACHE machinery down the road while
retaining the above, iput_final() will get a race where it can proceed
to teardown an inode with references.

Future-proof it.

Developing better ->drop_inode and sanitizing all users is left as en
exercise for the reader.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
---
 fs/inode.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index 0050eca6d83f..a417abc64822 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -1935,20 +1935,29 @@ static void iput_final(struct inode *inode)
 	else
 		drop = inode_generic_drop(inode);
 
-	if (!drop &&
-	    !(inode_state_read(inode) & I_DONTCACHE) &&
-	    (sb->s_flags & SB_ACTIVE)) {
+	/*
+	 * XXXCRAP: there are ->drop_inode hooks playing nasty games releasing the
+	 * spinlock and temporarily grabbing refs. This opens a possibility someone
+	 * else will sneak in and grab a ref while it happens.
+	 *
+	 * If such a hook returns 0 (== don't drop) this happens to be harmless as long
+	 * as the inode is not marked with I_DONTCACHE. Otherwise we are proceeding with
+	 * teardown despite references being present.
+	 *
+	 * Damage-control the problem by including the count in the decision. However,
+	 * assert no refs showed up if the hook decided to drop the inode.
+	 */
+	if (drop)
+		VFS_BUG_ON_INODE(icount_read(inode) != 0, inode);
+
+	if (icount_read(inode) > 0 ||
+	    (!drop && !(inode_state_read(inode) & I_DONTCACHE) &&
+	    (sb->s_flags & SB_ACTIVE))) {
 		__inode_lru_list_add(inode, true);
 		spin_unlock(&inode->i_lock);
 		return;
 	}
 
-	/*
-	 * Re-check ->i_count in case the ->drop_inode() hooks played games.
-	 * Note we only execute this if the verdict was to drop the inode.
-	 */
-	VFS_BUG_ON_INODE(icount_read(inode) != 0, inode);
-
 	if (drop) {
 		inode_state_set(inode, I_FREEING);
 	} else {
-- 
2.48.1


  parent reply	other threads:[~2026-03-29 17:20 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-29 17:19 [PATCH v3 0/7] assorted ->i_count changes + extension of lockless handling Mateusz Guzik
2026-03-29 17:19 ` [PATCH v3 1/7] fs: add icount_read_once() Mateusz Guzik
2026-03-29 17:19 ` [PATCH v3 2/7] Use icount_read() and icount_read_once() as appropriate Mateusz Guzik
2026-03-29 17:19 ` [PATCH v3 3/7] fs: enforce locking in icount_read(), add some commentary Mateusz Guzik
2026-03-29 17:19 ` [PATCH v3 4/7] fs: relocate and tidy up ihold() Mateusz Guzik
2026-03-29 17:20 ` Mateusz Guzik [this message]
2026-03-31 11:23   ` [PATCH v3 5/7] fs: handle hypothetical filesystems which use I_DONTCACHE and drop the lock in ->drop_inode Christian Brauner
2026-03-31 11:37     ` Mateusz Guzik
2026-03-29 17:20 ` [PATCH v3 6/7] fs: locklessly bump refs in igrab as long as it does not transition 0->1 Mateusz Guzik
2026-03-29 17:20 ` [PATCH v3 7/7] fs: locklessly bump refs in the inode hash when possible Mateusz Guzik
2026-03-30  7:38 ` [syzbot ci] Re: assorted ->i_count changes + extension of lockless handling syzbot ci
2026-03-31 11:31 ` [PATCH v3 0/7] " Christian Brauner
2026-03-31 11:36   ` Mateusz Guzik

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=20260329172002.3557801-6-mjguzik@gmail.com \
    --to=mjguzik@gmail.com \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --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 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.