[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 9 Sep 2021 10:52:59 +0800
From: Jason Wang <jasowang@...hat.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: netdev <netdev@...r.kernel.org>,
"Michael S. Tsirkin" <mst@...hat.com>, kvm <kvm@...r.kernel.org>,
virtualization <virtualization@...ts.linux-foundation.org>
Subject: Re: [PATCH] vhost_net: fix OoB on sendmsg() failure.
On Wed, Sep 8, 2021 at 7:42 PM Paolo Abeni <pabeni@...hat.com> wrote:
>
> If the sendmsg() call in vhost_tx_batch() fails, both the 'batched_xdp'
> and 'done_idx' indexes are left unchanged. If such failure happens
> when batched_xdp == VHOST_NET_BATCH, the next call to
> vhost_net_build_xdp() will access and write memory outside the xdp
> buffers area.
>
> Since sendmsg() can only error with EBADFD, this change addresses the
> issue explicitly freeing the XDP buffers batch on error.
>
> Fixes: 0a0be13b8fe2 ("vhost_net: batch submitting XDP buffers to underlayer sockets")
> Suggested-by: Jason Wang <jasowang@...hat.com>
> Signed-off-by: Paolo Abeni <pabeni@...hat.com>
> ---
> Note: my understanding is that this should go through MST's tree, please
> educate me otherwise, thanks!
> ---
Acked-by: Jason Wang <jasowang@...hat.com>
Thanks!
> drivers/vhost/net.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 3a249ee7e144..28ef323882fb 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -467,7 +467,7 @@ static void vhost_tx_batch(struct vhost_net *net,
> .num = nvq->batched_xdp,
> .ptr = nvq->xdp,
> };
> - int err;
> + int i, err;
>
> if (nvq->batched_xdp == 0)
> goto signal_used;
> @@ -476,6 +476,15 @@ static void vhost_tx_batch(struct vhost_net *net,
> err = sock->ops->sendmsg(sock, msghdr, 0);
> if (unlikely(err < 0)) {
> vq_err(&nvq->vq, "Fail to batch sending packets\n");
> +
> + /* free pages owned by XDP; since this is an unlikely error path,
> + * keep it simple and avoid more complex bulk update for the
> + * used pages
> + */
> + for (i = 0; i < nvq->batched_xdp; ++i)
> + put_page(virt_to_head_page(nvq->xdp[i].data));
> + nvq->batched_xdp = 0;
> + nvq->done_idx = 0;
> return;
> }
>
> --
> 2.26.3
>
Powered by blists - more mailing lists