[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_hand
From: |
Thomas Huth |
Subject: |
Re: [PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi() |
Date: |
Tue, 1 Dec 2020 17:01:46 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.1 |
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