mbox series

[0/4] esp: fix asserts/segfaults discovered by fuzzer

Message ID 20210316233024.13560-1-mark.cave-ayland@ilande.co.uk
Headers show
Series esp: fix asserts/segfaults discovered by fuzzer | expand

Message

Mark Cave-Ayland March 16, 2021, 11:30 p.m. UTC
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-ayland@ilande.co.uk>


Mark Cave-Ayland (4):
  esp: don't underflow cmdfifo if no message out/command data is present
  esp: don't overflow cmdfifo if TC is larger than the cmdfifo size
  esp: ensure cmdfifo is not empty and current_dev is non-NULL
  esp: always check current_req is not NULL before use in DMA callbacks

 hw/scsi/esp.c | 56 +++++++++++++++++++++++++++++++++------------------
 1 file changed, 36 insertions(+), 20 deletions(-)

Comments

Philippe Mathieu-Daudé March 17, 2021, 12:20 a.m. UTC | #1
+Laurent for 1 & 4.

On 3/17/21 12:30 AM, 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-ayland@ilande.co.uk>
> 
> 
> Mark Cave-Ayland (4):
>   esp: don't underflow cmdfifo if no message out/command data is present
>   esp: don't overflow cmdfifo if TC is larger than the cmdfifo size
>   esp: ensure cmdfifo is not empty and current_dev is non-NULL
>   esp: always check current_req is not NULL before use in DMA callbacks
> 
>  hw/scsi/esp.c | 56 +++++++++++++++++++++++++++++++++------------------
>  1 file changed, 36 insertions(+), 20 deletions(-)
>
Paolo Bonzini March 17, 2021, 7:59 a.m. UTC | #2
On 17/03/21 00:30, 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-ayland@ilande.co.uk>

They are certainly something that we can fix for 6.0.  However, please 
include the testcases even if they are ugly, they can be cleaned up 
later or (if never cleaned up) they still count as regression tests.

Paolo