Re: PROBLEM: [REGRESSION 7.1-rc4 -> 7.1-rc5] 9p: silent NUL corruption of file data on cache modes with CACHE_WRITEBACK

[Christian Schoenebeck <linux_oss@crudebyte.com>]

On Thursday, 10 September 2026 12:56:24 CEST Michael Mulqueen wrote:
> Hi,
> 
> This is my first kernel bug report, so let's hope I've done it right!
> 
> [1.] One line summary of the problem:
> 
> A 9p mount returns NULs in place of file contents the server holds, and can
> write those NULs back over the server's copy. Nothing reports an error - the
> read succeeds and the file is the correct length. Affects CACHE_WRITEBACK
> modes - mmap, loose and fscache - but not none or readahead.
> 
> [2.] Full description of the problem/report:
> 
> I enclose a reproducer script that shows the two presentations of this bug:
> 
>   - Read side: a range whose folio has been evicted reads back as NULs
>     while the server's copy is intact. The whole file comes back as NULs.
>     A fresh open() on the same mount reads them too, so it is not
>     confined to one descriptor.
> 
>   - Write side: A partial write into such a range destroys data the server
>     did hold.
> 
> I came across this bug on a cache=loose mount in ordinary use, I was seeing
> this sporadically and I think git was usually the trigger. I switched to
> cache=mmap, which made it rarer but did not stop it.
> 
> After the client's own writeback extends a file, zero_point appears to go
> stale and the zero-fill branch in fs/netfs/buffered_write.c fires for
> regions the server does have. The sites were that branch, fs/9p/vfs_inode.c
> (use_zero_point = true to netfs_inode_init), and fs/netfs/misc.c.
[...]
> I then built and tested the likeliest commit and its parent:
> 
>   - 2c8f4742bb76 ("netfs: Fix potential for tearing in ->remote_i_size and
>     ->zero_point") - clean.
> 
>   - 4543a4d73794 ("netfs: Fix zeropoint update where i_size >
>     remote_i_size") - CORRUPT.

So that was:

commit 4543a4d737944134a1394afe797622546fbcc98a
Author: David Howells <dhowells@redhat.com>
Date:   Tue May 12 13:33:43 2026 +0100

    netfs: Fix zeropoint update where i_size > remote_i_size
    
    Fix the update of the zero point[*] by netfs_release_folio() when there is
    uncommitted data in the pagecache beyond the folio being released but the
    on-server EOF is in this folio (ie. i_size > remote_i_size).  The update
    needs to limit zero_point to remote_i_size, not i_size as i_size is a local
    phenomenon reflecting updates made locally to the pagecache, not stuff
    written to the server.  remote_i_size tracks the server's i_size.

[...]

diff --git a/fs/netfs/misc.c b/fs/netfs/misc.c
index bad661ff2bec..723571ca1b88 100644
--- a/fs/netfs/misc.c
+++ b/fs/netfs/misc.c
@@ -307,10 +307,10 @@ bool netfs_release_folio(struct folio *folio, gfp_t gfp)
                return false;
 
        netfs_read_sizes(inode, &i_size, &remote_i_size, &zero_point);
-       end = umin(folio_next_pos(folio), i_size);
+       end = folio_next_pos(folio);
        if (end > zero_point) {
                spin_lock(&inode->i_lock);
-               end = umin(folio_next_pos(folio), inode->i_size);
+               end = umin(end, ctx->_remote_i_size);
                if (end > ctx->_zero_point)
                        netfs_write_zero_point(inode, end);
                spin_unlock(&inode->i_lock);

That would explain it, as _remote_i_size is only updated on explicit stat()
(Tgetattr 9p request); with cache modes "loose" and "fscache" not even then.

That's actually David's domain, but maybe something like the following might
fix it?

diff --git a/fs/netfs/write_collect.c b/fs/netfs/write_collect.c
index 210eb8f3958d..9585274b118a 100644
--- a/fs/netfs/write_collect.c
+++ b/fs/netfs/write_collect.c
@@ -73,6 +73,8 @@ int netfs_folio_written_back(struct folio *folio)
                spin_lock(&ictx->inode.i_lock);
                if (fend > ictx->_zero_point)
                        netfs_write_zero_point(inode, fend);
+               if (fend > ictx->_remote_i_size)
+                       netfs_write_remote_i_size(inode, fend);
                spin_unlock(&ictx->inode.i_lock);
 
                folio_detach_private(folio);
@@ -90,6 +92,15 @@ int netfs_folio_written_back(struct folio *folio)
                        goto end_wb;
                }
 
+               {
+                       unsigned long long wend = umin(folio_next_pos(folio),
+                                                      i_size_read(inode));
+                       spin_lock(&ictx->inode.i_lock);
+                       if (wend > ictx->_remote_i_size)
+                               netfs_write_remote_i_size(inode, wend);
+                       spin_unlock(&ictx->inode.i_lock);
+               }
+
                /* Need to detach the group pointer if the page didn't get
                 * redirtied.  If it has been redirtied, then it must be within
                 * the same group.

I.e. updating _remote_i_size on client's self-inflicted size extension after
write completed?

> I searched the v9fs and netfs public-inbox archives and did not find this
> reported - apologies if I have missed a thread.
> 
> The nearest existing thread is Pierre Barre's "[BUG] 9p: data corruption
> with cache=mmap under concurrent stat/write" (24 Dec 2025). I believe that
> is a different bug: it needs stat racing against writes and manifests as a
> wrong i_size. What I have here is single-threaded, deterministic, and
> destroys data rather than mis-sizing the file. It is also absent on v6.18,
> the release that report was made against.

Even though unrelated, that's not fixed yet either, is it?

/Christian