netdev - Re: [PATCH net 2/2] virtio-net: get build

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 31 May 2021 14:10:55 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, netdev@...r.kernel.org
Cc:     "Michael S. Tsirkin" <mst@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        virtualization@...ts.linux-foundation.org, bpf@...r.kernel.org
Subject: Re: [PATCH net 2/2] virtio-net: get build_skb() buf by data ptr


在 2021/5/14 下午11:16, Xuan Zhuo 写道:
> In the case of merge, the page passed into page_to_skb() may be a head
> page, not the page where the current data is located.


I don't get how this can happen?

Maybe you can explain a little bit more?

receive_mergeable() call page_to_skb() in two places:

1) XDP_PASS for linearized page , in this case we use xdp_page
2) page_to_skb() for "normal" page, in this case the page contains the data

Thanks


> So when trying to
> get the buf where the data is located, you should directly use the
> pointer(p) to get the address corresponding to the page.
>
> At the same time, the offset of the data in the page should also be
> obtained using offset_in_page().
>
> This patch solves this problem. But if you don’t use this patch, the
> original code can also run, because if the page is not the page of the
> current data, the calculated tailroom will be less than 0, and will not
> enter the logic of build_skb() . The significance of this patch is to
> modify this logical problem, allowing more situations to use
> build_skb().
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
> Acked-by: Michael S. Tsirkin <mst@...hat.com>
> ---
>   drivers/net/virtio_net.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 3e46c12dde08..073fec4c0df1 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -407,8 +407,12 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>   		 * see add_recvbuf_mergeable() + get_mergeable_buf_len()
>   		 */
>   		truesize = PAGE_SIZE;
> -		tailroom = truesize - len - offset;
> -		buf = page_address(page);
> +
> +		/* page maybe head page, so we should get the buf by p, not the
> +		 * page
> +		 */
> +		tailroom = truesize - len - offset_in_page(p);
> +		buf = (char *)((unsigned long)p & PAGE_MASK);
>   	} else {
>   		tailroom = truesize - len;
>   		buf = p;