On 210401 0849, Mark Cave-Ayland wrote: > Recently there have been a number of issues raised on Launchpad as a result of > fuzzing the am53c974 (ESP) device. I spent some time over the past couple of > days checking to see if anything had improved since my last patchset: from > what I can tell the issues are still present, but the cmdfifo related failures > now assert rather than corrupting memory. > > This patchset applied to master passes my local tests using the qtest fuzz > test > cases added by Alexander for the following Launchpad bugs: > > https://bugs.launchpad.net/qemu/+bug/1919035 > https://bugs.launchpad.net/qemu/+bug/1919036 > https://bugs.launchpad.net/qemu/+bug/1910723 > https://bugs.launchpad.net/qemu/+bug/1909247 > > I'm posting this now just before soft freeze since I see that some of the > issues > have recently been allocated CVEs and so it could be argued that even though > they have existed for some time, it is worth fixing them for 6.0. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> > > v3: > - Rebase onto master > - Rearrange patch ordering (move patch 5 to the front) to help reduce > cross-talk > between the regression tests > - Introduce patch 2 to remove unnecessary FIFO usage > - Introduce patches 3-4 to consolidate esp_fifo_pop()/esp_fifo_push() wrapper > functions to avoid having to introduce 2 variants of esp_fifo_pop_buf() > - Introduce esp_fifo_pop_buf() in patch 5 to prevent callers from overflowing > the array used to model the FIFO > - Introduce patch 10 to clarify cancellation logic should all occur in the > .cancel > SCSI callback rather than at the site of the caller > - Add extra qtests in patch 11 to cover addition test cases provided on LP >
Hi Mark, I applied this and ran through the whole fuzzer corpus, and all I'm seeing are just a few assertion failures: handle_satn_stop -> get_cmd -> util/fifo8.c:43:5 and hw/scsi/esp.c:790:5 Tested-by: Alexander Bulekov <alx...@bu.edu> Thank you -Alex > v2: > - Add Alexander's R-B tag for patch 2 and Phil's R-B for patch 3 > - Add patch 4 for additional testcase provided in Alexander's patch 1 comment > - Move current_req NULL checks forward in DMA functions (fixes ASAN bug > reported > at https://bugs.launchpad.net/qemu/+bug/1909247/comments/6) in patch 3 > - Add qtest for am53c974 containing a basic set of regression tests using the > automatic test cases generated by the fuzzer as requested by Paolo > > > Mark Cave-Ayland (11): > esp: always check current_req is not NULL before use in DMA callbacks > esp: rework write_response() to avoid using the FIFO for DMA > transactions > esp: consolidate esp_cmdfifo_push() into esp_fifo_push() > esp: consolidate esp_cmdfifo_pop() into esp_fifo_pop() > esp: introduce esp_fifo_pop_buf() and use it instead of > fifo8_pop_buf() > esp: ensure cmdfifo is not empty and current_dev is non-NULL > esp: don't underflow cmdfifo in do_cmd() > esp: don't overflow cmdfifo in get_cmd() > esp: don't overflow cmdfifo if TC is larger than the cmdfifo size > esp: don't reset async_len directly in esp_select() if cancelling > request > tests/qtest: add tests for am53c974 device > > MAINTAINERS | 1 + > hw/scsi/esp.c | 116 ++++++++++--------- > tests/qtest/am53c974-test.c | 216 ++++++++++++++++++++++++++++++++++++ > tests/qtest/meson.build | 1 + > 4 files changed, 282 insertions(+), 52 deletions(-) > create mode 100644 tests/qtest/am53c974-test.c > > -- > 2.20.1 >