Hi!

> 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.
> 

> @@ -551,7 +551,7 @@ static void handle_tx_copy(struct vhost_
>       int err;
>       int sent_pkts = 0;
>  
> -     for (;;) {
> +     do {
>               bool busyloop_intr = false;
>  
>               head = get_tx_bufs(net, nvq, &msg, &out, &in, &len,
> @@ -592,9 +592,7 @@ static void handle_tx_copy(struct vhost_
>                                err, len);
>               if (++nvq->done_idx >= VHOST_NET_BATCH)
>                       vhost_net_signal_used(nvq);
> -             if (vhost_exceeds_weight(vq, ++sent_pkts, total_len))
> -                     break;
> -     }
> +     } while (likely(!vhost_exceeds_weight(vq, ++sent_pkts, total_len)));
>  
>       vhost_net_signal_used(nvq);
>  }

So this part does not really change anything, right?

> @@ -618,7 +616,7 @@ static void handle_tx_zerocopy(struct vh
>       bool zcopy_used;
>       int sent_pkts = 0;
>  
> -     for (;;) {
> +     do {
>               bool busyloop_intr;
>  
>               /* Release DMAs done buffers first */
> @@ -693,10 +691,7 @@ static void handle_tx_zerocopy(struct vh
>               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)));
>  }
>  
>  /* Expects to be always run from workqueue - which acts as

Neither does this. Equivalent code. Changelog says it fixes something
for the transmit so... is that intentional?

                                                                        Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Attachment: signature.asc
Description: Digital signature

Reply via email to