Re: [PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()

[Thomas Huth <thuth@redhat.com>]

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 <alxndr@bu.edu>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  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