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 | 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.23.0
Am 23.12.2019 um 18:51 hat Alexander Popov geschrieben:
> 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>
The time this test takes is much better now (~5s for me).
> +/*
> + * 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);
I'm wondering why the initialisation has to be inside the outer for
loop. I expected that moving it outside would further improve the speed.
But sure enough, doing that makes the test fail.
Did you have a look why this happens? I suppose we might be running out
of some resources in the qtest framework becasue each send_dma_request()
calls get_pci_device() again?
5 seconds isn't that bad, so this shouldn't block this series, but it's
still by far the slowest test in ide-test, so any improvement certainly
wouldn't hurt.
> + 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;
If you end up sending another version for some reason, I would also
consider renaming req_status, because reg_status already exists, which
looks almost the same. This confused me for a moment when reading the
code below.
Kevin
On 07.01.2020 10:44, Kevin Wolf wrote:
> Am 23.12.2019 um 18:51 hat Alexander Popov geschrieben:
>> 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>
>
> The time this test takes is much better now (~5s for me).
>
>> +/*
>> + * 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);
>
> I'm wondering why the initialisation has to be inside the outer for
> loop. I expected that moving it outside would further improve the speed.
> But sure enough, doing that makes the test fail.
Yes, that's why I came to the current solution.
> Did you have a look why this happens? I suppose we might be running out
> of some resources in the qtest framework becasue each send_dma_request()
> calls get_pci_device() again?
I've spent some time on investigating, but didn't succeed.
1. After several hundreds of send_dma_request() calls the following assertion in
that function fails:
assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), BSY | DRQ);
2. If I comment out this assertion, the test system proceeds but eventually stalls.
3. I tried to send the CMD_FLUSH_CACHE command to the device, it didn't help.
4. That behavior is not influenced by ide_dma_cb() code that I changed.
I guess it would be better if that effect is examined by somebody with more
knowledge about DMA and qtest.
> 5 seconds isn't that bad, so this shouldn't block this series, but it's
> still by far the slowest test in ide-test, so any improvement certainly
> wouldn't hurt.
Thanks for not making that mandatory. It would take me much more time.
>> + 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;
>
> If you end up sending another version for some reason, I would also
> consider renaming req_status, because reg_status already exists, which
> looks almost the same. This confused me for a moment when reading the
> code below.
Heh! Ok, let's wait for more reviews.
Best regards,
Alexander
Am 07.01.2020 um 23:39 hat Alexander Popov geschrieben: > > Did you have a look why this happens? I suppose we might be running out > > of some resources in the qtest framework becasue each send_dma_request() > > calls get_pci_device() again? > > I've spent some time on investigating, but didn't succeed. > > 1. After several hundreds of send_dma_request() calls the following assertion in > that function fails: > assert_bit_clear(qpci_io_readb(dev, ide_bar, reg_status), BSY | DRQ); > > 2. If I comment out this assertion, the test system proceeds but eventually stalls. > > 3. I tried to send the CMD_FLUSH_CACHE command to the device, it didn't help. > > 4. That behavior is not influenced by ide_dma_cb() code that I changed. > > I guess it would be better if that effect is examined by somebody with more > knowledge about DMA and qtest. > > > 5 seconds isn't that bad, so this shouldn't block this series, but it's > > still by far the slowest test in ide-test, so any improvement certainly > > wouldn't hurt. > > Thanks for not making that mandatory. It would take me much more time. Ok, don't bother then. I seem to remember that I ran into something similar some time ago and found out that it was related to some integer overflow, I think during the PCI BAR mapping. This might be the same. Maybe I'll have another look later. Kevin
© 2016 - 2026 Red Hat, Inc.