* [Qemu-devel] [PATCH v2 1/2] virtio: check VirtQueue Vring object is set
2017-11-24 17:52 [Qemu-devel] [PATCH v2 0/2] check VirtiQueue Vring objects P J P
@ 2017-11-24 17:52 ` P J P
2017-11-24 17:52 ` [Qemu-devel] [PATCH v2 2/2] tests: add test to check VirtQueue object P J P
2017-11-24 17:59 ` [Qemu-devel] [PATCH v2 0/2] check VirtiQueue Vring objects no-reply
2 siblings, 0 replies; 4+ messages in thread
From: P J P @ 2017-11-24 17:52 UTC (permalink / raw)
To: Qemu Developers
Cc: Stefan Hajnoczi, zhangboxian, Paolo Bonzini, Cornelia Huck,
Prasad J Pandit
From: Prasad J Pandit <pjp@fedoraproject.org>
An user could attempt to use an uninitialised VirtQueue object
or unset Vring.align leading to a arithmetic exception. Add check
to avoid it.
Reported-by: Zhangboxian <zhangboxian@huawei.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
hw/virtio/virtio.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
Update: add vring.num check to virtio_queue_set_rings
-> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg04549.html
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5884ce3480..f4bfc23344 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -182,7 +182,7 @@ void virtio_queue_update_rings(VirtIODevice *vdev, int n)
{
VRing *vring = &vdev->vq[n].vring;
- if (!vring->desc) {
+ if (!vdev->vq[n].vring.num || !vring->desc || !vring->align) {
/* not yet setup -> nothing to do */
return;
}
@@ -1414,6 +1414,9 @@ void virtio_config_modern_writel(VirtIODevice *vdev,
void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr)
{
+ if (!vdev->vq[n].vring.num) {
+ return;
+ }
vdev->vq[n].vring.desc = addr;
virtio_queue_update_rings(vdev, n);
}
@@ -1426,6 +1429,9 @@ hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n)
void virtio_queue_set_rings(VirtIODevice *vdev, int n, hwaddr desc,
hwaddr avail, hwaddr used)
{
+ if(!vdev->vq[n].vring.num || !desc || !vdev->vq[n].vring.align) {
+ return;
+ }
vdev->vq[n].vring.desc = desc;
vdev->vq[n].vring.avail = avail;
vdev->vq[n].vring.used = used;
@@ -1494,8 +1500,10 @@ void virtio_queue_set_align(VirtIODevice *vdev, int n, int align)
*/
assert(k->has_variable_vring_alignment);
- vdev->vq[n].vring.align = align;
- virtio_queue_update_rings(vdev, n);
+ if (align) {
+ vdev->vq[n].vring.align = align;
+ virtio_queue_update_rings(vdev, n);
+ }
}
static bool virtio_queue_notify_aio_vq(VirtQueue *vq)
--
2.13.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] tests: add test to check VirtQueue object
2017-11-24 17:52 [Qemu-devel] [PATCH v2 0/2] check VirtiQueue Vring objects P J P
2017-11-24 17:52 ` [Qemu-devel] [PATCH v2 1/2] virtio: check VirtQueue Vring object is set P J P
@ 2017-11-24 17:52 ` P J P
2017-11-24 17:59 ` [Qemu-devel] [PATCH v2 0/2] check VirtiQueue Vring objects no-reply
2 siblings, 0 replies; 4+ messages in thread
From: P J P @ 2017-11-24 17:52 UTC (permalink / raw)
To: Qemu Developers
Cc: Stefan Hajnoczi, zhangboxian, Paolo Bonzini, Cornelia Huck,
Prasad J Pandit
From: Prasad J Pandit <pjp@fedoraproject.org>
An uninitialised VirtQueue object or one with Vring.align field
set to zero(0) could lead to arithmetic exceptions. Add a unit
test to validate it.
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
tests/virtio-blk-test.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
Update: rename the test and add doc comments
-> https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg04578.html
diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index e6fb9bac87..45f368dcd9 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -674,6 +674,30 @@ static void pci_hotplug(void)
qtest_shutdown(qs);
}
+/*
+ * Check that setting the vring addr on a non-existent virtqueue does
+ * not crash.
+ */
+static void test_nonexistent_virtqueue(void)
+{
+ QPCIBar bar0;
+ QOSState *qs;
+ QPCIDevice *dev;
+
+ qs = pci_test_start();
+ dev = qpci_device_find(qs->pcibus, QPCI_DEVFN(4, 0));
+ g_assert(dev != NULL);
+
+ qpci_device_enable(dev);
+ bar0 = qpci_iomap(dev, 0, NULL);
+
+ qpci_io_writeb(dev, bar0, VIRTIO_PCI_QUEUE_SEL, 2);
+ qpci_io_writel(dev, bar0, VIRTIO_PCI_QUEUE_PFN, 1);
+
+ g_free(dev);
+ qtest_shutdown(qs);
+}
+
static void mmio_basic(void)
{
QVirtioMMIODevice *dev;
@@ -724,6 +748,7 @@ int main(int argc, char **argv)
qtest_add_func("/virtio/blk/pci/basic", pci_basic);
qtest_add_func("/virtio/blk/pci/indirect", pci_indirect);
qtest_add_func("/virtio/blk/pci/config", pci_config);
+ qtest_add_func("/virtio/blk/pci/nxvirtq", test_nonexistent_virtqueue);
if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
qtest_add_func("/virtio/blk/pci/msix", pci_msix);
qtest_add_func("/virtio/blk/pci/idx", pci_idx);
--
2.13.6
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/2] check VirtiQueue Vring objects
2017-11-24 17:52 [Qemu-devel] [PATCH v2 0/2] check VirtiQueue Vring objects P J P
2017-11-24 17:52 ` [Qemu-devel] [PATCH v2 1/2] virtio: check VirtQueue Vring object is set P J P
2017-11-24 17:52 ` [Qemu-devel] [PATCH v2 2/2] tests: add test to check VirtQueue object P J P
@ 2017-11-24 17:59 ` no-reply
2 siblings, 0 replies; 4+ messages in thread
From: no-reply @ 2017-11-24 17:59 UTC (permalink / raw)
To: ppandit; +Cc: famz, qemu-devel, pbonzini, cohuck, zhangboxian, stefanha, pjp
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [Qemu-devel] [PATCH v2 0/2] check VirtiQueue Vring objects
Type: series
Message-id: 20171124175211.2234-1-ppandit@redhat.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20171124175211.2234-1-ppandit@redhat.com -> patchew/20171124175211.2234-1-ppandit@redhat.com
Switched to a new branch 'test'
c81077c585 tests: add test to check VirtQueue object
65c6440326 virtio: check VirtQueue Vring object is set
=== OUTPUT BEGIN ===
Checking PATCH 1/2: virtio: check VirtQueue Vring object is set...
ERROR: space required before the open parenthesis '('
#41: FILE: hw/virtio/virtio.c:1432:
+ if(!vdev->vq[n].vring.num || !desc || !vdev->vq[n].vring.align) {
total: 1 errors, 0 warnings, 38 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/2: tests: add test to check VirtQueue object...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
^ permalink raw reply [flat|nested] 4+ messages in thread