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

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

[Breno Leitao]

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

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,
--  
Breno Leitao <leitao@debian.org>

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

[Dominique Martinet]

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

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

[Breno Leitao]

hello Dominique,

On Sun, Feb 15, 2026 at 06:27:45PM +0900, Dominique Martinet wrote:
> Thanks for the patch and sorry for the delay

I had this fix in my tree, and I had forgot about. Now that I moved to
the 7.1, I started seeing it again, so, let's revamp this.

> 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?

Agreed -- in cacheless mode the server is authoritative and the inode is
on its way out, so adjusting i_nlink locally buys us nothing. I don't
see anything else keying off nlink hitting zero in that path.

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

What about sometehing like:

commit 369beed134ff0b7ce1cf68d0c46b08ec07d625db
Author: Breno Leitao <leitao@debian.org>
Date:   Mon Jan 26 02:23:37 2026 -0800

    9p: skip nlink update in cacheless mode to fix WARN_ON
    
    v9fs_dec_count() unconditionally calls drop_nlink() on regular files,
    even when the inode's nlink is already zero. In cacheless mode the
    client refetches inode metadata from the server (the source of truth)
    on every operation, so by the time v9fs_remove() returns, the locally
    cached nlink may already reflect the post-unlink value:
    
      1. Client initiates unlink, server processes it and sets nlink to 0
      2. Client refetches 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
    
    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
       ...
    
    In cacheless mode the server is authoritative and the inode is on its
    way out, so locally adjusting nlink buys nothing. Skip v9fs_dec_count()
    entirely when neither CACHE_META nor CACHE_LOOSE is set, which both
    avoids the warning and removes a class of nlink races (two concurrent
    unlinkers observing nlink > 0 and both calling drop_nlink()) that an
    nlink == 0 guard alone would only narrow rather than close.
    
    Fixes: ac89b2ef9b55 ("9p: don't maintain dir i_nlink if the exported fs doesn't either")
    Cc: stable@vger.kernel.org
    Suggested-by: Dominique Martinet <asmadeus@codewreck.org>
    Signed-off-by: Breno Leitao <leitao@debian.org>

diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index d1508b1fe1092..50cf837979d9c 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -488,10 +488,19 @@ 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.
  *
+ * In cacheless mode the server is the source of truth for nlink and the
+ * inode is going away immediately, so locally adjusting i_nlink buys
+ * nothing and races with concurrent metadata fetches that may already
+ * have observed the post-unlink value (nlink == 0).
+ *
  * @inode: inode whose nlink is being dropped
  */
 static void v9fs_dec_count(struct inode *inode)
 {
+	struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode);
+
+	if (!(v9ses->cache & (CACHE_META | CACHE_LOOSE)))
+		return;
 	if (!S_ISDIR(inode->i_mode) || inode->i_nlink > 2)
 		drop_nlink(inode);
 }

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

[Dominique Martinet]

Breno Leitao wrote on Mon, Apr 20, 2026 at 07:31:14AM -0700:
>     In cacheless mode the server is authoritative and the inode is on its
>     way out, so locally adjusting nlink buys nothing. Skip v9fs_dec_count()
>     entirely when neither CACHE_META nor CACHE_LOOSE is set, which both
>     avoids the warning and removes a class of nlink races (two concurrent
>     unlinkers observing nlink > 0 and both calling drop_nlink()) that an
>     nlink == 0 guard alone would only narrow rather than close.

I need to check this doesn't actually leak memory or something but this
sounds better to me, thanks.

Please send as a proper PATCH mail and I'll tentatively apply for 7.2
(a bit too late for 7.1)

-- 
Dominique Martinet | Asmadeus

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

[Breno Leitao]

On Tue, Apr 21, 2026 at 12:59:51AM +0900, Dominique Martinet wrote:
> Breno Leitao wrote on Mon, Apr 20, 2026 at 07:31:14AM -0700:
> >     In cacheless mode the server is authoritative and the inode is on its
> >     way out, so locally adjusting nlink buys nothing. Skip v9fs_dec_count()
> >     entirely when neither CACHE_META nor CACHE_LOOSE is set, which both
> >     avoids the warning and removes a class of nlink races (two concurrent
> >     unlinkers observing nlink > 0 and both calling drop_nlink()) that an
> >     nlink == 0 guard alone would only narrow rather than close.
> 
> I need to check this doesn't actually leak memory or something but this
> sounds better to me, thanks.
> 
> Please send as a proper PATCH mail and I'll tentatively apply for 7.2
> (a bit too late for 7.1)

Ack, 7.2 is more than fine.

Thanks for your help,
--breno