Re: [Qemu-devel] [PATCH for 3.1 4/4] virtio-net-test: add large tx buffer test

[Jason Wang <jasowang@redhat.com>]

On 2018/11/29 下午2:21, Thomas Huth wrote:
> On 2018-11-29 04:12, Jason Wang wrote:
>> This test tries to build a packet whose size is greater than INT_MAX
>> which tries to trigger integer overflow in qemu_net_queue_append_iov()
>> which may result OOB.
>>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>   tests/virtio-net-test.c | 46 +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>
>> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
>> index 33d26ab079..09c220c2fa 100644
>> --- a/tests/virtio-net-test.c
>> +++ b/tests/virtio-net-test.c
>> @@ -245,6 +245,51 @@ static void pci_basic(gconstpointer data)
>>       g_free(dev);
>>       qtest_shutdown(qs);
>>   }
>> +
>> +static void large_tx(gconstpointer data)
>> +{
>> +    QVirtioPCIDevice *dev;
>> +    QOSState *qs;
>> +    QVirtQueuePCI *tx, *rx;
>> +    QVirtQueue *vq;
>> +    const char *cmd = "-netdev hubport,id=hp0,hubid=0 "
>> +                      "-device virtio-net-pci,netdev=hp0 ";
>> +    uint64_t req_addr;
>> +    uint32_t free_head;
>> +    size_t alloc_size = UINT_MAX / 64;
>> +    int i;
>> +
>> +    qs = pci_test_start(cmd);
>> +    dev = virtio_net_pci_init(qs->pcibus, PCI_SLOT);
>> +
>> +    rx = (QVirtQueuePCI *)qvirtqueue_setup(&dev->vdev, qs->alloc, 0);
>> +    tx = (QVirtQueuePCI *)qvirtqueue_setup(&dev->vdev, qs->alloc, 1);
>> +
>> +    driver_init(&dev->vdev);
>> +    vq = &tx->vq;
>> +
>> +    /* Bypass the limitation by pointing several descriptors to a single
>> +     * smaller area */
>> +    req_addr = guest_alloc(qs->alloc, alloc_size);
>> +    for (i = 0; i < 64; i ++) {
>> +        if (i == 0)
>> +            free_head = qvirtqueue_add(vq, req_addr, alloc_size, false, true);
>> +        else
>> +            qvirtqueue_add(vq, req_addr, alloc_size, false, true);
> QEMU coding style requires always curly braces. Maybe move the initial
> case before the for-loop and then start the for loop with i=1 instead?


Ok, fix and post V2.

Thanks


>
>> +    }
>> +    qvirtqueue_add(vq, req_addr, alloc_size, false, false);
>> +    qvirtqueue_kick(&dev->vdev, vq, free_head);
>> +
>> +    qvirtio_wait_used_elem(&dev->vdev, vq, free_head, NULL,
>> +                           QVIRTIO_NET_TIMEOUT_US);
>> +
>> +    qvirtqueue_cleanup(dev->vdev.bus, &tx->vq, qs->alloc);
>> +    qvirtqueue_cleanup(dev->vdev.bus, &rx->vq, qs->alloc);
>> +    qvirtio_pci_device_disable(dev);
>> +    g_free(dev->pdev);
>> +    g_free(dev);
>> +    qtest_shutdown(qs);
>> +}
>>   #endif
>>   
>>   static void hotplug(void)
>> @@ -270,6 +315,7 @@ int main(int argc, char **argv)
>>       qtest_add_data_func("/virtio/net/pci/basic", send_recv_test, pci_basic);
>>       qtest_add_data_func("/virtio/net/pci/rx_stop_cont",
>>                           stop_cont_test, pci_basic);
>> +    qtest_add_data_func("/virtio/net/pci/large_tx", NULL, large_tx);
>>   #endif
>>       qtest_add_func("/virtio/net/pci/hotplug", hotplug);
>   Thomas