vfs: fix take_dentry_name_snapshot() semantics

The take_dentry_name_snapshot() function would keep trying to take a
reference on a dentry name forever, even if that dentry had been
released and the dentry name would never become valid.

All normal users held a reference to the dentry, but this "dentry name
is dead" could hit the overlayfs RCU walk case, and Amir fixed it by
adding a new try_take_dentry_name_snapshot() that returns on failure.
See commit 15c6bd5cf109 ("vfs: add try_take_dentry_name_snapshot()").

However, that fix was uncomfortably ugly, and the new "try" version was
subtly buggy in that it would also fail spuriously during a rename. The
only current user of that doesn't really care: a spurious failure would
result in the lookup just exiting RCU mode and being retried with the
dentry ref held (using the non-try version), but this all just rubbed me
the wrong way.

Instead of introducing a new and differently buggy helper, this just
makes take_dentry_name_snapshot() deal with all these situations
correctly.

It does that by instead of retrying when the external name reference
cannot be taken, it now retries purely based on the dentry sequence
counter. That means that if the external name reference failed due to a
rename, we will correctly retry it, but if it fails because the dentry
is dead and the name really is gone, it will now return that failure
rather than looping forever.

In this model, take_dentry_name_snapshot() will never return a failure
when a dentry ref is held, so the existing non-overlayfs users don't
need to care and they can safely ignore the return value.

And users that don't hold a reference (ie currently only that overlayfs
case) can use the improved take_dentry_name_snapshot() function safely
and it won't loop forever when racing with the dentry being free'd.

To make this all really robust and easy to use for any possible new use
cases, the failure case will not just return false, but also make sure
that the returned name will be empty, and that you can still call
release_dentry_name_snapshot() on the name.

But since there won't actually be anything to release on failure, you
can also do what overlayfs does: return an error and *not* call the
release function.

IOW, instead of two different functions that each had their own problem
cases, we're now back to just one single function that is robust and
easy to use in all situations. Knock wood.

Link: https://lore.kernel.org/all/CAHk-=wiPhTLn1p0ZHx-7SMdk4_zazqDK1k7MMEn9scRfqeDgtw@mail.gmail.com/
Fixes: 15c6bd5cf109 ("vfs: add try_take_dentry_name_snapshot()")
Cc: Amir Goldstein <amir73il@gmail.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

0 files changed, 0 insertions, 0 deletions