Skip to content

Commit e2412c0

Browse files
jasowangmstsirkin
authored andcommitted
vhost_net: fix possible infinite loop
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: d8316f3 ("vhost: fix total length when packets are too short") Fixes: 3a4d5c9 ("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>
1 parent e82b9b0 commit e2412c0

File tree

1 file changed

+13
-16
lines changed

1 file changed

+13
-16
lines changed

drivers/vhost/net.c

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,7 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock)
773773
int sent_pkts = 0;
774774
bool sock_can_batch = (sock->sk->sk_sndbuf == INT_MAX);
775775

776-
for (;;) {
776+
do {
777777
bool busyloop_intr = false;
778778

779779
if (nvq->done_idx == VHOST_NET_BATCH)
@@ -839,9 +839,7 @@ static void handle_tx_copy(struct vhost_net *net, struct socket *sock)
839839
vq->heads[nvq->done_idx].id = cpu_to_vhost32(vq, head);
840840
vq->heads[nvq->done_idx].len = 0;
841841
++nvq->done_idx;
842-
if (vhost_exceeds_weight(vq, ++sent_pkts, total_len))
843-
break;
844-
}
842+
} while (likely(!vhost_exceeds_weight(vq, ++sent_pkts, total_len)));
845843

846844
vhost_tx_batch(net, nvq, sock, &msg);
847845
}
@@ -866,7 +864,7 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock)
866864
bool zcopy_used;
867865
int sent_pkts = 0;
868866

869-
for (;;) {
867+
do {
870868
bool busyloop_intr;
871869

872870
/* Release DMAs done buffers first */
@@ -943,10 +941,7 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock)
943941
else
944942
vhost_zerocopy_signal_used(net, vq);
945943
vhost_net_tx_packet(net);
946-
if (unlikely(vhost_exceeds_weight(vq, ++sent_pkts,
947-
total_len)))
948-
break;
949-
}
944+
} while (likely(!vhost_exceeds_weight(vq, ++sent_pkts, total_len)));
950945
}
951946

952947
/* Expects to be always run from workqueue - which acts as
@@ -1144,8 +1139,11 @@ static void handle_rx(struct vhost_net *net)
11441139
vq->log : NULL;
11451140
mergeable = vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF);
11461141

1147-
while ((sock_len = vhost_net_rx_peek_head_len(net, sock->sk,
1148-
&busyloop_intr))) {
1142+
do {
1143+
sock_len = vhost_net_rx_peek_head_len(net, sock->sk,
1144+
&busyloop_intr);
1145+
if (!sock_len)
1146+
break;
11491147
sock_len += sock_hlen;
11501148
vhost_len = sock_len + vhost_hlen;
11511149
headcount = get_rx_bufs(vq, vq->heads + nvq->done_idx,
@@ -1230,12 +1228,11 @@ static void handle_rx(struct vhost_net *net)
12301228
vhost_log_write(vq, vq_log, log, vhost_len,
12311229
vq->iov, in);
12321230
total_len += vhost_len;
1233-
if (unlikely(vhost_exceeds_weight(vq, ++recv_pkts, total_len)))
1234-
goto out;
1235-
}
1231+
} while (likely(!vhost_exceeds_weight(vq, ++recv_pkts, total_len)));
1232+
12361233
if (unlikely(busyloop_intr))
12371234
vhost_poll_queue(&vq->poll);
1238-
else
1235+
else if (!sock_len)
12391236
vhost_net_enable_vq(net, vq);
12401237
out:
12411238
vhost_net_signal_used(nvq);
@@ -1328,7 +1325,7 @@ static int vhost_net_open(struct inode *inode, struct file *f)
13281325
}
13291326
vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX,
13301327
UIO_MAXIOV + VHOST_NET_BATCH,
1331-
VHOST_NET_WEIGHT, VHOST_NET_PKT_WEIGHT);
1328+
VHOST_NET_PKT_WEIGHT, VHOST_NET_WEIGHT);
13321329

13331330
vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, EPOLLOUT, dev);
13341331
vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, EPOLLIN, dev);

0 commit comments

Comments
 (0)