On 01/12/2020 16.13, Philippe Mathieu-Daudé wrote:
> cdb_len can not be zero... (or less than 6) here, else we have a
> out-of-bound read first in scsi_cdb_length():
>
> 71 int scsi_cdb_length(uint8_t *buf)
> 72 {
> 73 int cdb_len;
> 74
> 75 switch (buf[0] >> 5) {
> 76 case 0:
> 77 cdb_len = 6;
> 78 break;
>
> Then another out-of-bound read when the size returned by
> scsi_cdb_length() is used.
>
> Add a reproducer which triggers:
>
> $ make check-qtest-x86_64
> Running test qtest-x86_64/fuzz-test
> qemu-system-x86_64: hw/scsi/megasas.c:1679: megasas_handle_scsi: Assertion
> `cdb_len > 0 && scsi_cdb_length(cdb) >= cdb_len' failed.
> tests/qtest/libqtest.c:181: kill_qemu() detected QEMU death from signal 6
> (Aborted) (core dumped)
> ERROR qtest-x86_64/fuzz-test - too few tests run (expected 1, got 0)
>
> Inspired-by: Alexander Bulekov <[email protected]>
> Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
> ---
> hw/scsi/megasas.c | 1 +
> tests/qtest/fuzz-test.c | 196 ++++++++++++++++++++++++++++++++++++++++
For the final version, I think you should add the fuzz-test after the fix
(patch 3) ... otherwise this breaks bisection later...
Thomas
