Re: [PATCH] NFS: ensure nfs_safe_remove() atomic nlink drop

[Trond Myklebust <trondmy@kernel.org>]

On Mon, 2025-11-17 at 13:03 -0500, Aiden Lambert wrote:
> A race condition occurs when both unlink() and link() are running
> concurrently on the same inode, and the nlink count from the nfs
> server
> received in link()->nfs_do_access() clobbers the nlink count of the
> inode in nfs_safe_remove() after the "remove" RPC is made to the
> server
> but before we decrement the link count. If the nlink value from
> nfs_do_access() reflects the decremented nlink of the "remove" RPC, a
> double decrement occurs, which can lead to the dropping of the client
> side inode, causing the link call to return ENOENT. To fix this, we
> record an expected nlink value before the "remove" RPC and compare it
> with the value afterwards---if these two are the same, the drop is
> performed. Note that this does not take into account nlink values
> that
> are a result of multi-client (un)link operations as these are not
> guaranteed to be atomic by the NFS spec.


Why do we end up running nfs_do_access() at all in the above test? That
sounds like a bug. We shouldn't ever need to validate if we can create
or delete things using ACCESS. That just ends up producing an
unnecessary TOCTOU race.


-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trondmy@kernel.org, trond.myklebust@hammerspace.com