[PATCH] 9p/trans_virtio: bound RERROR copy by mapped pages

[PATCH] 9p/trans_virtio: bound RERROR copy by mapped pages

[Yizhou Zhao]

handle_rerror() copies the variable-length error string of a zero-copy
RERROR response from the receive pages into the request's static response
buffer.  The amount copied is bounded by P9_ZC_HDR_SZ, so the data can
span at most two pages, but the helper is not told how many receive pages
were actually mapped.

If a malicious or broken virtio 9p device reports an RERROR length that
exceeds the remaining bytes in the first mapped receive page, the error
string is treated as crossing into a second page.  When only one receive
page was mapped, handle_rerror() still advances the page pointer and
dereferences the next entry, reading past the allocated in_pages array.

Pass the number of mapped receive pages to handle_rerror().  If the error
string would cross a page boundary but only one page is available, copy the
bytes that fit in that page and leave the response truncated, matching the
existing behavior for overlong RERROR messages.  Otherwise continue with
the second-page copy as before.

Fixes: f615625a44c4 ("9p: handling Rerror without copy_from_iter_full()")
Cc: stable@vger.kernel.org
Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
Reported-by: Ao Wang <wangao@seu.edu.cn>
Reported-by: Xuewei Feng <fengxw06@126.com>
Reported-by: Qi Li <qli01@tsinghua.edu.cn>
Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
Assisted-by: GLM:GLM-5.1
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
---
 net/9p/trans_virtio.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/9p/trans_virtio.c b/net/9p/trans_virtio.c
index 4cdab7094b27..a71ce8870c53 100644
--- a/net/9p/trans_virtio.c
+++ b/net/9p/trans_virtio.c
@@ -377,7 +377,7 @@ static int p9_get_mapped_pages(struct virtio_chan *chan,
 }
 
 static void handle_rerror(struct p9_req_t *req, int in_hdr_len,
-			  size_t offs, struct page **pages)
+			  size_t offs, struct page **pages, int in_nr_pages)
 {
 	unsigned size, n;
 	void *to = req->rc.sdata + in_hdr_len;
@@ -398,6 +398,8 @@ static void handle_rerror(struct p9_req_t *req, int in_hdr_len,
 	n = PAGE_SIZE - offs;
 	if (size > n) {
 		memcpy_from_page(to, *pages++, offs, n);
+		if (in_nr_pages < 2)
+			return;
 		offs = 0;
 		to += n;
 		size -= n;
@@ -535,7 +537,7 @@ p9_virtio_zc_request(struct p9_client *client, struct p9_req_t *req,
 	// RERROR needs reply (== error string) in static data
 	if (READ_ONCE(req->status) == REQ_STATUS_RCVD &&
 	    unlikely(req->rc.sdata[4] == P9_RERROR))
-		handle_rerror(req, in_hdr_len, offs, in_pages);
+		handle_rerror(req, in_hdr_len, offs, in_pages, in_nr_pages);
 
 	/*
 	 * Non kernel buffers are pinned, unpin them
-- 
2.43.0

Re: [PATCH] 9p/trans_virtio: bound RERROR copy by mapped pages

[Dominique Martinet]

Yizhou Zhao wrote on Sun, Jun 07, 2026 at 10:06:01PM +0800:
> handle_rerror() copies the variable-length error string of a zero-copy
> RERROR response from the receive pages into the request's static response
> buffer.  The amount copied is bounded by P9_ZC_HDR_SZ, so the data can
> span at most two pages, but the helper is not told how many receive pages
> were actually mapped.
> 
> If a malicious or broken virtio 9p device reports an RERROR length that
> exceeds the remaining bytes in the first mapped receive page, the error
> string is treated as crossing into a second page.  When only one receive
> page was mapped, handle_rerror() still advances the page pointer and
> dereferences the next entry, reading past the allocated in_pages array.

I'm not sure that can actually happen:
If there was an "in page" (if there wasn't this is all noop anyway and
data was written directly to req->rc.sdata), then req->rc.size amount of
data was received into the pages, so if it does happen to span over a
page boundary then there are at least two pages and we don't need to
double-check here.

Christian, do you agree with me this patch is not required?

> Pass the number of mapped receive pages to handle_rerror().  If the error
> string would cross a page boundary but only one page is available, copy the
> bytes that fit in that page and leave the response truncated, matching the
> existing behavior for overlong RERROR messages.  Otherwise continue with
> the second-page copy as before.


-- 
Dominique Martinet | Asmadeus