[PATCH 0/3] nfs: fix the silly-delete hand-off that 2/9 makes reachable

[Jonathan Flynn <jonathan.flynn@hammerspace.com>]

Hi Neil,

Your "nfs: fix nfs_call_unlink()" computes the silly name's hash before
d_alloc_parallel(), so nfs_call_unlink() can now find another cached
entry for that name and hand it the pending silly-delete.  With it
applied, that happens: an entry made by a fresh lookup of the silly
name, after the original entry was invalidated, receives the delete
when the file is closed.  Three problems in the hand-off become
reachable with it, and these patches fix them:

  1/3  the hand-off can overwrite NFS_FSDATA_BLOCKED on an entry whose
       REMOVE or RENAME is in flight (Sashiko's review of 2/9 reports
       this too);
  2/3  the other order: nfs_rename() tests its target before it takes
       ->d_lock, so a hand-off landing in between is kfree()d by
       block_revalidate(), leaving DCACHE_NFSFS_RENAMED set with no
       data and a NULL dereference at the final dput(); a hand-off
       that the VFS's unlocked test missed also trips the WARN_ON()s
       in nfs_unlink() and nfs_rename().  nfs_rename() now tests under
       ->d_lock, and both return -EBUSY, as may_delete_dentry() does;
  3/3  nfs_call_unlink() holds rmdir_sem for read across its lookup and
       across the dput() of the alias, and either can kill a dentry
       carrying a silly-delete, which re-enters nfs_call_unlink() with
       rmdir_sem still held: that deadlocks against a waiting rmdir()
       (Sashiko's review of 2/9 reports the alias case).  It now takes
       rmdir_sem only for the REMOVE it sends, which also closes an
       older route to the same re-entry that does not need 2/9:
       NFSv4's delegation return in ->unlink_setup().

They have to go wherever 2/9 goes, and its Fixes: tag will take it to
stable.  Fold them into 2/9 if you prefer, as you did with the d_seq fix
in 1/9.

One window is not covered, and you may prefer to close it another way.
After nfs_unlink() or nfs_rename() clears the mark, and before the
VFS's d_delete() or nfs_rename()'s d_move(), a hand-off can still land
on the entry.  For an unlink, that costs only a second REMOVE, which
finds nothing.  For a rename onto the silly name, the target is left
owing a REMOVE of a name that now belongs to the renamed file.  That
REMOVE is normally dropped, because nfs_call_unlink() then finds the
renamed file's own entry and its file handle differs; but if that entry
has left the dcache by the time the target's last reference goes, the
REMOVE is sent and deletes the renamed file.  Closing the rename case
would take the hand-off refusing an alias that is no longer hashed, and
nfs_rename() dropping, under ->d_lock and after d_move(), a
silly-delete that landed after its mark cleared.  I have a draft of
that, and can test and send it if you want it.

Tested with your v4 and these patches on a 7.3-based kernel, over
NFSv4.2 to a pNFS metadata server that is not knfsd.  The hand-off
happens as described above.  With 3/3, the nested nfs_call_unlink()
that follows it starts with rmdir_sem not held, where without 3/3 it
ran with rmdir_sem held for read, and the REMOVE is sent once.  The
removal race tests of my RFC, posted in reply to your 0/9, passed on
this kernel, as did a PROVE_LOCKING and KASAN build running them,
though lockdep does not see rmdir_sem's read hold, which is taken with
down_read_non_owner().  No test here hits the races that 1/3 and 2/3
close, or the deadlock itself, so for those the fixes rest on the code.

The series applies to 83f32c773255 in Linus's tree with your v4, and
fs/ compiles without warnings at each patch.

Tools: these patches, their changelogs and this letter were written
with Claude Code, using Anthropic's Claude Opus 5 and Claude Opus 5.5
models, at my direction.  It found the problems fixed by 1/3 and 2/3,
and two of the three routes in 3/3, reading the code with and without
Sashiko's review prompts; Sashiko's own review of 2/9 reported the
problem in 1/3 as well, and the route through the alias in 3/3.

Thanks,
Jonathan

Jonathan Flynn (3):
  nfs: do not transfer a sillydelete onto a blocked dentry
  nfs: answer a handed-off silly-delete on a removal target with -EBUSY
  nfs: take rmdir_sem for a silly-delete only when its REMOVE is sent

 fs/nfs/dir.c    | 32 ++++++++++++++++++++++++++------
 fs/nfs/unlink.c | 39 ++++++++++++++++++++++++++++++++-------
 2 files changed, 58 insertions(+), 13 deletions(-)


base-commit: 83f32c773255720a661aa45d83e36278135f23f5
prerequisite-patch-id: 3405b4d6ab8b0bf24ba63a898083d617deec4f24
prerequisite-patch-id: 096657d6d80d7ca533ccb574d2af65611ede4af2
prerequisite-patch-id: 1a2319c14739918aef86116647fea28ae85ad4df
prerequisite-patch-id: 725cb18c3936d50141ef27dafc159bf50b64a5bd
prerequisite-patch-id: b39cdd352b97e2cd247311fd5db620dbff227bd7
prerequisite-patch-id: f9f3f0e945cadf96113d4ac9ab8f7b0c207af4e5
prerequisite-patch-id: 4c09a15390bb556d79bea1ee733f52a58bec80ef
prerequisite-patch-id: 7c026717a97b2c86a169539822f5d14650f68f8a
prerequisite-patch-id: a702d54495d0f3d7f88b82411d884dcf5aacf9a0
-- 
2.43.0