Re: [PATCH] virtio-net: make copy len check in xdp_linearize_page

On Mon, Dec 13, 2021 at 5:14 PM 孙蒙恩 [off-list ref] wrote:

Jason Wang [off-list ref] 于2021年12月13日周一 15:49写道：

quoted

On Mon, Dec 13, 2021 at 12:50 PM [off-list ref] wrote:

quoted

From: mengensun <redacted>

xdp_linearize_page asume ring elem size is smaller then page size
when copy the first ring elem, but, there may be a elem size bigger
then page size.

add_recvbuf_mergeable may add a hole to ring elem, the hole size is
not sure, according EWMA.

The logic is to try to avoid dropping packets in this case, so I
wonder if it's better to "fix" the add_recvbuf_mergeable().

Adding lists back.

Then the truesize here is wrong.

        err = virtqueue_add_inbuf_ctx(rq->vq, rq->sg, 1, buf, ctx, gfp);

it seems a rule that, length of elem giving to vring is away smaller
or equall then PAGE_SIZE

It aims to be consistent to what EWMA tries to do:

        len = hdr_len + clamp_t(unsigned int, ewma_pkt_len_read(avg_pkt_len),
                        rq->min_buf_len, PAGE_SIZE - hdr_len);

Thanks

quoted

Or another idea is to switch to use XDP generic here where we can use
skb_linearize() which should be more robust and we can drop the
xdp_linearize_page() logic completely.

Thanks

quoted

so, fix it by check copy len,if checked failed, just dropped the
whole frame, not make the memory dirty after the page.

Signed-off-by: mengensun <redacted>
Reviewed-by: MengLong Dong <redacted>
Reviewed-by: ZhengXiong Jiang <redacted>
---
 drivers/net/virtio_net.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 36a4b7c195d5..844bdbd67ff7 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c

@@ -662,8 +662,12 @@ static struct page *xdp_linearize_page(struct receive_queue *rq,
                                       int page_off,
                                       unsigned int *len)
 {
-       struct page *page = alloc_page(GFP_ATOMIC);
+       struct page *page;

+       if (*len > PAGE_SIZE - page_off)
+               return NULL;
+
+       page = alloc_page(GFP_ATOMIC);
        if (!page)
                return NULL;

--

2.27.0