netdev - Re: [PATCH net 1/4] virtio-net: ensure the received length does not exceed allocated size

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACGkMEvioXkt3_zB-KijwhoUx5NS5xa0Jvd=w2fhBZFf3un1Ww@mail.gmail.com>
Date: Thu, 26 Jun 2025 10:34:02 +0800
From: Jason Wang <jasowang@...hat.com>
To: Bui Quang Minh <minhquangbui99@...il.com>
Cc: netdev@...r.kernel.org, "Michael S. Tsirkin" <mst@...hat.com>, 
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Eugenio Pérez <eperezma@...hat.com>, 
	Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Jesper Dangaard Brouer <hawk@...nel.org>, John Fastabend <john.fastabend@...il.com>, 
	Stanislav Fomichev <sdf@...ichev.me>, virtualization@...ts.linux.dev, 
	linux-kernel@...r.kernel.org, bpf@...r.kernel.org
Subject: Re: [PATCH net 1/4] virtio-net: ensure the received length does not
 exceed allocated size

On Thu, Jun 26, 2025 at 12:10 AM Bui Quang Minh
<minhquangbui99@...il.com> wrote:
>
> In xdp_linearize_page, when reading the following buffers from the ring,
> we forget to check the received length with the true allocate size. This
> can lead to an out-of-bound read. This commit adds that missing check.
>
> Fixes: 4941d472bf95 ("virtio-net: do not reset during XDP set")

I think we should cc stable.

> Signed-off-by: Bui Quang Minh <minhquangbui99@...il.com>
> ---
>  drivers/net/virtio_net.c | 27 ++++++++++++++++++++++-----
>  1 file changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index e53ba600605a..2a130a3e50ac 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1797,7 +1797,8 @@ static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
>   * across multiple buffers (num_buf > 1), and we make sure buffers
>   * have enough headroom.
>   */
> -static struct page *xdp_linearize_page(struct receive_queue *rq,
> +static struct page *xdp_linearize_page(struct net_device *dev,
> +                                      struct receive_queue *rq,
>                                        int *num_buf,
>                                        struct page *p,
>                                        int offset,
> @@ -1818,17 +1819,33 @@ static struct page *xdp_linearize_page(struct receive_queue *rq,
>         page_off += *len;
>
>         while (--*num_buf) {
> -               unsigned int buflen;
> +               unsigned int headroom, tailroom, room;
> +               unsigned int truesize, buflen;
>                 void *buf;
> +               void *ctx;
>                 int off;
>
> -               buf = virtnet_rq_get_buf(rq, &buflen, NULL);
> +               buf = virtnet_rq_get_buf(rq, &buflen, &ctx);
>                 if (unlikely(!buf))
>                         goto err_buf;
>
>                 p = virt_to_head_page(buf);
>                 off = buf - page_address(p);
>
> +               truesize = mergeable_ctx_to_truesize(ctx);

This won't work for receive_small_xdp().

> +               headroom = mergeable_ctx_to_headroom(ctx);
> +               tailroom = headroom ? sizeof(struct skb_shared_info) : 0;
> +               room = SKB_DATA_ALIGN(headroom + tailroom);
> +
> +               if (unlikely(buflen > truesize - room)) {
> +                       put_page(p);
> +                       pr_debug("%s: rx error: len %u exceeds truesize %lu\n",
> +                                dev->name, buflen,
> +                                (unsigned long)(truesize - room));
> +                       DEV_STATS_INC(dev, rx_length_errors);
> +                       goto err_buf;
> +               }

I wonder if this issue only affect XDP should we check other places?

> +
>                 /* guard against a misconfigured or uncooperative backend that
>                  * is sending packet larger than the MTU.
>                  */
> @@ -1917,7 +1934,7 @@ static struct sk_buff *receive_small_xdp(struct net_device *dev,
>                 headroom = vi->hdr_len + header_offset;
>                 buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
>                         SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> -               xdp_page = xdp_linearize_page(rq, &num_buf, page,
> +               xdp_page = xdp_linearize_page(dev, rq, &num_buf, page,
>                                               offset, header_offset,
>                                               &tlen);
>                 if (!xdp_page)
> @@ -2252,7 +2269,7 @@ static void *mergeable_xdp_get_buf(struct virtnet_info *vi,
>          */
>         if (!xdp_prog->aux->xdp_has_frags) {
>                 /* linearize data for XDP */
> -               xdp_page = xdp_linearize_page(rq, num_buf,
> +               xdp_page = xdp_linearize_page(vi->dev, rq, num_buf,
>                                               *page, offset,
>                                               XDP_PACKET_HEADROOM,
>                                               len);
> --
> 2.43.0
>