qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Michael Roth <mdroth@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org
Cc: John Snow <jsnow@redhat.com>,
	qemu-stable@nongnu.org, Alexander Popov <alex.popov@linux.com>
Subject: [PATCH 14/78] tests/ide-test: Create a single unit-test covering more PRDT cases
Date: Tue, 16 Jun 2020 09:14:43 -0500	[thread overview]
Message-ID: <20200616141547.24664-15-mdroth@linux.vnet.ibm.com> (raw)
In-Reply-To: <20200616141547.24664-1-mdroth@linux.vnet.ibm.com>

From: Alexander Popov <alex.popov@linux.com>

Fuzzing the Linux kernel with syzkaller allowed to find how to crash qemu
using a special SCSI_IOCTL_SEND_COMMAND. It hits the assertion in
ide_dma_cb() introduced in the commit a718978ed58a in July 2015.
Currently this bug is not reproduced by the unit tests.

Let's improve the ide-test to cover more PRDT cases including one
that causes this particular qemu crash.

The test is developed according to the Programming Interface for
Bus Master IDE Controller (Revision 1.0 5/16/94).

Signed-off-by: Alexander Popov <alex.popov@linux.com>
Message-id: 20191223175117.508990-3-alex.popov@linux.com
Signed-off-by: John Snow <jsnow@redhat.com>
(cherry picked from commit 59805ae92dfe4f67105e36b539d567caec4f8304)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 tests/ide-test.c | 174 ++++++++++++++++++++---------------------------
 1 file changed, 74 insertions(+), 100 deletions(-)

diff --git a/tests/ide-test.c b/tests/ide-test.c
index 0277e7d5a9..5cfd97f915 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -445,104 +445,81 @@ static void test_bmdma_trim(void)
     test_bmdma_teardown(qts);
 }
 
-static void test_bmdma_short_prdt(void)
-{
-    QTestState *qts;
-    QPCIDevice *dev;
-    QPCIBar bmdma_bar, ide_bar;
-    uint8_t status;
-
-    PrdtEntry prdt[] = {
-        {
-            .addr = 0,
-            .size = cpu_to_le32(0x10 | PRDT_EOT),
-        },
-    };
-
-    qts = test_bmdma_setup();
-
-    dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
-
-    /* Normal request */
-    status = send_dma_request(qts, CMD_READ_DMA, 0, 1,
-                              prdt, ARRAY_SIZE(prdt), NULL);
-    g_assert_cmphex(status, ==, 0);
-    assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
-
-    /* Abort the request before it completes */
-    status = send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0, 1,
-                              prdt, ARRAY_SIZE(prdt), NULL);
-    g_assert_cmphex(status, ==, 0);
-    assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
-    free_pci_device(dev);
-    test_bmdma_teardown(qts);
-}
-
-static void test_bmdma_one_sector_short_prdt(void)
-{
-    QTestState *qts;
-    QPCIDevice *dev;
-    QPCIBar bmdma_bar, ide_bar;
-    uint8_t status;
-
-    /* Read 2 sectors but only give 1 sector in PRDT */
-    PrdtEntry prdt[] = {
-        {
-            .addr = 0,
-            .size = cpu_to_le32(0x200 | PRDT_EOT),
-        },
-    };
-
-    qts = test_bmdma_setup();
-
-    dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
-
-    /* Normal request */
-    status = send_dma_request(qts, CMD_READ_DMA, 0, 2,
-                              prdt, ARRAY_SIZE(prdt), NULL);
-    g_assert_cmphex(status, ==, 0);
-    assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
-
-    /* Abort the request before it completes */
-    status = send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0, 2,
-                              prdt, ARRAY_SIZE(prdt), NULL);
-    g_assert_cmphex(status, ==, 0);
-    assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
-    free_pci_device(dev);
-    test_bmdma_teardown(qts);
-}
-
-static void test_bmdma_long_prdt(void)
+/*
+ * This test is developed according to the Programming Interface for
+ * Bus Master IDE Controller (Revision 1.0 5/16/94)
+ */
+static void test_bmdma_various_prdts(void)
 {
-    QTestState *qts;
-    QPCIDevice *dev;
-    QPCIBar bmdma_bar, ide_bar;
-    uint8_t status;
-
-    PrdtEntry prdt[] = {
-        {
-            .addr = 0,
-            .size = cpu_to_le32(0x1000 | PRDT_EOT),
-        },
-    };
-
-    qts = test_bmdma_setup();
-
-    dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
-
-    /* Normal request */
-    status = send_dma_request(qts, CMD_READ_DMA, 0, 1,
-                              prdt, ARRAY_SIZE(prdt), NULL);
-    g_assert_cmphex(status, ==, BM_STS_ACTIVE | BM_STS_INTR);
-    assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
+    int sectors = 0;
+    uint32_t size = 0;
+
+    for (sectors = 1; sectors <= 256; sectors *= 2) {
+        QTestState *qts = NULL;
+        QPCIDevice *dev = NULL;
+        QPCIBar bmdma_bar, ide_bar;
+
+        qts = test_bmdma_setup();
+        dev = get_pci_device(qts, &bmdma_bar, &ide_bar);
+
+        for (size = 0; size < 65536; size += 256) {
+            uint32_t req_size = sectors * 512;
+            uint32_t prd_size = size & 0xfffe; /* bit 0 is always set to 0 */
+            uint8_t ret = 0;
+            uint8_t req_status = 0;
+            uint8_t abort_req_status = 0;
+            PrdtEntry prdt[] = {
+                {
+                    .addr = 0,
+                    .size = cpu_to_le32(size | PRDT_EOT),
+                },
+            };
+
+            /* A value of zero in PRD size indicates 64K */
+            if (prd_size == 0) {
+                prd_size = 65536;
+            }
+
+            /*
+             * 1. If PRDs specified a smaller size than the IDE transfer
+             * size, then the Interrupt and Active bits in the Controller
+             * status register are not set (Error Condition).
+             *
+             * 2. If the size of the physical memory regions was equal to
+             * the IDE device transfer size, the Interrupt bit in the
+             * Controller status register is set to 1, Active bit is set to 0.
+             *
+             * 3. If PRDs specified a larger size than the IDE transfer size,
+             * the Interrupt and Active bits in the Controller status register
+             * are both set to 1.
+             */
+            if (prd_size < req_size) {
+                req_status = 0;
+                abort_req_status = 0;
+            } else if (prd_size == req_size) {
+                req_status = BM_STS_INTR;
+                abort_req_status = BM_STS_INTR;
+            } else {
+                req_status = BM_STS_ACTIVE | BM_STS_INTR;
+                abort_req_status = BM_STS_INTR;
+            }
+
+            /* Test the request */
+            ret = send_dma_request(qts, CMD_READ_DMA, 0, sectors,
+                                   prdt, ARRAY_SIZE(prdt), NULL);
+            g_assert_cmphex(ret, ==, req_status);
+            assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
+
+            /* Now test aborting the same request */
+            ret = send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0,
+                                   sectors, prdt, ARRAY_SIZE(prdt), NULL);
+            g_assert_cmphex(ret, ==, abort_req_status);
+            assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
+        }
 
-    /* Abort the request before it completes */
-    status = send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0, 1,
-                              prdt, ARRAY_SIZE(prdt), NULL);
-    g_assert_cmphex(status, ==, BM_STS_INTR);
-    assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), DF | ERR);
-    free_pci_device(dev);
-    test_bmdma_teardown(qts);
+        free_pci_device(dev);
+        test_bmdma_teardown(qts);
+    }
 }
 
 static void test_bmdma_no_busmaster(void)
@@ -1066,10 +1043,7 @@ int main(int argc, char **argv)
 
     qtest_add_func("/ide/bmdma/simple_rw", test_bmdma_simple_rw);
     qtest_add_func("/ide/bmdma/trim", test_bmdma_trim);
-    qtest_add_func("/ide/bmdma/short_prdt", test_bmdma_short_prdt);
-    qtest_add_func("/ide/bmdma/one_sector_short_prdt",
-                   test_bmdma_one_sector_short_prdt);
-    qtest_add_func("/ide/bmdma/long_prdt", test_bmdma_long_prdt);
+    qtest_add_func("/ide/bmdma/various_prdts", test_bmdma_various_prdts);
     qtest_add_func("/ide/bmdma/no_busmaster", test_bmdma_no_busmaster);
 
     qtest_add_func("/ide/flush", test_flush);
-- 
2.17.1



  parent reply	other threads:[~2020-06-16 14:19 UTC|newest]

Thread overview: 92+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-16 14:14 [PATCH 00/78] Patch Round-up for stable 4.2.1, freeze on 2020-06-22 Michael Roth
2020-06-16 14:14 ` [PATCH 01/78] block/nbd: extract the common cleanup code Michael Roth
2020-06-16 14:14 ` [PATCH 02/78] block/nbd: fix memory leak in nbd_open() Michael Roth
2020-06-16 14:14 ` [PATCH 03/78] i386: Resolve CPU models to v1 by default Michael Roth
2020-06-16 14:14 ` [PATCH 04/78] qapi: better document NVMe blockdev @device parameter Michael Roth
2020-06-16 14:14 ` [PATCH 05/78] target/arm: ensure we use current exception state after SCR update Michael Roth
2020-06-16 14:14 ` [PATCH 06/78] block: Activate recursively even for already active nodes Michael Roth
2020-06-16 14:14 ` [PATCH 07/78] virtio-blk: fix out-of-bounds access to bitmap in notify_guest_bh Michael Roth
2020-06-16 14:14 ` [PATCH 08/78] numa: remove not needed check Michael Roth
2020-06-16 14:14 ` [PATCH 09/78] numa: properly check if numa is supported Michael Roth
2020-06-16 14:14 ` [PATCH 10/78] backup-top: Begin drain earlier Michael Roth
2020-06-16 14:14 ` [PATCH 11/78] arm/arm-powerctl: set NSACR.{CP11, CP10} bits in arm_set_cpu_on() Michael Roth
2020-06-16 14:14 ` [PATCH 12/78] arm/arm-powerctl: rebuild hflags after setting CP15 " Michael Roth
2020-06-16 14:14 ` [PATCH 13/78] hw/i386/pc: fix regression in parsing vga cmdline parameter Michael Roth
2020-06-16 14:14 ` Michael Roth [this message]
2020-06-16 14:14 ` [PATCH 15/78] ide: Fix incorrect handling of some PRDTs in ide_dma_cb() Michael Roth
2020-06-16 14:14 ` [PATCH 16/78] target/arm: Set ISSIs16Bit in make_issinfo Michael Roth
2020-06-16 14:14 ` [PATCH 17/78] virtio: update queue size on guest write Michael Roth
2020-06-16 14:14 ` [PATCH 18/78] virtio-mmio: " Michael Roth
2020-06-16 14:14 ` [PATCH 19/78] virtio: add ability to delete vq through a pointer Michael Roth
2020-06-16 14:14 ` [PATCH 20/78] virtio: make virtio_delete_queue idempotent Michael Roth
2020-06-16 14:14 ` [PATCH 21/78] virtio: reset region cache when on queue deletion Michael Roth
2020-06-16 14:14 ` [PATCH 22/78] virtio-net: delete also control queue when TX/RX deleted Michael Roth
2020-06-16 14:14 ` [PATCH 23/78] intel_iommu: a fix to vtd_find_as_from_bus_num() Michael Roth
2020-06-16 14:14 ` [PATCH 24/78] intel_iommu: add present bit check for pasid table entries Michael Roth
2020-06-16 14:14 ` [PATCH 25/78] vfio/pci: Don't remove irqchip notifier if not registered Michael Roth
2020-06-16 14:14 ` [PATCH 26/78] qcow2-bitmaps: fix qcow2_can_store_new_dirty_bitmap Michael Roth
2020-06-16 14:14 ` [PATCH 27/78] dp8393x: Mask EOL bit from descriptor addresses Michael Roth
2020-06-16 14:14 ` [PATCH 28/78] dp8393x: Always use 32-bit accesses Michael Roth
2020-06-16 14:14 ` [PATCH 29/78] dp8393x: Clean up endianness hacks Michael Roth
2020-06-16 14:14 ` [PATCH 30/78] dp8393x: Have dp8393x_receive() return the packet size Michael Roth
2020-06-16 14:15 ` [PATCH 31/78] dp8393x: Update LLFA and CRDA registers from rx descriptor Michael Roth
2020-06-16 14:15 ` [PATCH 32/78] dp8393x: Clear RRRA command register bit only when appropriate Michael Roth
2020-06-16 14:15 ` [PATCH 33/78] dp8393x: Implement packet size limit and RBAE interrupt Michael Roth
2020-06-16 14:15 ` [PATCH 34/78] dp8393x: Don't clobber packet checksum Michael Roth
2020-06-16 14:15 ` [PATCH 35/78] dp8393x: Use long-word-aligned RRA pointers in 32-bit mode Michael Roth
2020-06-16 14:15 ` [PATCH 36/78] dp8393x: Pad frames to word or long word boundary Michael Roth
2020-06-16 14:15 ` [PATCH 37/78] dp8393x: Clear descriptor in_use field to release packet Michael Roth
2020-06-16 14:15 ` [PATCH 38/78] dp8393x: Always update RRA pointers and sequence numbers Michael Roth
2020-06-16 14:15 ` [PATCH 39/78] dp8393x: Don't reset Silicon Revision register Michael Roth
2020-06-16 14:15 ` [PATCH 40/78] dp8393x: Don't stop reception upon RBE interrupt assertion Michael Roth
2020-06-16 14:15 ` [PATCH 41/78] s390/sclp: improve special wait psw logic Michael Roth
2020-06-16 14:15 ` [PATCH 42/78] plugins/core: add missing break in cb_to_tcg_flags Michael Roth
2020-06-16 14:15 ` [PATCH 43/78] tcg: save vaddr temp for plugin usage Michael Roth
2020-06-16 14:15 ` [PATCH 44/78] qcow2: update_refcount(): Reset old_table_index after qcow2_cache_put() Michael Roth
2020-06-16 14:15 ` [PATCH 45/78] qcow2: Fix qcow2_alloc_cluster_abort() for external data file Michael Roth
2020-06-16 14:15 ` [PATCH 46/78] iotests: Test copy offloading with " Michael Roth
2020-06-16 14:15 ` [PATCH 47/78] qcow2: Fix alloc_cluster_abort() for pre-existing clusters Michael Roth
2020-06-16 14:15 ` [PATCH 48/78] iotests/026: Test EIO on preallocated zero cluster Michael Roth
2020-06-16 14:15 ` [PATCH 49/78] iotests/026: Test EIO on allocation in a data-file Michael Roth
2020-06-16 14:15 ` [PATCH 50/78] virtio: gracefully handle invalid region caches Michael Roth
2020-06-16 14:15 ` [PATCH 51/78] scsi/qemu-pr-helper: Fix out-of-bounds access to trnptid_list[] Michael Roth
2020-06-16 14:15 ` [PATCH 52/78] block/qcow2-threads: fix qcow2_decompress Michael Roth
2020-06-16 14:15 ` [PATCH 53/78] job: refactor progress to separate object Michael Roth
2020-06-16 14:15 ` [PATCH 54/78] block/block-copy: fix progress calculation Michael Roth
2020-06-16 14:15 ` [PATCH 55/78] target/ppc: Fix rlwinm on ppc64 Michael Roth
2020-06-16 14:15 ` [PATCH 56/78] block/io: fix bdrv_co_do_copy_on_readv Michael Roth
2020-06-16 14:15 ` [PATCH 57/78] compat: disable edid on correct virtio-gpu device Michael Roth
2020-06-16 14:15 ` [PATCH 58/78] qga: Installer: Wait for installation to finish Michael Roth
2020-06-16 14:15 ` [PATCH 59/78] qga-win: Handle VSS_E_PROVIDER_ALREADY_REGISTERED error Michael Roth
2020-06-16 14:15 ` [PATCH 60/78] qga-win: prevent crash when executing guest-file-read with large count Michael Roth
2020-06-16 14:15 ` [PATCH 61/78] qga: Fix undefined C behavior Michael Roth
2020-06-16 14:15 ` [PATCH 62/78] qemu-ga: document vsock-listen in the man page Michael Roth
2020-06-16 14:15 ` [PATCH 63/78] hw/i386/amd_iommu.c: Fix corruption of log events passed to guest Michael Roth
2020-06-16 14:15 ` [PATCH 64/78] tcg/i386: Fix INDEX_op_dup2_vec Michael Roth
2020-06-16 14:15 ` [PATCH 65/78] dump: Fix writing of ELF section Michael Roth
2020-06-16 14:15 ` [PATCH 66/78] xen-block: Fix double qlist remove and request leak Michael Roth
2020-06-16 14:15 ` [PATCH 67/78] vhost-user-gpu: Release memory returned by vu_queue_pop() with free() Michael Roth
2020-06-16 14:15 ` [PATCH 68/78] target/ppc: Fix mtmsr(d) L=1 variant that loses interrupts Michael Roth
2020-06-16 14:15 ` [PATCH 69/78] hostmem: don't use mbind() if host-nodes is empty Michael Roth
2020-06-16 14:15 ` [PATCH 70/78] target/arm: Clear tail in gvec_fmul_idx_*, gvec_fmla_idx_* Michael Roth
2020-06-16 14:15 ` [PATCH 71/78] qemu-nbd: Close inherited stderr Michael Roth
2020-06-16 14:15 ` [PATCH 72/78] 9p: Lock directory streams with a CoMutex Michael Roth
2020-06-16 15:14   ` Greg Kurz
2020-06-16 16:09     ` Christian Schoenebeck
2020-06-16 16:41       ` Greg Kurz
2020-06-16 22:46         ` Michael Roth
2020-06-18 13:47           ` Christian Schoenebeck
2020-06-16 14:15 ` [PATCH 73/78] net: Do not include a newline in the id of -nic devices Michael Roth
2020-06-16 14:15 ` [PATCH 74/78] nbd/server: Avoid long error message assertions CVE-2020-10761 Michael Roth
2020-06-16 14:15 ` [PATCH 75/78] virtio-balloon: fix free page hinting without an iothread Michael Roth
2020-06-16 14:15 ` [PATCH 76/78] virtio-balloon: fix free page hinting check on unrealize Michael Roth
2020-06-16 14:15 ` [PATCH 77/78] virtio-balloon: unref the iothread when unrealizing Michael Roth
2020-06-16 14:15 ` [PATCH 78/78] block: Call attention to truncation of long NBD exports Michael Roth
2020-06-17 14:39 ` [PATCH 00/78] Patch Round-up for stable 4.2.1, freeze on 2020-06-22 Cole Robinson
2020-06-17 15:54 ` Liam Merwick
2020-06-17 20:02 ` Karl Heubaum
2020-06-20  0:14 ` Finn Thain
2020-06-20  3:39   ` Finn Thain
2020-06-22 20:31     ` Michael Roth
2020-06-20 21:44 ` Bruce Rogers
2020-06-22 20:26 ` Michael Roth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200616141547.24664-15-mdroth@linux.vnet.ibm.com \
    --to=mdroth@linux.vnet.ibm.com \
    --cc=alex.popov@linux.com \
    --cc=jsnow@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).