linux-kernel - [PATCH] fs/9p: don't use cached metadata in revalidate for cache=mmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20251022-mmap-regression-v1-1-980365ee524e@codewreck.org>
Date: Wed, 22 Oct 2025 07:09:57 +0900
From: Dominique Martinet via B4 Relay <devnull+asmadeus.codewreck.org@...nel.org>
To: Song Liu <song@...nel.org>, Andrii Nakryiko <andrii.nakryiko@...il.com>, 
 Eric Van Hensbergen <ericvh@...nel.org>, 
 Latchesar Ionkov <lucho@...kov.net>, 
 Christian Schoenebeck <linux_oss@...debyte.com>, 
 Tingmao Wang <m@...wtm.org>, Alexei Starovoitov <ast@...nel.org>
Cc: linux-kernel@...r.kernel.org, v9fs@...ts.linux.dev, bpf@...r.kernel.org, 
 Dominique Martinet <asmadeus@...ewreck.org>
Subject: [PATCH] fs/9p: don't use cached metadata in revalidate for
 cache=mmap

From: Dominique Martinet <asmadeus@...ewreck.org>

cache=mmap is a mode that doesn't cache metadata, but still has
writeback cache.
In commit 290434474c33 ("fs/9p: Refresh metadata in d_revalidate
for uncached mode too") we considered metadata cache to be enough to
not look at the server, but in writeback cache too looking at the server
size would make the vfs consider the file has been truncated before the
data has been flushed out, making the following repro fail (nothing is
ever read back, the resulting file ends up with no data written)

---
I was suspecting cache=mmap when I saw vmtest used that, and it's
confirmed with Song's reproducer...
This makes the repro work, would one of you be able to confirm this?
Once confirmed I'll send this to Linus directly

Thanks again and sorry for lack of cache=mmap tests :/

Signed-off-by: Dominique Martinet <asmadeus@...ewreck.org>
---
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

char buf[4096];

int main(int argc, char *argv[])
{
        int ret, i;
        int fdw, fdr;

        if (argc < 2)
                return 1;

        fdw = openat(AT_FDCWD, argv[1], O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC, 0600);
        if (fdw < 0) {
                fprintf(stderr, "cannot open fdw\n");
                return 1;
        }
        write(fdw, buf, sizeof(buf));

        fdr = openat(AT_FDCWD, argv[1], O_RDONLY|O_CLOEXEC);

        if (fdr < 0) {
                fprintf(stderr, "cannot open fdr\n");
                close(fdw);
                return 1;
        }

        for (i = 0; i < 10; i++) {
                ret = read(fdr, buf, sizeof(buf));
                fprintf(stderr, "i: %d, read returns %d\n", i, ret);
        }

        close(fdr);
        close(fdw);
        return 0;
}
---

Reported-by: Song Liu <song@...nel.org>
Link: https://lkml.kernel.org/r/CAHzjS_u_SYdt5=2gYO_dxzMKXzGMt-TfdE_ueowg-Hq5tRCAiw@mail.gmail.com
Reported-by: Andrii Nakryiko <andrii.nakryiko@...il.com>
Link: https://lore.kernel.org/bpf/CAEf4BzZbCE4tLoDZyUf_aASpgAGFj75QMfSXX4a4dLYixnOiLg@mail.gmail.com/
Fixes: 290434474c33 ("fs/9p: Refresh metadata in d_revalidate for uncached mode too")
Signed-off-by: Dominique Martinet <asmadeus@...ewreck.org>
---
 fs/9p/vfs_dentry.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/9p/vfs_dentry.c b/fs/9p/vfs_dentry.c
index f3248a3e54023489054337bf98e87a1d7b1fbafc..2f3654bf6275ffa802dc25b9a84bd61a62d5723c 100644
--- a/fs/9p/vfs_dentry.c
+++ b/fs/9p/vfs_dentry.c
@@ -78,7 +78,11 @@ static int __v9fs_lookup_revalidate(struct dentry *dentry, unsigned int flags)
 	v9inode = V9FS_I(inode);
 	struct v9fs_session_info *v9ses = v9fs_inode2v9ses(inode);
 
-	cached = v9ses->cache & (CACHE_META | CACHE_LOOSE);
+	/* We also don't want to refresh attr here in writeback cache mode,
+	 * otherwise a write that hasn't been propagated to server would
+	 * incorrectly get the old size back and truncate the file before
+	 * the write happens */
+	cached = v9ses->cache & (CACHE_META | CACHE_WRITEBACK | CACHE_LOOSE);
 
 	if (!cached || v9inode->cache_validity & V9FS_INO_INVALID_ATTR) {
 		int retval;

---
base-commit: 211ddde0823f1442e4ad052a2f30f050145ccada
change-id: 20251022-mmap-regression-f0f40d51046d

Best regards,
-- 
Dominique Martinet <asmadeus@...ewreck.org>