|
From: | Jason Wang |
Subject: | Re: [Qemu-devel] [PATCH V4 for 3.1 4/4] virtio-net-test: add large tx buffer test |
Date: | Tue, 4 Dec 2018 10:52:54 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 |
On 2018/12/4 上午12:46, Eric Blake wrote:
On 12/3/18 4:06 AM, 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.Can you also add a packet just slightly larger than NET_BUFSIZE (68k) to show that we aren't having any further issues at even smaller (and more likely) values of oversized packets?
Ok.
Signed-off-by: Jason Wang <address@hidden> --- tests/virtio-net-test.c | 44 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)+++ b/tests/virtio-net-test.c @@ -245,6 +245,49 @@ 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; + uint64_t req_addr; + uint32_t free_head; + size_t alloc_size = UINT_MAX / 64; + int i; + + qs = pci_test_start("-netdev hubport,id=hp0,hubid=0 " + "-device virtio-net-pci,netdev=hp0 ");Why the trailing space?
I guess this is a cut and paste error.
+ 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); + free_head = qvirtqueue_add(vq, req_addr, alloc_size, false, true); + + for (i = 0; i < 64; i++) { + qvirtqueue_add(vq, req_addr, alloc_size, false, i == 63 ? + false : true);Any time I see both 'true' and 'false' in a ?: operator, I have to wonder why you didn't just write the simpler version directly on the condition. This is the same as 'i != 63'.
Ok.
+ } + 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 +313,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);I can reproduce Peter's complaints of this introducing noise to 'make check':qemu-system-x86_64: warning: hub 0 is not connected to host network
Yes, this is intended since the test does not require any host network. I will add a patch to suppress this warning if qtest is enabled.
With patches 2-4 applied but patch 1 omitted, 'make check' fails with: Broken pipetests/libqtest.c:125: kill_qemu() detected QEMU death from signal 11 (Segmentation fault) (core dumped)GTester: last random seed: R02S9569aabcb2d834a9dadcce272c5588dbso the test is definitely triggering the problem as patched by part 1. Although I'm not confident enough of what the test is doing for an R-b, and would like it to be less noisy, I can at least add:Tested-by: Eric Blake <address@hidden>
Thanks
[Prev in Thread] | Current Thread | [Next in Thread] |