On Wed, Sep 8, 2021 at 7:42 PM Paolo Abeni <pabeni@xxxxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: Paolo Abeni <pabeni@xxxxxxxxxx>
> ---
> Note: my understanding is that this should go through MST's tree, please
> educate me otherwise, thanks!
> ---
Acked-by: Jason Wang <jasowang@xxxxxxxxxx>
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
>
