netdev - Re: [PATCH] virtio_net: Fix mismatched buf address when unmapping for small packets

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

Message-ID: <CACGkMEuH4ERuvGPPzBvmAT0B51ccbeBvE=PiQxn2s-J_wXugOg@mail.gmail.com>
Date: Wed, 4 Sep 2024 11:33:32 +0800
From: Jason Wang <jasowang@...hat.com>
To: Wenbo Li <liwenbo.martin@...edance.com>
Cc: mst@...hat.com, xuanzhuo@...ux.alibaba.com, eperezma@...hat.com, 
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com, 
	virtualization@...ts.linux.dev, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Jiahui Cen <cenjiahui@...edance.com>, 
	Ying Fang <fangying.tommy@...edance.com>
Subject: Re: [PATCH] virtio_net: Fix mismatched buf address when unmapping for
 small packets

On Wed, Sep 4, 2024 at 10:51 AM Wenbo Li <liwenbo.martin@...edance.com> wrote:
>
> Currently, the virtio-net driver will perform a pre-dma-mapping for
> small or mergeable RX buffer. But for small packets, a mismatched address
> without VIRTNET_RX_PAD and xdp_headroom is used for unmapping.
>
> That will result in unsynchronized buffers when SWIOTLB is enabled, for
> example, when running as a TDX guest.
>
> This patch handles small and mergeable packets separately and fixes
> the mismatched buffer address.
>

Missing fixes tag.

> Signed-off-by: Wenbo Li <liwenbo.martin@...edance.com>
> Signed-off-by: Jiahui Cen <cenjiahui@...edance.com>
> Signed-off-by: Ying Fang <fangying.tommy@...edance.com>
> ---
>  drivers/net/virtio_net.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index c6af18948..6215b66d8 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -891,6 +891,20 @@ static void *virtnet_rq_get_buf(struct receive_queue *rq, u32 *len, void **ctx)
>         return buf;
>  }
>
> +static void *virtnet_rq_get_buf_small(struct receive_queue *rq,
> +                                     u32 *len,
> +                                     void **ctx,
> +                                     unsigned int header_offset)
> +{
> +       void *buf;
> +
> +       buf = virtqueue_get_buf_ctx(rq->vq, len, ctx);
> +       if (buf)
> +               virtnet_rq_unmap(rq, buf + header_offset, *len);
> +
> +       return buf;
> +}
> +
>  static void virtnet_rq_init_one_sg(struct receive_queue *rq, void *buf, u32 len)
>  {
>         struct virtnet_rq_dma *dma;
> @@ -2692,13 +2706,23 @@ static int virtnet_receive_packets(struct virtnet_info *vi,
>         int packets = 0;
>         void *buf;
>
> -       if (!vi->big_packets || vi->mergeable_rx_bufs) {
> +       if (vi->mergeable_rx_bufs) {
>                 void *ctx;
>                 while (packets < budget &&
>                        (buf = virtnet_rq_get_buf(rq, &len, &ctx))) {
>                         receive_buf(vi, rq, buf, len, ctx, xdp_xmit, stats);
>                         packets++;
>                 }
> +       } else if (!vi->big_packets) {
> +               void *ctx;
> +               unsigned int xdp_headroom = virtnet_get_headroom(vi);

I wonder if this is safe. The headroom is stored as the context, it
looks to me we should fetch the headroom there.

The rx buffer could be allocated before XDP is disabled. For example we had this

        unsigned int xdp_headroom = (unsigned long)ctx;

at the beginning of receive_small().


> +               unsigned int header_offset = VIRTNET_RX_PAD + xdp_headroom;
> +
> +               while (packets < budget &&
> +                      (buf = virtnet_rq_get_buf_small(rq, &len, &ctx, header_offset))) {
> +                       receive_buf(vi, rq, buf, len, ctx, xdp_xmit, stats);
> +                       packets++;
> +               }
>         } else {
>                 while (packets < budget &&
>                        (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
> --
> 2.20.1
>

Thanks