vhost: no protection against malformed queue descriptors in rte_vhost_dequeue_burst()

Hello,

When taking a snapshot of a running VM instance, using OpenStack "nova image-create", I noticed that one OVS pmd-thread eventually failed in DPDK rte_vhost_dequeue_burst() with repeating log entries:

compute-0-6 ovs-vswitchd[38172]: VHOST_DATA: Failed to allocate
memory for mbuf.

Debugging (data included further down) this issue lead to the observation that there is no protection against malformed vhost queue descriptors, thus tenant separation might be violated as a single faulty VM might bring down the connectivity of all VMs connected to the same virtual switch.

To avoid this, validation would be needed at some points in the rte_vhost_dequeue_burst() code:

1) when the queue descriptor is picked up for processing,

      desc->flags and desc->len might both be 0

       ...
       desc = &vq->desc[head[entry_success]];
       ...
       /* Discard first buffer as it is the virtio header */
       if (desc->flags & VRING_DESC_F_NEXT) {
            desc = &vq->desc[desc->next];
            vb_offset = 0;
            vb_avail = desc->len;
       } else {
            vb_offset = vq->vhost_hlen;
            vb_avail = desc->len - vb_offset;
       }
        ....

2) at buffer address translation gpa_to_vva(), might fail

      returning NULL as indication

       vb_addr = gpa_to_vva(dev, desc->addr);
       ...
       while (cpy_len != 0) {
            rte_memcpy(rte_pktmbuf_mtod_offset(cur, void *, seg_offset),
                (void *)((uintptr_t)(vb_addr + vb_offset)),
                cpy_len);
       ...
       }
       ...

Wondering if there are any plans of adding any kind of validation in DPDK, or if it would be useful to suggest specific implementation of such validations in the DPDK code?

Or is there some mechanism that gives us the confidence to trust the vhost queue content absolutely?

Debugging data:

For my scenario the problem occurs in DPDK rte_vhost_dequeue_burst() due to use of a vhost queue descriptor that has all fields 0:

(gdb) print *desc

           {addr = 0, len = 0, flags = 0, next = 0}

Subsequent use of desc->len to compute vb_avail = desc->len - vb_offset, leads to the problem observed. What happens is that the packet needs to be segmented -- on my system it fails roughly at segment 122000 when memory available for mbufs run out.

The relevant local variables for rte_vhost_dequeue_burst() when breaking on the condition desc->len == 0:

   vb_avail = 4294967284  (0xfffffff4)
   seg_avail = 2608
   vb_offset = 12
   cpy_len = 2608
   seg_num = 1
   desc = 0x2aadb6e5c000
   vb_addr = 46928960159744
   entry_success = 0

Note also that there is no crash despite to the desc->addr being zero, it is a valid address in the regions mapped to the device. Although, the 3 regions mapped does not seem to be correct either at this stage.

The versions that I'm running are OVS 2.4.0, with corrections from the 2.4 branch, and DPDK 2.1.0. QEMU emulator version 2.2.0 and libvirt version 1.2.12.

Regards,

Patrik

Quoted 90 lines · Patrik Andersson R

Hello,

When taking a snapshot of a running VM instance, using OpenStack "nova image-create", I noticed that one OVS pmd-thread eventually failed in DPDK rte_vhost_dequeue_burst() with repeating log entries:

compute-0-6 ovs-vswitchd[38172]: VHOST_DATA: Failed to allocate
memory for mbuf.

Debugging (data included further down) this issue lead to the observation that there is no protection against malformed vhost queue descriptors, thus tenant separation might be violated as a single faulty VM might bring down the connectivity of all VMs connected to the same virtual switch.

To avoid this, validation would be needed at some points in the rte_vhost_dequeue_burst() code:

   1) when the queue descriptor is picked up for processing,
       desc->flags and desc->len might both be 0

        ...
        desc = &vq->desc[head[entry_success]];
        ...
        /* Discard first buffer as it is the virtio header */
        if (desc->flags & VRING_DESC_F_NEXT) {
             desc = &vq->desc[desc->next];
             vb_offset = 0;
             vb_avail = desc->len;
        } else {
             vb_offset = vq->vhost_hlen;
             vb_avail = desc->len - vb_offset;
        }
         ....

   2) at buffer address translation gpa_to_vva(), might fail
       returning NULL as indication

        vb_addr = gpa_to_vva(dev, desc->addr);
        ...
        while (cpy_len != 0) {
             rte_memcpy(rte_pktmbuf_mtod_offset(cur, void *, seg_offset),
                 (void *)((uintptr_t)(vb_addr + vb_offset)),
                 cpy_len);
        ...
        }
        ...

Wondering if there are any plans of adding any kind of validation in DPDK, or if it would be useful to suggest specific implementation of such validations in the DPDK code?

Or is there some mechanism that gives us the confidence to trust the vhost queue content absolutely?

Debugging data:

For my scenario the problem occurs in DPDK rte_vhost_dequeue_burst() due to use of a vhost queue descriptor that has all fields 0:

(gdb) print *desc

            {addr = 0, len = 0, flags = 0, next = 0}

Subsequent use of desc->len to compute vb_avail = desc->len - vb_offset, leads to the problem observed. What happens is that the packet needs to be segmented -- on my system it fails roughly at segment 122000 when memory available for mbufs run out.

The relevant local variables for rte_vhost_dequeue_burst() when breaking on the condition desc->len == 0:

    vb_avail = 4294967284  (0xfffffff4)
    seg_avail = 2608
    vb_offset = 12
    cpy_len = 2608
    seg_num = 1
    desc = 0x2aadb6e5c000
    vb_addr = 46928960159744
    entry_success = 0

Note also that there is no crash despite to the desc->addr being zero, it is a valid address in the regions mapped to the device. Although, the 3 regions mapped does not seem to be correct either at this stage.

Quoted 8 lines

The versions that I'm running are OVS 2.4.0, with corrections from the 2.4 branch, and DPDK 2.1.0. QEMU emulator version 2.2.0 and libvirt version 1.2.12.

Regards,

Patrik

Thanks Patrik. You are right. We had planned to enhance the robustness of vhost so that neither malicious nor buggy guest virtio driver could corrupt vhost. Actually the 16.04 RC1 has fixed some issues (the return of gpa_to_vva isn't checked).