Re: 9p read corruption of mmaped content (Was: [PATCH] 9p/virtio: restrict page pinning to user_backed_iter() iovec)

[Dominique Martinet]

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