All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dominique Martinet <asmadeus@codewreck.org>
To: David Howells <dhowells@redhat.com>,
	Christian Schoenebeck <linux_oss@crudebyte.com>
Cc: Eric Van Hensbergen <ericvh@kernel.org>,
	Latchesar Ionkov <lucho@ionkov.net>,
	Chris Arges <carges@cloudflare.com>,
	v9fs@lists.linux.dev, linux-kernel@vger.kernel.org,
	Matthew Wilcox <willy@infradead.org>,
	linux-fsdevel@vger.kernel.org
Subject: Re: 9p read corruption of mmaped content (Was: [PATCH] 9p/virtio: restrict page pinning to user_backed_iter() iovec)
Date: Fri, 19 Dec 2025 22:46:11 +0900	[thread overview]
Message-ID: <aUVXI5wt9jm9x66E@codewreck.org> (raw)
In-Reply-To: <aUU8thEsa0X4YrlF@codewreck.org>

David pointing this out on IRC (thanks!)
> you may have actually caught the bug in your trace
>    kworker/u16:2-233     [002] .....  3031.183459: netfs_folio: i=157f3 ix=00005-00005 read-unlock
> ...
>            clang-318     [002] .....  3031.183462: netfs_sreq: R=00001b55[2] ZERO SUBMT f=000 
> s=5fb2 0/4e s=0 e=0
> we shouldn't have unlocked page 5 yet
> we still have to clear the tail of the page

And indeed, if I update the read_collect code to not unlock as soon as
we've read i_size but wait for the tail to be cleared as follow, I can't
reproduce anymore:
---------
diff --git a/fs/netfs/read_collect.c b/fs/netfs/read_collect.c
index a95e7aadafd0..7a0ffa675fb1 100644
--- a/fs/netfs/read_collect.c
+++ b/fs/netfs/read_collect.c
@@ -137,7 +137,7 @@ static void netfs_read_unlock_folios(struct netfs_io_request *rreq,
 		rreq->front_folio_order = order;
 		fsize = PAGE_SIZE << order;
 		fpos = folio_pos(folio);
-		fend = umin(fpos + fsize, rreq->i_size);
+		fend = fpos + fsize;
 
 		trace_netfs_collect_folio(rreq, folio, fend, collected_to);
--------- 

It's too late for me to think of the implications and consider this
diff's correctness, but with that patch the kernel is happily building
with LLVM=1, so I'm fairly confident this particular bug goes away...

(Now that umin() dates back v6.14-rc1 (commit e2d46f2ec332 ("netfs:
Change the read result collector to only use one work item")), I guess
I'll want to check if it's been broken this long next, or if it's a side
effect of another change... Tomorrow...)

I'm off to bed, feel free to send a patch with the above if you think
it's correct
-- 
Dominique Martinet | Asmadeus

  reply	other threads:[~2025-12-19 13:46 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-09 21:04 [PATCH] 9p/virtio: restrict page pinning to user_backed_iter() iovec Dominique Martinet
2025-12-09 21:04 ` Dominique Martinet via B4 Relay
2025-12-10  4:21 ` Matthew Wilcox
2025-12-10  6:04 ` Christoph Hellwig
2025-12-10  7:38   ` asmadeus
2025-12-10  8:32     ` Christoph Hellwig
2025-12-13 13:28       ` asmadeus
2025-12-15  5:55         ` Christoph Hellwig
2025-12-15  7:34           ` Dominique Martinet
2025-12-15 11:16             ` Christian Schoenebeck
2025-12-15 14:37             ` Christoph Hellwig
2025-12-19 12:03             ` David Howells
2025-12-19 12:00           ` David Howells
2025-12-19 11:26         ` David Howells
2025-12-10 13:33   ` Christian Schoenebeck
2025-12-17 13:41 ` Christian Schoenebeck
2025-12-17 21:49   ` asmadeus
2025-12-18 14:21     ` asmadeus
2025-12-18 15:14       ` Christian Schoenebeck
2025-12-18 23:14         ` asmadeus
2025-12-19  4:09           ` 9p read corruption of mmaped content (Was: [PATCH] 9p/virtio: restrict page pinning to user_backed_iter() iovec) Dominique Martinet
2025-12-19 11:53             ` Dominique Martinet
2025-12-19 13:46               ` Dominique Martinet [this message]
2025-12-19 14:01             ` David Howells
2025-12-19 12:06   ` [PATCH] 9p/virtio: restrict page pinning to user_backed_iter() iovec David Howells

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aUVXI5wt9jm9x66E@codewreck.org \
    --to=asmadeus@codewreck.org \
    --cc=carges@cloudflare.com \
    --cc=dhowells@redhat.com \
    --cc=ericvh@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux_oss@crudebyte.com \
    --cc=lucho@ionkov.net \
    --cc=v9fs@lists.linux.dev \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.