From: Al Viro <viro@zeniv.linux.org.uk>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [GIT PULL] fuse update for 6.19
Date: Sun, 12 Apr 2026 07:16:44 +0100 [thread overview]
Message-ID: <20260412061644.GA2485189@ZenIV> (raw)
In-Reply-To: <CAJfpegtsDZW7v=YDhs+gPq25G+QOh-5mjiaXT04DDW=iU+R75A@mail.gmail.com>
On Wed, Jan 14, 2026 at 04:23:04PM +0100, Miklos Szeredi wrote:
> Posted a short patchset fixing this.
>
> I really think there's no point in doing the get-ref, drop-ref dance.
> Retrieving the dentry from any source will require locking and that
> needs to nest d_lock with or without the refcount manipulation.
>
> So I kept the d_dispose_if_unused() API, but added a note to the
> function doc that additional locking is necessary to prevent eviction.
Looking at that thing again, I really hate how subtle it is ;-/
Consider the lockless case in your ->d_release() and try to write down
the reasons why it's safe. Among the other things, kfree_rcu() is there
to protect RCU callers of ->d_revalidate(); fair enough, but it quietly
doubles as delaying that freeing past the scope of dentry_hash[].lock in
which you'd done RB_CLEAR_NODE(&fd->node) that had pushed ->d_release()
to lockless path. Another side of that fun is the proof that if
fuse_dentry_tree_work() sees a dentry, it won't get freed under us.
Locked case of ->d_release() is easy; proving that the lockless one
is OK without explict barriers is more interesting. Basically, all
insertions are ordered wrt ->d_release() (on ->d_lock), so if the value
we are observing in RB_EMPTY_NODE() has come from those we will hit the
locked case. If the value we observe has come from RB_CLEAR_NODE() in
earlier fuse_dentry_tree_work() (these are ordered on dentry_hash[].lock,
wrt each other and insertions), there mustn't have been any subsequent
insertions, or ->d_release() would've observed the effect of those.
If the value has come from the _same_ fuse_dentry_tree_work(), the
implicit barrier in spin_lock() would've ordered that store wrt beginning
of the scope, and since dentry_free() is ordered wrt ->d_release()
the callback of call_rcu() in there wouldn't run until the the end of
the scope in question. So I think it's OK, but having it done without
a single comment either on barriers or on memory safety... Ouch.
Note that we *can* run into a dentry getting killed just after
RB_CLEAR_NODE() in there; it's just that store to ->d_flags of dying
or killed dentry is safe under ->d_lock and d_dispose_if_unused() is
a no-op for dying and killed ones - it's not just "If dentry has no
external references, move it to shrink list" as your comment in
fs/dcache.c says. And in your case it very much does have an
external reference - it's just that it's an equivalent of RCU one,
with all the joy that inflicts upon the user.
FWIW, right now I'm still looking for the source of the UAF
reported by jlayton; I think this stuff in fuse can be excluded as
a possible cause, but I'm not happy with that primitive at all... ;-/
next prev parent reply other threads:[~2026-04-12 6:12 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-04 8:25 [GIT PULL] fuse update for 6.19 Miklos Szeredi
2025-12-05 23:47 ` Linus Torvalds
2025-12-06 1:42 ` Al Viro
2025-12-06 1:52 ` Linus Torvalds
2025-12-06 2:28 ` Al Viro
2025-12-06 3:10 ` Al Viro
2025-12-06 3:29 ` Linus Torvalds
2025-12-06 3:54 ` Al Viro
2025-12-06 4:22 ` Al Viro
2025-12-08 10:37 ` Miklos Szeredi
2026-01-14 15:23 ` Miklos Szeredi
2026-04-12 6:16 ` Al Viro [this message]
2026-04-13 0:29 ` Al Viro
2026-04-13 15:32 ` Miklos Szeredi
2025-12-06 0:17 ` pr-tracker-bot
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=20260412061644.GA2485189@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=miklos@szeredi.hu \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox