All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Laurent Vivier" <lvivier@redhat.com>,
	"Fam Zheng" <fam@euphon.net>, "Thomas Huth" <thuth@redhat.com>,
	qemu-block@nongnu.org, "Li Qiang" <liq3ea@163.com>,
	"Hannes Reinecke" <hare@suse.com>,
	"Alexander Bulekov" <alxndr@bu.edu>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Daniele Buono" <dbuono@linux.vnet.ibm.com>
Subject: [PATCH v2 2/4] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()
Date: Tue,  1 Dec 2020 20:10:24 +0100	[thread overview]
Message-ID: <20201201191026.4149955-3-philmd@redhat.com> (raw)
In-Reply-To: <20201201191026.4149955-1-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.

Figured out after reviewing:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg757937.html

And reproduced fuzzing:

  qemu-fuzz-i386: hw/scsi/megasas.c:1679: int megasas_handle_scsi(MegasasState *, MegasasCmd *, int):
  Assertion `len > 0 && cdb_len >= len' failed.
  ==1689590== ERROR: libFuzzer: deadly signal
      #8 0x7f7a5d918e75 in __assert_fail (/lib64/libc.so.6+0x34e75)
      #9 0x55a1b95cf6d4 in megasas_handle_scsi hw/scsi/megasas.c:1679:5
      #10 0x55a1b95cf6d4 in megasas_handle_frame hw/scsi/megasas.c:1975:24
      #11 0x55a1b95cf6d4 in megasas_mmio_write hw/scsi/megasas.c:2132:9
      #12 0x55a1b981972e in memory_region_write_accessor softmmu/memory.c:491:5
      #13 0x55a1b981972e in access_with_adjusted_size softmmu/memory.c:552:18
      #14 0x55a1b981972e in memory_region_dispatch_write softmmu/memory.c:1501:16
      #15 0x55a1b97f0ab0 in flatview_write_continue softmmu/physmem.c:2759:23
      #16 0x55a1b97ec3f2 in flatview_write softmmu/physmem.c:2799:14
      #17 0x55a1b97ec3f2 in address_space_write softmmu/physmem.c:2891:18
      #18 0x55a1b985c7cd in cpu_outw softmmu/ioport.c:70:5
      #19 0x55a1b99577ac in qtest_process_command softmmu/qtest.c:481:13

Inspired-by: Daniele Buono <dbuono@linux.vnet.ibm.com>
Inspired-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/scsi/megasas.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 1a5fc5857db..f5ad4425b5b 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1667,6 +1667,7 @@ static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
 {
     uint8_t *cdb;
     int target_id, lun_id, cdb_len;
+    int len = -1;
     bool is_write;
     struct SCSIDevice *sdev = NULL;
     bool is_logical = (frame_cmd == MFI_CMD_LD_SCSI_IO);
@@ -1676,6 +1677,10 @@ static int megasas_handle_scsi(MegasasState *s, MegasasCmd *cmd,
     lun_id = cmd->frame->header.lun_id;
     cdb_len = cmd->frame->header.cdb_len;
 
+    if (cdb_len > 0) {
+        len = scsi_cdb_length(cdb);
+    }
+    assert(len > 0 && cdb_len >= len);
     if (is_logical) {
         if (target_id >= MFI_MAX_LD || lun_id != 0) {
             trace_megasas_scsi_target_not_present(
-- 
2.26.2



  parent reply	other threads:[~2020-12-01 19:13 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-01 19:10 [PATCH v2 0/4] hw/scsi/megasas: Avoid buffer overrun in megasas_handle_scsi() Philippe Mathieu-Daudé
2020-12-01 19:10 ` [PATCH v2 1/4] tests/qtest/fuzz-test: Quit test_lp1878642 once done Philippe Mathieu-Daudé
2020-12-03 10:47   ` Li Qiang
2020-12-01 19:10 ` Philippe Mathieu-Daudé [this message]
2020-12-03 11:21   ` [PATCH v2 2/4] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi() Li Qiang
2020-12-03 11:36     ` Philippe Mathieu-Daudé
2020-12-03 12:02       ` Li Qiang
2020-12-03 12:38         ` Philippe Mathieu-Daudé
2020-12-04  4:50           ` Li Qiang
2020-12-01 19:10 ` [PATCH v2 3/4] tests/qtest/fuzz-test: Add test_megasas_cdb_len_zero() reproducer Philippe Mathieu-Daudé
2020-12-01 19:12   ` Philippe Mathieu-Daudé
2020-12-02  5:40   ` Thomas Huth
2020-12-01 19:10 ` [RFC PATCH v2 4/4] hw/scsi/megasas: Have incorrect cdb return MFI_STAT_ABORT_NOT_POSSIBLE Philippe Mathieu-Daudé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201201191026.4149955-3-philmd@redhat.com \
    --to=philmd@redhat.com \
    --cc=alxndr@bu.edu \
    --cc=dbuono@linux.vnet.ibm.com \
    --cc=fam@euphon.net \
    --cc=hare@suse.com \
    --cc=liq3ea@163.com \
    --cc=lvivier@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.