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.po...@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