Re: [PATCH] 9p: fix WARN_ON when dropping nlink on files with nlink=0

[Dominique Martinet <asmadeus@codewreck.org>]

Breno Leitao wrote on Mon, Jan 26, 2026 at 02:23:37AM -0800:
> v9fs_dec_count() guards against decrementing nlink on directories that
> have nlink <= 2, but does not guard against decrementing nlink on
> regular files that already have nlink == 0.
> 
> In the 9p filesystem, the client caches inode metadata including nlink,
> but the server is the source of truth. During an unlink operation, the
> following race can occur:
>
>   1. Client initiates unlink, server processes it and sets nlink to 0
>   2. Client fetches updated inode metadata (nlink=0) before unlink returns
>   3. Client's v9fs_remove() completes successfully
>   4. Client calls v9fs_dec_count() which calls drop_nlink() on nlink=0

Thanks for the patch and sorry for the delay


hmm.. I'm not actually sure if we should call drop_nlink() at all in
cacheless mode, actually..
We don't really care about nlink in this context, as inode should be
gone immediately anyway, or does nlink hitting zero imply something
else?

So we could get the v9fs_session_info out of the inode
(v9fs_inode2v9ses) and just return if CACHE_META is set?

Others opinion would be great, but I get the feeling that just checking
before update only makes the race smaller, and not totally fixed.


> This race is easily triggered under heavy unlink workloads, such as
> stress-ng's unlink stressor, producing the following warning:
> 
>   WARNING: fs/inode.c:417 at drop_nlink+0x4c/0xc8
>   Call trace:
>    drop_nlink+0x4c/0xc8
>    v9fs_remove+0x1e0/0x250 [9p]
>    v9fs_vfs_unlink+0x20/0x38 [9p]
>    vfs_unlink+0x13c/0x258
>    ...
> 
> Fix this by returning early from v9fs_dec_count() if the inode's nlink
> is already zero, extending the protection that commit ac89b2ef9b55
> ("9p: don't maintain dir i_nlink if the exported fs doesn't either")
> added for directories to also cover regular files.
> 
> Fixes: ac89b2ef9b55 ("9p: don't maintain dir i_nlink if the exported fs doesn't either")
> Cc: stable@vger.kernel.org
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  fs/9p/vfs_inode.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
> index 97abe65bf7c1f..b75f656af4c98 100644
> --- a/fs/9p/vfs_inode.c
> +++ b/fs/9p/vfs_inode.c
> @@ -488,10 +488,16 @@ static int v9fs_at_to_dotl_flags(int flags)
>   * - ext4 (with dir_nlink feature enabled) sets nlink to 1 if a dir has more
>   *   than EXT4_LINK_MAX (65000) links.
>   *
> + * For regular files, the server may have already decremented nlink to 0
> + * before the client's unlink completes, so we must also guard against
> + * decrementing an already-zero nlink.
> + *
>   * @inode: inode whose nlink is being dropped
>   */
>  static void v9fs_dec_count(struct inode *inode)
>  {
> +	if (!inode->i_nlink)
> +		return;
>  	if (!S_ISDIR(inode->i_mode) || inode->i_nlink > 2)
>  		drop_nlink(inode);
>  }
> 
> ---
> base-commit: ca3a02fda4da8e2c1cb6baee5d72352e9e2cfaea
> change-id: 20260126-9p-50d206e2f6f6
> 
> Best regards,

-- 
Dominique Martinet | Asmadeus