diff mbox series

hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()

Message ID 20201201172250.208197-1-alxndr@bu.edu
State New
Headers show
Series hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi() | expand

Commit Message

Alexander Bulekov Dec. 1, 2020, 5:22 p.m. UTC 
From: Philippe Mathieu-Daudé <philmd@redhat.com>

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)

Signed-off-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---

Ran it through the minimizer - No more blobs.

 hw/scsi/megasas.c       |  1 +
 tests/qtest/fuzz-test.c | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)
diff mbox series

Patch

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 1a5fc5857d..28efd09411 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1676,6 +1676,7 @@  static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
     lun_id = cmd->frame->header.lun_id;
     cdb_len = cmd->frame->header.cdb_len;
 
+    assert(cdb_len > 0 && scsi_cdb_length(cdb) >= cdb_len);
     if (is_logical) {
         if (target_id >= MFI_MAX_LD || lun_id != 0) {
             trace_megasas_scsi_target_not_present(
diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-test.c
index 87b72307a5..31f90cfb4f 100644
--- a/tests/qtest/fuzz-test.c
+++ b/tests/qtest/fuzz-test.c
@@ -48,6 +48,23 @@  static void test_lp1878642_pci_bus_get_irq_level_assert(void)
     qtest_quit(s);
 }
 
+static void test_megasas_cdb_len_zero(void)
+{
+    QTestState *s;
+
+    s = qtest_init("-M pc -nodefaults "
+                   "-device megasas-gen2 -device scsi-cd,drive=null0 "
+                   "-blockdev driver=null-co,read-zeroes=on,node-name=null0");
+
+    qtest_outl(s, 0xcf8, 0x80001011);
+    qtest_outb(s, 0xcfc, 0xbb);
+    qtest_outl(s, 0xcf8, 0x80001002);
+    qtest_outl(s, 0xcfc, 0xf3ff2966);
+    qtest_writeb(s, 0x4600, 0x03);
+    qtest_outw(s, 0xbb40, 0x460b);
+    qtest_quit(s);
+}
+
 int main(int argc, char **argv)
 {
     const char *arch = qtest_get_arch();
@@ -59,6 +76,8 @@  int main(int argc, char **argv)
                        test_lp1878263_megasas_zero_iov_cnt);
         qtest_add_func("fuzz/test_lp1878642_pci_bus_get_irq_level_assert",
                        test_lp1878642_pci_bus_get_irq_level_assert);
+        qtest_add_func("fuzz/test_megasas_cdb_len_zero",
+                       test_megasas_cdb_len_zero);
     }
 
     return g_test_run();