vhost_net: fix possible infinite loop [Linux 4.14.133]

vhost_net: fix possible infinite loop [Linux 4.14.133]

This Linux kernel change "vhost_net: fix possible infinite loop" is included in the Linux 4.14.133 release. This change is authored by Jason Wang <jasowang [at] redhat.com> on Fri May 17 00:29:50 2019 -0400. The commit for this change in Linux stable tree is ae44674 (patch) which is from upstream commit e2412c0. The same Linux upstream change may have been applied to various maintained Linux releases and you can find all Linux releases containing changes from upstream e2412c0.

vhost_net: fix possible infinite loop

commit e2412c07f8f3040593dfb88207865a3cd58680c0 upstream.

When the rx buffer is too small for a packet, we will discard the vq
descriptor and retry it for the next packet:

while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk,
                          &busyloop_intr))) {
...
    /* On overrun, truncate and discard */
    if (unlikely(headcount > UIO_MAXIOV)) {
        iov_iter_init(&msg.msg_iter, READ, vq->iov, 1, 1);
        err = sock->ops->recvmsg(sock, &msg,
                     1, MSG_DONTWAIT | MSG_TRUNC);
        pr_debug("Discarded rx packet: len %zd\n", sock_len);
        continue;
    }
...
}

This makes it possible to trigger a infinite while..continue loop
through the co-opreation of two VMs like:

1) Malicious VM1 allocate 1 byte rx buffer and try to slow down the
   vhost process as much as possible e.g using indirect descriptors or
   other.
2) Malicious VM2 generate packets to VM1 as fast as possible

Fixing this by checking against weight at the end of RX and TX
loop. This also eliminate other similar cases when:

- userspace is consuming the packets in the meanwhile
- theoretical TOCTOU attack if guest moving avail index back and forth
  to hit the continue after vhost find guest just add new buffers

This addresses CVE-2019-3900.

Fixes: d8316f3991d20 ("vhost: fix total length when packets are too short")
Fixes: 3a4d5c94e9593 ("vhost_net: a kernel-level virtio server")
Signed-off-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Balbir Singh <sblbir@amzn.com>

There are 20 lines of Linux source code added/deleted in this change. Code changes to Linux kernel are as follows.

 drivers/vhost/net.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index a4ca034..b40e8de 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -482,7 +482,7 @@ static void handle_tx(struct vhost_net *net)
    hdr_size = nvq->vhost_hlen;
    zcopy = nvq->ubufs;

-   for (;;) {
+   do {
        /* Release DMAs done buffers first */
        if (zcopy)
            vhost_zerocopy_signal_used(net, vq);
@@ -578,10 +578,7 @@ static void handle_tx(struct vhost_net *net)
        else
            vhost_zerocopy_signal_used(net, vq);
        vhost_net_tx_packet(net);
-       if (unlikely(vhost_exceeds_weight(vq, ++sent_pkts,
-                         total_len)))
-           break;
-   }
+   } while (likely(!vhost_exceeds_weight(vq, ++sent_pkts, total_len)));
 out:
    mutex_unlock(&vq->mutex);
 }
@@ -779,7 +776,11 @@ static void handle_rx(struct vhost_net *net)
        vq->log : NULL;
    mergeable = vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF);

-   while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk))) {
+   do {
+       sock_len = vhost_net_rx_peek_head_len(net, sock->sk);
+
+       if (!sock_len)
+           break;
        sock_len += sock_hlen;
        vhost_len = sock_len + vhost_hlen;
        headcount = get_rx_bufs(vq, vq->heads, vhost_len,
@@ -860,9 +861,8 @@ static void handle_rx(struct vhost_net *net)
            vhost_log_write(vq, vq_log, log, vhost_len,
                    vq->iov, in);
        total_len += vhost_len;
-       if (unlikely(vhost_exceeds_weight(vq, ++recv_pkts, total_len)))
-           goto out;
-   }
+   } while (likely(!vhost_exceeds_weight(vq, ++recv_pkts, total_len)));
+
    vhost_net_enable_vq(net, vq);
 out:
    mutex_unlock(&vq->mutex);
@@ -941,7 +941,7 @@ static int vhost_net_open(struct inode *inode, struct file *f)
        vhost_net_buf_init(&n->vqs[i].rxq);
    }
    vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX,
-              VHOST_NET_WEIGHT, VHOST_NET_PKT_WEIGHT);
+              VHOST_NET_PKT_WEIGHT, VHOST_NET_WEIGHT);

    vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
    vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);

Leave a Reply

Your email address will not be published. Required fields are marked *