* [PATCH 0/3] hw/scsi/megasas: Avoid buffer overrun in megasas_handle_scsi()
@ 2020-12-01 15:13 Philippe Mathieu-Daudé
2020-12-01 15:13 ` [PATCH 1/3] tests/qtest/fuzz-test: Quit test_lp1878642 once done Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-01 15:13 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, Li Qiang,
Hannes Reinecke, Alexander Bulekov, Paolo Bonzini,
Philippe Mathieu-Daudé
FWIW megasas is not use by KVM.
Not sure what is the proper fix, but at least we
have a reproducer.
We might improve "scsi/utils" by adding length argument to
scsi_cdb_length() and check valid there, but this will take
time (large refactor). Add assertions there is too violent.
Philippe Mathieu-Daudé (3):
tests/qtest/fuzz-test: Quit test_lp1878642 once done
hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()
hw/scsi/megasas: Have incorrect cdb return MFI_STAT_ABORT_NOT_POSSIBLE
hw/scsi/megasas.c | 6 ++
tests/qtest/fuzz-test.c | 197 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 203 insertions(+)
--
2.26.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] tests/qtest/fuzz-test: Quit test_lp1878642 once done
2020-12-01 15:13 [PATCH 0/3] hw/scsi/megasas: Avoid buffer overrun in megasas_handle_scsi() Philippe Mathieu-Daudé
@ 2020-12-01 15:13 ` Philippe Mathieu-Daudé
2020-12-01 15:59 ` Thomas Huth
2020-12-01 15:13 ` [PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi() Philippe Mathieu-Daudé
2020-12-01 15:13 ` [RFC PATCH 3/3] hw/scsi/megasas: Have incorrect cdb return MFI_STAT_ABORT_NOT_POSSIBLE Philippe Mathieu-Daudé
2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-01 15:13 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, Li Qiang,
Hannes Reinecke, Alexander Bulekov, Paolo Bonzini,
Philippe Mathieu-Daudé
Missed in fd250172842 ("qtest: add a reproducer for LP#1878642").
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tests/qtest/fuzz-test.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-test.c
index 9cb4c42bdea..87b72307a5b 100644
--- a/tests/qtest/fuzz-test.c
+++ b/tests/qtest/fuzz-test.c
@@ -45,6 +45,7 @@ static void test_lp1878642_pci_bus_get_irq_level_assert(void)
qtest_outl(s, 0xcf8, 0x8400f841);
qtest_outl(s, 0xcfc, 0xebed205d);
qtest_outl(s, 0x5d02, 0xebed205d);
+ qtest_quit(s);
}
int main(int argc, char **argv)
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()
2020-12-01 15:13 [PATCH 0/3] hw/scsi/megasas: Avoid buffer overrun in megasas_handle_scsi() Philippe Mathieu-Daudé
2020-12-01 15:13 ` [PATCH 1/3] tests/qtest/fuzz-test: Quit test_lp1878642 once done Philippe Mathieu-Daudé
@ 2020-12-01 15:13 ` Philippe Mathieu-Daudé
2020-12-01 16:01 ` Thomas Huth
2020-12-01 17:22 ` [PATCH] " Alexander Bulekov
2020-12-01 15:13 ` [RFC PATCH 3/3] hw/scsi/megasas: Have incorrect cdb return MFI_STAT_ABORT_NOT_POSSIBLE Philippe Mathieu-Daudé
2 siblings, 2 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-01 15:13 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, Li Qiang,
Hannes Reinecke, Alexander Bulekov, Paolo Bonzini,
Philippe Mathieu-Daudé
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 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 197 insertions(+)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 1a5fc5857db..28efd094111 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 87b72307a5b..42e88d761b8 100644
--- a/tests/qtest/fuzz-test.c
+++ b/tests/qtest/fuzz-test.c
@@ -48,6 +48,200 @@ static void test_lp1878642_pci_bus_get_irq_level_assert(void)
qtest_quit(s);
}
+static void test_megasas_cdb_len_zero(void)
+{
+ static const unsigned char megasas_blob1[] = {
+ 0x03, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60
+ },
+ megasas_blob2[] = {
+ 0x03, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e,
+ 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa,
+ 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x2e, 0x3e, 0x00, 0xff, 0x00,
+ 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d,
+ 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00,
+ 0xff, 0xff, 0x59, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe,
+ 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a,
+ 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x84, 0x3e, 0x00,
+ 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51,
+ 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00,
+ 0xeb, 0x00, 0xff, 0xff, 0xaf, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60,
+ 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15,
+ 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0xda,
+ 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00,
+ 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14,
+ 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x05, 0x3e, 0x00, 0xff, 0x00, 0x00,
+ 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea,
+ 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff,
+ 0xff, 0x30, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff,
+ 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff,
+ 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x5b, 0x3e, 0x00, 0xff,
+ 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00,
+ 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb,
+ 0x00, 0xff, 0xff, 0x86, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff,
+ 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a,
+ 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0xb1, 0x3e,
+ 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17,
+ 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02,
+ 0x00, 0xeb, 0x00, 0xff, 0xff, 0xdc, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00,
+ 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46,
+ 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff,
+ 0x07, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e,
+ 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa,
+ 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x32, 0x3e, 0x00, 0xff, 0x00,
+ 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d,
+ 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00,
+ 0xff, 0xff, 0x5d, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe,
+ 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a,
+ 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x88, 0x3e, 0x00,
+ 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51,
+ 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00,
+ 0xeb, 0x00, 0xff, 0xff, 0xb3, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60,
+ 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15,
+ 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0xde,
+ 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00,
+ 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14,
+ 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x09, 0x3e, 0x00, 0xff, 0x00, 0x00,
+ 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea,
+ 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff,
+ 0xff, 0x34, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff,
+ 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff,
+ 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x5f, 0x3e, 0x00, 0xff,
+ 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00,
+ 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb,
+ 0x00, 0xff, 0xff, 0x8a, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff,
+ 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a,
+ 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0xb5, 0x3e,
+ 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17,
+ 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02,
+ 0x00, 0xeb, 0x00, 0xff, 0xff, 0xe0, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00,
+ 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46,
+ 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff,
+ 0x0b, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e,
+ 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa,
+ 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x36, 0x3e, 0x00, 0xff, 0x00,
+ 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d,
+ 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00,
+ 0xff, 0xff, 0x61, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe,
+ 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a,
+ 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x8c, 0x3e, 0x00,
+ 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51,
+ 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00,
+ 0xeb, 0x00, 0xff, 0xff, 0xb7, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60,
+ 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15,
+ 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0xe2,
+ 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00,
+ 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14,
+ 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x0d, 0x3e, 0x00, 0xff, 0x00, 0x00,
+ 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea,
+ 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff,
+ 0xff, 0x38, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff,
+ 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff,
+ 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x63, 0x3e, 0x00, 0xff,
+ 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00,
+ 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb,
+ 0x00, 0xff, 0xff, 0x8e, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff,
+ 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a,
+ 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0xb9, 0x3e,
+ 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17,
+ 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02,
+ 0x00, 0xeb, 0x00, 0xff, 0xff, 0xe4, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00,
+ 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46,
+ 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff,
+ 0x0f, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e,
+ 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa,
+ 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x3a, 0x3e, 0x00, 0xff, 0x00,
+ 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d,
+ 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00,
+ 0xff, 0xff, 0x65, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe,
+ 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a,
+ 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x90, 0x3e, 0x00,
+ 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51,
+ 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00,
+ 0xeb, 0x00, 0xff, 0xff, 0xbb, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60,
+ 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15,
+ 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0xe6,
+ 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00,
+ 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14,
+ 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x11, 0x3e, 0x00, 0xff, 0x00, 0x00,
+ 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea,
+ 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff,
+ 0xff, 0x3c, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff,
+ 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff,
+ 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x67, 0x3e, 0x00, 0xff,
+ 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00,
+ 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb,
+ 0x00, 0xff, 0xff, 0x92, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff,
+ 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a,
+ 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0xbd, 0x3e,
+ 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17,
+ 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02,
+ 0x00, 0xeb, 0x00, 0xff, 0xff, 0xe8, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00,
+ 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46,
+ 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff,
+ 0x13, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e,
+ 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa,
+ 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x3e, 0x3e, 0x00, 0xff, 0x00,
+ 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d,
+ 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00,
+ 0xff, 0xff, 0x69, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe,
+ 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a,
+ 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x94, 0x3e, 0x00,
+ 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51,
+ 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00,
+ 0xeb, 0x00, 0xff, 0xff, 0xbf, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60,
+ 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15,
+ 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0xea,
+ 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00,
+ 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14,
+ 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x15, 0x3e, 0x00, 0xff, 0x00, 0x00,
+ 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea,
+ 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff,
+ 0xff, 0x40, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff,
+ 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff,
+ 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x6b, 0x3e, 0x00, 0xff,
+ 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00,
+ 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb,
+ 0x00, 0xff, 0xff, 0x96, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff,
+ 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a,
+ 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0xc1, 0x3e,
+ 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17,
+ 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02,
+ 0x00, 0xeb, 0x00, 0xff, 0xff, 0xec, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00,
+ 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46,
+ 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff,
+ 0x17, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e,
+ 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa,
+ 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x42, 0x3e, 0x00, 0xff, 0x00,
+ 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d,
+ 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00,
+ 0xff, 0xff, 0x6d, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe,
+ 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a,
+ 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0x98, 0x3e, 0x00,
+ 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51,
+ 0x00, 0x0d, 0xea, 0x46, 0x15, 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00,
+ 0xeb, 0x00, 0xff, 0xff, 0xc3, 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60,
+ 0xff, 0xfe, 0xff, 0x3e, 0x00, 0x17, 0x51, 0x00, 0x0d, 0xea, 0x46, 0x15,
+ 0x5a, 0x5a, 0xff, 0xaa, 0x14, 0x02, 0x00, 0xeb, 0x00, 0xff, 0xff, 0xee,
+ 0x3e, 0x00, 0xff, 0x00, 0x00, 0x00, 0x60, 0xff, 0xfe, 0xff, 0x3e, 0x00,
+ 0x17, 0x51, 0x00, 0x0d
+ };
+ 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_memwrite(s, 0x4608, megasas_blob1, sizeof(megasas_blob1));
+ qtest_memwrite(s, 0x4600, megasas_blob1, sizeof(megasas_blob1));
+ qtest_memwrite(s, 0x4610, megasas_blob2, sizeof(megasas_blob2));
+ qtest_outw(s, 0xbb40, 0x460b);
+ qtest_quit(s);
+}
+
int main(int argc, char **argv)
{
const char *arch = qtest_get_arch();
@@ -59,6 +253,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();
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC PATCH 3/3] hw/scsi/megasas: Have incorrect cdb return MFI_STAT_ABORT_NOT_POSSIBLE
2020-12-01 15:13 [PATCH 0/3] hw/scsi/megasas: Avoid buffer overrun in megasas_handle_scsi() Philippe Mathieu-Daudé
2020-12-01 15:13 ` [PATCH 1/3] tests/qtest/fuzz-test: Quit test_lp1878642 once done Philippe Mathieu-Daudé
2020-12-01 15:13 ` [PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi() Philippe Mathieu-Daudé
@ 2020-12-01 15:13 ` Philippe Mathieu-Daudé
2 siblings, 0 replies; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-12-01 15:13 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, Li Qiang,
Hannes Reinecke, Alexander Bulekov, Paolo Bonzini,
Philippe Mathieu-Daudé
Avoid out-of-bound array access with invalid CDB is provided.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
RFC because I have no clue how hardware works.
Maybe returning MFI_STAT_ARRAY_INDEX_INVALID is better?
Do we need to call megasas_write_sense()?
hw/scsi/megasas.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 28efd094111..d89a3c8c3ce 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -1676,7 +1676,12 @@ 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 (!cdb_len || scsi_cdb_length(cdb) < cdb_len) {
+ trace_megasas_scsi_invalid_cdb_len(mfi_frame_desc(frame_cmd),
+ is_logical, target_id,
+ lun_id, cdb_len);
+ return MFI_STAT_ABORT_NOT_POSSIBLE;
+ }
if (is_logical) {
if (target_id >= MFI_MAX_LD || lun_id != 0) {
trace_megasas_scsi_target_not_present(
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] tests/qtest/fuzz-test: Quit test_lp1878642 once done
2020-12-01 15:13 ` [PATCH 1/3] tests/qtest/fuzz-test: Quit test_lp1878642 once done Philippe Mathieu-Daudé
@ 2020-12-01 15:59 ` Thomas Huth
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Huth @ 2020-12-01 15:59 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Fam Zheng, Laurent Vivier, Hannes Reinecke, qemu-block, Li Qiang,
Alexander Bulekov, Paolo Bonzini
On 01/12/2020 16.13, Philippe Mathieu-Daudé wrote:
> Missed in fd250172842 ("qtest: add a reproducer for LP#1878642").
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> tests/qtest/fuzz-test.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-test.c
> index 9cb4c42bdea..87b72307a5b 100644
> --- a/tests/qtest/fuzz-test.c
> +++ b/tests/qtest/fuzz-test.c
> @@ -45,6 +45,7 @@ static void test_lp1878642_pci_bus_get_irq_level_assert(void)
> qtest_outl(s, 0xcf8, 0x8400f841);
> qtest_outl(s, 0xcfc, 0xebed205d);
> qtest_outl(s, 0x5d02, 0xebed205d);
> + qtest_quit(s);
> }
>
> int main(int argc, char **argv)
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()
2020-12-01 15:13 ` [PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi() Philippe Mathieu-Daudé
@ 2020-12-01 16:01 ` Thomas Huth
2020-12-01 17:22 ` [PATCH] " Alexander Bulekov
1 sibling, 0 replies; 7+ messages in thread
From: Thomas Huth @ 2020-12-01 16:01 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Fam Zheng, Laurent Vivier, Hannes Reinecke, qemu-block, Li Qiang,
Alexander Bulekov, Paolo Bonzini
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi()
2020-12-01 15:13 ` [PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi() Philippe Mathieu-Daudé
2020-12-01 16:01 ` Thomas Huth
@ 2020-12-01 17:22 ` Alexander Bulekov
1 sibling, 0 replies; 7+ messages in thread
From: Alexander Bulekov @ 2020-12-01 17:22 UTC (permalink / raw)
To: qemu-devel
Cc: Fam Zheng, Laurent Vivier, Thomas Huth, qemu-block, Li Qiang,
Hannes Reinecke, Alexander Bulekov, Paolo Bonzini,
Philippe Mathieu-Daudé
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 --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();
--
2.28.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-12-01 17:42 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-01 15:13 [PATCH 0/3] hw/scsi/megasas: Avoid buffer overrun in megasas_handle_scsi() Philippe Mathieu-Daudé
2020-12-01 15:13 ` [PATCH 1/3] tests/qtest/fuzz-test: Quit test_lp1878642 once done Philippe Mathieu-Daudé
2020-12-01 15:59 ` Thomas Huth
2020-12-01 15:13 ` [PATCH 2/3] hw/scsi/megasas: Assert cdb_len is valid in megasas_handle_scsi() Philippe Mathieu-Daudé
2020-12-01 16:01 ` Thomas Huth
2020-12-01 17:22 ` [PATCH] " Alexander Bulekov
2020-12-01 15:13 ` [RFC PATCH 3/3] hw/scsi/megasas: Have incorrect cdb return MFI_STAT_ABORT_NOT_POSSIBLE Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).