From: Alexander Popov <alex.popov@linux.com>
To: "Michael S . Tsirkin" <mst@redhat.com>,
John Snow <jsnow@redhat.com>,
qemu-block@nongnu.org, qemu-devel@nongnu.org,
qemu-stable@nongnu.org, pmatouse@redhat.com,
sstabellini@kernel.org, mdroth@linux.vnet.ibm.com,
pjp@redhat.com, Paolo Bonzini <pbonzini@redhat.com>,
Andrea Arcangeli <aarcange@redhat.com>,
Kashyap Chamarthy <kashyap.cv@gmail.com>,
Darren Kenny <darren.kenny@oracle.com>,
Kevin Wolf <kwolf@redhat.com>, Thomas Huth <thuth@redhat.com>,
Laurent Vivier <lvivier@redhat.com>,
Alexander Popov <alex.popov@linux.com>
Subject: [PATCH v2 1/2] tests/ide-test: Create a single unit-test covering more PRDT cases
Date: Mon, 16 Dec 2019 21:14:04 +0300 [thread overview]
Message-ID: <20191216181405.462292-1-alex.popov@linux.com> (raw)
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>
---
tests/ide-test.c | 137 +++++++++++++++++++++--------------------------
1 file changed, 61 insertions(+), 76 deletions(-)
diff --git a/tests/ide-test.c b/tests/ide-test.c
index 0277e7d5a9..f042d8a700 100644
--- a/tests/ide-test.c
+++ b/tests/ide-test.c
@@ -445,7 +445,8 @@ static void test_bmdma_trim(void)
test_bmdma_teardown(qts);
}
-static void test_bmdma_short_prdt(void)
+static void test_bmdma_prdt(uint32_t prdt_size, int nb_sectors,
+ uint8_t req_status, uint8_t abort_req_status)
{
QTestState *qts;
QPCIDevice *dev;
@@ -455,94 +456,81 @@ static void test_bmdma_short_prdt(void)
PrdtEntry prdt[] = {
{
.addr = 0,
- .size = cpu_to_le32(0x10 | PRDT_EOT),
+ .size = cpu_to_le32(prdt_size | 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,
+ /* Test the request */
+ status = send_dma_request(qts, CMD_READ_DMA, 0, nb_sectors,
prdt, ARRAY_SIZE(prdt), NULL);
- g_assert_cmphex(status, ==, 0);
+ g_assert_cmphex(status, ==, 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, ==, 0);
+ /* Now test aborting the same request */
+ status = send_dma_request(qts, CMD_READ_DMA | CMDF_ABORT, 0,
+ nb_sectors, prdt, ARRAY_SIZE(prdt), NULL);
+ g_assert_cmphex(status, ==, abort_req_status);
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);
+ uint32_t size = 0;
+ uint32_t prd_size = 0;
+ int req_sectors = 0;
+ uint32_t req_size = 0;
+ uint8_t s1 = 0, s2 = 0;
+
+ for (size = 0; size < 65536; size += 256) {
+ /*
+ * Two bytes specify the count of the region in bytes.
+ * The bit 0 is always set to 0.
+ * A value of zero in these two bytes indicates 64K.
+ */
+ prd_size = size & 0xfffe;
+ if (prd_size == 0) {
+ prd_size = 65536;
+ }
- /* 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);
+ for (req_sectors = 1; req_sectors <= 256; req_sectors *= 2) {
+ req_size = req_sectors * 512;
+
+ /*
+ * 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) {
+ s1 = 0;
+ s2 = 0;
+ } else if (prd_size == req_size) {
+ s1 = BM_STS_INTR;
+ s2 = BM_STS_INTR;
+ } else {
+ s1 = BM_STS_ACTIVE | BM_STS_INTR;
+ s2 = BM_STS_INTR;
+ }
+ test_bmdma_prdt(size, req_sectors, s1, s2);
+ }
+ }
}
static void test_bmdma_no_busmaster(void)
@@ -1066,10 +1054,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.23.0
next reply other threads:[~2019-12-16 18:18 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-16 18:14 Alexander Popov [this message]
2019-12-16 18:14 ` [PATCH v2 2/2] ide: Fix incorrect handling of some PRDTs in ide_dma_cb() Alexander Popov
2019-12-19 15:01 ` Kevin Wolf
2019-12-19 15:46 ` Alexander Popov
2019-12-24 0:20 ` John Snow
2019-12-24 4:18 ` Alexander Popov
2020-01-01 22:45 ` Alexander Popov
2019-12-19 15:12 ` [PATCH v2 1/2] tests/ide-test: Create a single unit-test covering more PRDT cases Kevin Wolf
2019-12-19 15:33 ` Alexander Popov
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=20191216181405.462292-1-alex.popov@linux.com \
--to=alex.popov@linux.com \
--cc=aarcange@redhat.com \
--cc=darren.kenny@oracle.com \
--cc=jsnow@redhat.com \
--cc=kashyap.cv@gmail.com \
--cc=kwolf@redhat.com \
--cc=lvivier@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=pjp@redhat.com \
--cc=pmatouse@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-stable@nongnu.org \
--cc=sstabellini@kernel.org \
--cc=thuth@redhat.com \
/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).