This patch will check the weight and exit the loop if we exceeds the
weight. This is useful for preventing scsi kthread from hogging cpu
which is guest triggerable.
This addresses CVE-2019-3900.
Cc: Paolo Bonzini <email address hidden>
Cc: Stefan Hajnoczi <email address hidden>
Fixes: 057cbf49a1f0 ("tcm_vhost: Initial merge for vhost level target fabric driver")
Signed-off-by: Jason Wang <email address hidden>
Reviewed-by: Stefan Hajnoczi <email address hidden>
Signed-off-by: Michael S. Tsirkin <email address hidden>
Reviewed-by: Stefan Hajnoczi <email address hidden>
CVE-2019-3900
(backported from commit c1ea02f15ab5efb3e93fc3144d895410bf79fcf2)
[tyhicks: Backport to Xenial:
- Minor context adjustment in local variables
- Adjust context around the loop in vhost_scsi_handle_vq()
- No need to modify vhost_scsi_ctl_handle_vq() since it was added later
in commit 0d02dbd68c47 ("vhost/scsi: Respond to control queue
operations")]
Signed-off-by: Tyler Hicks <email address hidden>
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 <email address hidden>
Reviewed-by: Stefan Hajnoczi <email address hidden>
Signed-off-by: Michael S. Tsirkin <email address hidden>
CVE-2019-3900
(backported from commit e2412c07f8f3040593dfb88207865a3cd58680c0)
[tyhicks: Backport to Xenial:
- Adjust handle_tx() instead of handle_tx_{copy,zerocopy}() due to
missing commit 0d20bdf34dc7 ("vhost_net: split out datacopy logic")
- Minor context adjustments due to a lack of missing the iov_limit
member of the vhost_dev struct which was added later in commit
b46a0bf78ad7 ("vhost: fix OOB in get_rx_bufs()")
- handle_rx() still uses peek_head_len() due to missing and unneeded commit
030881372460 ("vhost_net: basic polling support")
- Context adjustment in call to vhost_log_write() in hunk #3 of net.c due to
missing and unneeded commit cc5e71075947 ("vhost: log dirty page correctly")
- Context adjustment in hunk #4 due to using break instead of goto out
- Context adjustment in hunk #5 due to missing and unneeded commit
c67df11f6e48 ("vhost_net: try batch dequing from skb array")]
Signed-off-by: Tyler Hicks <email address hidden>
We used to have vhost_exceeds_weight() for vhost-net to:
- prevent vhost kthread from hogging the cpu
- balance the time spent between TX and RX
This function could be useful for vsock and scsi as well. So move it
to vhost.c. Device must specify a weight which counts the number of
requests, or it can also specific a byte_weight which counts the
number of bytes that has been processed.
Signed-off-by: Jason Wang <email address hidden>
Reviewed-by: Stefan Hajnoczi <email address hidden>
Signed-off-by: Michael S. Tsirkin <email address hidden>
CVE-2019-3900
(backported from commit e82b9b0727ff6d665fff2d326162b460dded554d)
[tyhicks: Backport to Xenial:
- Adjust handle_tx() instead of handle_tx_{copy,zerocopy}() due to
missing commit 0d20bdf34dc7 ("vhost_net: split out datacopy logic")
- Considerable context adjustments throughout the patch due to a lack
of missing the iov_limit member of the vhost_dev struct which was
added later in commit b46a0bf78ad7 ("vhost: fix OOB in get_rx_bufs()")
- Context adjustment in call to vhost_log_write() in hunk #3 of net.c due to
missing and unneeded commit cc5e71075947 ("vhost: log dirty page correctly")
- Context adjustment in hunk #3 of net.c due to using break instead of goto
out
- Context adjustment in hunk #4 of net.c due to missing and unneeded commit
c67df11f6e48 ("vhost_net: try batch dequing from skb array")
- Don't patch vsock.c since Xenial doesn't have vhost vsock support
- Adjust context in vhost_dev_init() to account for different local variables
- Adjust context in struct vhost_dev to account for different struct members]
Signed-off-by: Tyler Hicks <email address hidden>
Signed-off-by: Jason Wang <email address hidden>
Signed-off-by: David S. Miller <email address hidden>
CVE-2019-3900
(backported from commit 272f35cba53d088085e5952fd81d7a133ab90789)
[tyhicks: Backport to Xenial:
- Minor context adjustment in net.c due to missing commit b0d0ea50e782
("vhost_net: introduce helper to initialize tx iov iter")
- Context adjustment in call to vhost_log_write() in hunk #4 due to missing
and unneeded commit cc5e71075947 ("vhost: log dirty page correctly")
- Context adjustment in hunk #4 due to using break instead of goto out]
Signed-off-by: Tyler Hicks <email address hidden>
Similar to commit a2ac99905f1e ("vhost-net: set packet weight of
tx polling to 2 * vq size"), we need a packet-based limit for
handler_rx, too - elsewhere, under rx flood with small packets,
tx can be delayed for a very long time, even without busypolling.
The pkt limit applied to handle_rx must be the same applied by
handle_tx, or we will get unfair scheduling between rx and tx.
Tying such limit to the queue length makes it less effective for
large queue length values and can introduce large process
scheduler latencies, so a constant valued is used - likewise
the existing bytes limit.
The selected limit has been validated with PVP[1] performance
test with different queue sizes:
Signed-off-by: Paolo Abeni <email address hidden>
Acked-by: Jason Wang <email address hidden>
Signed-off-by: David S. Miller <email address hidden>
CVE-2019-3900
(backported from commit db688c24eada63b1efe6d0d7d835e5c3bdd71fd3)
[tyhicks: Backport to Xenial:
- Context adjustment in call to mutex_lock_nested() in hunk #3 due to missing
and unneeded commit aaa3149bbee9 ("vhost_net: add missing lock nesting
notation")
- Context adjustment in call to vhost_log_write() in hunk #4 due to missing
and unneeded commit cc5e71075947 ("vhost: log dirty page correctly")
- Context adjustment in hunk #4 due to using break instead of goto out]
Signed-off-by: Tyler Hicks <email address hidden>
vhost-net: set packet weight of tx polling to 2 * vq size
handle_tx will delay rx for tens or even hundreds of milliseconds when tx busy
polling udp packets with small length(e.g. 1byte udp payload), because setting
VHOST_NET_WEIGHT takes into account only sent-bytes but no single packet length.
Ping-Latencies shown below were tested between two Virtual Machines using
netperf (UDP_STREAM, len=1), and then another machine pinged the client:
Seems pretty consistent, a small dip at 2 VQ sizes.
Ring size is a hint from device about a burst size it can tolerate. Based on
benchmarks, set the weight to 2 * vq size.
To evaluate this change, another tests were done using netperf(RR, TX) between
two machines with Intel(R) Xeon(R) Gold 6133 CPU @ 2.50GHz, and vq size was
tweaked through qemu. Results shown below does not show obvious changes.
07b7d6d...
by
Willem de Bruijn <email address hidden>
vhost_net: do not stall on zerocopy depletion
Vhost-net has a hard limit on the number of zerocopy skbs in flight.
When reached, transmission stalls. Stalls cause latency, as well as
head-of-line blocking of other flows that do not use zerocopy.
Instead of stalling, revert to copy-based transmission.
Tested by sending two udp flows from guest to host, one with payload
of VHOST_GOODCOPY_LEN, the other too small for zerocopy (1B). The
large flow is redirected to a netem instance with 1MBps rate limit
and deep 1000 entry queue.
modprobe ifb
ip link set dev ifb0 up
tc qdisc add dev ifb0 root netem limit 1000 rate 1MBit
tc qdisc add dev tap0 ingress
tc filter add dev tap0 parent ffff: protocol ip \
u32 match ip dport 8000 0xffff \
action mirred egress redirect dev ifb0
Before the delay, both flows process around 80K pps. With the delay,
before this patch, both process around 400. After this patch, the
large flow is still rate limited, while the small reverts to its
original rate. See also discussion in the first link, below.
Without rate limiting, {1, 10, 100}x TCP_STREAM tests continued to
send at 100% zerocopy.
The limit in vhost_exceeds_maxpend must be carefully chosen. With
vq->num >> 1, the flows remain correlated. This value happens to
correspond to VHOST_MAX_PENDING for vq->num == 256. Allow smaller
fractions and ensure correctness also for much smaller values of
vq->num, by testing the min() of both explicitly. See also the
discussion in the second link below.
Changes
v1 -> v2
- replaced min with typed min_t
- avoid unnecessary whitespace change
Link:http://lkml.<email address hidden>
Link:http://<email address hidden>
Signed-off-by: Willem de Bruijn <email address hidden>
Signed-off-by: David S. Miller <email address hidden>
This patch tries to utilize tuntap rx batching by peeking the tx
virtqueue during transmission, if there's more available buffers in
the virtqueue, set MSG_MORE flag for a hint for backend (e.g tuntap)
to batch the packets.
Reviewed-by: Stefan Hajnoczi <email address hidden>
Signed-off-by: Jason Wang <email address hidden>
Acked-by: Michael S. Tsirkin <email address hidden>
Signed-off-by: David S. Miller <email address hidden>
CVE-2019-3900
(backported from commit 0ed005ce02fa0a88e5e6b7b5f7ff452171881610)
[tyhicks: Minor context adjustment due to missing commit 030881372460
("vhost_net: basic polling support")]
Signed-off-by: Tyler Hicks <email address hidden>
This patch introduces a helper which will return true if we're sure
that the available ring is empty for a specific vq. When we're not
sure, e.g vq access failure, return false instead. This could be used
for busy polling code to exit the busy loop.
Signed-off-by: Jason Wang <email address hidden>
Signed-off-by: Michael S. Tsirkin <email address hidden>
