lkml.org 
[lkml]   [2022]   [Nov]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] 9p/xen: check logical size for buffer size
On Fri, 18 Nov 2022, Dominique Martinet wrote:
> trans_xen did not check the data fits into the buffer before copying
> from the xen ring, but we probably should.
> Add a check that just skips the request and return an error to
> userspace if it did not fit
>
> Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
> ---
>
> This comes more or less as a follow up of a fix for trans_fd:
> https://lkml.kernel.org/r/20221117091159.31533-1-guozihua@huawei.com
> Where msize should be replaced by capacity check, except trans_xen
> did not actually use to check the size fits at all.
>
> While we normally trust the hypervisor (they can probably do whatever
> they want with our memory), a bug in the 9p server is always possible so
> sanity checks never hurt, especially now buffers got drastically smaller
> with a recent patch.
>
> My setup for xen is unfortunately long dead so I cannot test this:
> Stefano, you've tested v9fs xen patches in the past, would you mind
> verifying this works as well?
>
> net/9p/trans_xen.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> index b15c64128c3e..66ceb3b3ae30 100644
> --- a/net/9p/trans_xen.c
> +++ b/net/9p/trans_xen.c
> @@ -208,6 +208,14 @@ static void p9_xen_response(struct work_struct *work)
> continue;
> }
>
> + if (h.size > req->rc.capacity) {
> + dev_warn(&priv->dev->dev,
> + "requested packet size too big: %d for tag %d with capacity %zd\n",
> + h.size, h.tag, rreq->rc.capacity);

"req" instead of "rreq"

I made this change and tried the two patches together. Unfortunately I
get the following error as soon as I try to write a file:

/bin/sh: can't create /mnt/file: Input/output error


Next I reverted the second patch and only kept this patch. With that, it
worked as usual. It looks like the second patch is the problem. I have
not investigated further.



> + req->status = REQ_STATUS_ERROR;
> + goto recv_error;
> + }
> +
> memcpy(&req->rc, &h, sizeof(h));
> req->rc.offset = 0;
>
> @@ -217,6 +225,7 @@ static void p9_xen_response(struct work_struct *work)
> masked_prod, &masked_cons,
> XEN_9PFS_RING_SIZE(ring));
>
> +recv_error:
> virt_mb();
> cons += h.size;
> ring->intf->in_cons = cons;
> --
> 2.38.1
>

\
 
 \ /
  Last update: 2022-11-19 03:00    [W:0.197 / U:0.952 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site