fix CVE-2021-3507 and fix -acpitable regression (openeuler !300!301)

Signed-off-by: yezengruan <yezengruan@huawei.com>
This commit is contained in:
yezengruan 2022-05-30 18:43:29 +08:00
parent accd81d09e
commit ba8cdf7ac0
8 changed files with 412 additions and 1 deletions

Binary file not shown.

View File

@ -0,0 +1,90 @@
From ad7b272f693e2fbf4a74c47f566b6c3f8c27247c Mon Sep 17 00:00:00 2001
From: Igor Mammedov <imammedo@redhat.com>
Date: Mon, 27 Dec 2021 14:31:17 -0500
Subject: [PATCH 1/6] acpi: fix QEMU crash when started with SLIC table
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
if QEMU is started with used provided SLIC table blob,
-acpitable sig=SLIC,oem_id='CRASH ',oem_table_id="ME",oem_rev=00002210,asl_compiler_id="",asl_compiler_rev=00000000,data=/dev/null
it will assert with:
hw/acpi/aml-build.c:61:build_append_padded_str: assertion failed: (len <= maxlen)
and following backtrace:
...
build_append_padded_str (array=0x555556afe320, str=0x555556afdb2e "CRASH ME", maxlen=0x6, pad=0x20) at hw/acpi/aml-build.c:61
acpi_table_begin (desc=0x7fffffffd1b0, array=0x555556afe320) at hw/acpi/aml-build.c:1727
build_fadt (tbl=0x555556afe320, linker=0x555557ca3830, f=0x7fffffffd318, oem_id=0x555556afdb2e "CRASH ME", oem_table_id=0x555556afdb34 "ME") at hw/acpi/aml-build.c:2064
...
which happens due to acpi_table_begin() expecting NULL terminated
oem_id and oem_table_id strings, which is normally the case, but
in case of user provided SLIC table, oem_id points to table's blob
directly and as result oem_id became longer than expected.
Fix issue by handling oem_id consistently and make acpi_get_slic_oem()
return NULL terminated strings.
PS:
After [1] refactoring, oem_id semantics became inconsistent, where
NULL terminated string was coming from machine and old way pointer
into byte array coming from -acpitable option. That used to work
since build_header() wasn't expecting NULL terminated string and
blindly copied the 1st 6 bytes only.
However commit [2] broke that by replacing build_header() with
acpi_table_begin(), which was expecting NULL terminated string
and was checking oem_id size.
1) 602b45820 ("acpi: Permit OEM ID and OEM table ID fields to be changed")
2)
Fixes: 4b56e1e4eb08 ("acpi: build_fadt: use acpi_table_begin()/acpi_table_end() instead of build_header()")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/786
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20211227193120.1084176-2-imammedo@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Denis Lisov <dennis.lissov@gmail.com>
Tested-by: Alexander Tsoy <alexander@tsoy.me>
Cc: qemu-stable@nongnu.org
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/acpi/core.c | 4 ++--
hw/i386/acpi-build.c | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index eb631caa91..a2d790d432 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -346,8 +346,8 @@ int acpi_get_slic_oem(AcpiSlicOem *oem)
struct acpi_table_header *hdr = (void *)(u - sizeof(hdr->_length));
if (memcmp(hdr->sig, "SLIC", 4) == 0) {
- oem->id = hdr->oem_id;
- oem->table_id = hdr->oem_table_id;
+ oem->id = g_strndup(hdr->oem_id, 6);
+ oem->table_id = g_strndup(hdr->oem_table_id, 8);
return 0;
}
}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 1ce2d67c2e..0ec2932ec2 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2721,6 +2721,8 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
/* Cleanup memory that's no longer used. */
g_array_free(table_offsets, true);
+ g_free(slic_oem.id);
+ g_free(slic_oem.table_id);
}
static void acpi_ram_update(MemoryRegion *mr, GArray *data)
--
2.27.0

View File

@ -0,0 +1,87 @@
From 4c72f406e3ff02844977fcd8bc696080088d3d08 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= <philmd@redhat.com>
Date: Thu, 18 Nov 2021 12:57:32 +0100
Subject: [PATCH 5/6] hw/block/fdc: Prevent end-of-track overrun
(CVE-2021-3507)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Per the 82078 datasheet, if the end-of-track (EOT byte in
the FIFO) is more than the number of sectors per side, the
command is terminated unsuccessfully:
* 5.2.5 DATA TRANSFER TERMINATION
The 82078 supports terminal count explicitly through
the TC pin and implicitly through the underrun/over-
run and end-of-track (EOT) functions. For full sector
transfers, the EOT parameter can define the last
sector to be transferred in a single or multisector
transfer. If the last sector to be transferred is a par-
tial sector, the host can stop transferring the data in
mid-sector, and the 82078 will continue to complete
the sector as if a hardware TC was received. The
only difference between these implicit functions and
TC is that they return "abnormal termination" result
status. Such status indications can be ignored if they
were expected.
* 6.1.3 READ TRACK
This command terminates when the EOT specified
number of sectors have been read. If the 82078
does not find an I D Address Mark on the diskette
after the second· occurrence of a pulse on the
INDX# pin, then it sets the IC code in Status Regis-
ter 0 to "01" (Abnormal termination), sets the MA bit
in Status Register 1 to "1", and terminates the com-
mand.
* 6.1.6 VERIFY
Refer to Table 6-6 and Table 6-7 for information
concerning the values of MT and EC versus SC and
EOT value.
* Table 6·6. Result Phase Table
* Table 6-7. Verify Command Result Phase Table
Fix by aborting the transfer when EOT > # Sectors Per Side.
Cc: qemu-stable@nongnu.org
Cc: Hervé Poussineau <hpoussin@reactos.org>
Fixes: baca51faff0 ("floppy driver: disk geometry auto detect")
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/339
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20211118115733.4038610-2-philmd@redhat.com>
Reviewed-by: Hanna Reitz <hreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/block/fdc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 21d18ac2e3..24b05406e6 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1529,6 +1529,14 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)
int tmp;
fdctrl->data_len = 128 << (fdctrl->fifo[5] > 7 ? 7 : fdctrl->fifo[5]);
tmp = (fdctrl->fifo[6] - ks + 1);
+ if (tmp < 0) {
+ FLOPPY_DPRINTF("invalid EOT: %d\n", tmp);
+ fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00);
+ fdctrl->fifo[3] = kt;
+ fdctrl->fifo[4] = kh;
+ fdctrl->fifo[5] = ks;
+ return;
+ }
if (fdctrl->fifo[0] & 0x80)
tmp += fdctrl->fifo[6];
fdctrl->data_len *= tmp;
--
2.27.0

View File

@ -1,6 +1,6 @@
Name: qemu
Version: 6.2.0
Release: 37
Release: 38
Epoch: 2
Summary: QEMU is a generic and open source machine emulator and virtualizer
License: GPLv2 and BSD and MIT and CC-BY-SA-4.0
@ -248,6 +248,12 @@ Patch0234: hw-intc-arm_gicv3-Check-for-MEMTX_OK-instead-of-MEMT.patch
Patch0235: softmmu-physmem-Simplify-flatview_write-and-address_.patch
Patch0236: softmmu-physmem-Introduce-MemTxAttrs-memory-field-an.patch
Patch0237: acpi-modify-build_ppt-del-macro-add-arm-build_pptt.patch
Patch0238: acpi-fix-QEMU-crash-when-started-with-SLIC-table.patch
Patch0239: tests-acpi-whitelist-expected-blobs-before-changing-.patch
Patch0240: tests-acpi-add-SLIC-table-test.patch
Patch0241: tests-acpi-SLIC-update-expected-blobs.patch
Patch0242: hw-block-fdc-Prevent-end-of-track-overrun-CVE-2021-3.patch
Patch0243: tests-qtest-fdc-test-Add-a-regression-test-for-CVE-2.patch
BuildRequires: flex
BuildRequires: gcc
@ -739,6 +745,14 @@ getent passwd qemu >/dev/null || \
%endif
%changelog
* Mon May 30 2022 yezengruan <yezengruan@huawei.com> - 2:6.2.0-38
- acpi: fix QEMU crash when started with SLIC table
- tests: acpi: whitelist expected blobs before changing them
- tests: acpi: add SLIC table test
- tests: acpi: SLIC: update expected blobs
- hw/block/fdc: Prevent end-of-track overrun (CVE-2021-3507)
- tests/qtest/fdc-test: Add a regression test for CVE-2021-3507
* Mon May 30 2022 zhangziyang <zhangziyang1@huawei.com> - 2:6.2.0-37
- add qemu-system-x86_64, qemu-system-aarch64, qemu-system-arm rpm package build

View File

@ -0,0 +1,26 @@
From 86392e80092e62197f51507513ee09100f9c1653 Mon Sep 17 00:00:00 2001
From: Igor Mammedov <imammedo@redhat.com>
Date: Mon, 27 Dec 2021 14:31:20 -0500
Subject: [PATCH 4/6] tests: acpi: SLIC: update expected blobs
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20211227193120.1084176-5-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
tests/data/acpi/q35/FACP.slic | Bin 244 -> 244 bytes
tests/data/acpi/q35/SLIC.slic | Bin 0 -> 36 bytes
tests/qtest/bios-tables-test-allowed-diff.h | 2 --
3 files changed, 2 deletions(-)
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index 49dbf8fa3e..dfb8523c8b 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,3 +1 @@
/* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/q35/FACP.slic",
-"tests/data/acpi/q35/SLIC.slic",
--
2.27.0

View File

@ -0,0 +1,55 @@
From b9c96b0a111e5333857682a5dc9770cbebcbabea Mon Sep 17 00:00:00 2001
From: Igor Mammedov <imammedo@redhat.com>
Date: Mon, 27 Dec 2021 14:31:19 -0500
Subject: [PATCH 3/6] tests: acpi: add SLIC table test
When user uses '-acpitable' to add SLIC table, some ACPI
tables (FADT) will change its 'Oem ID'/'Oem Table ID' fields to
match that of SLIC. Test makes sure thati QEMU handles
those fields correctly when SLIC table is added with
'-acpitable' option.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20211227193120.1084176-4-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
tests/qtest/bios-tables-test.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 258874167e..184937e6ca 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1465,6 +1465,20 @@ static void test_acpi_virt_tcg(void)
free_test_data(&data);
}
+static void test_acpi_q35_slic(void)
+{
+ test_data data = {
+ .machine = MACHINE_Q35,
+ .variant = ".slic",
+ };
+
+ test_acpi_one("-acpitable sig=SLIC,oem_id='CRASH ',oem_table_id='ME',"
+ "oem_rev=00002210,asl_compiler_id='qemu',"
+ "asl_compiler_rev=00000000,data=/dev/null",
+ &data);
+ free_test_data(&data);
+}
+
static void test_oem_fields(test_data *data)
{
int i;
@@ -1639,6 +1653,7 @@ int main(int argc, char *argv[])
qtest_add_func("acpi/q35/kvm/xapic", test_acpi_q35_kvm_xapic);
qtest_add_func("acpi/q35/kvm/dmar", test_acpi_q35_kvm_dmar);
}
+ qtest_add_func("acpi/q35/slic", test_acpi_q35_slic);
} else if (strcmp(arch, "aarch64") == 0) {
if (has_tcg) {
qtest_add_func("acpi/virt", test_acpi_virt_tcg);
--
2.27.0

View File

@ -0,0 +1,29 @@
From 83229a5034161d021ac21f7c7f921d4398d388c6 Mon Sep 17 00:00:00 2001
From: Igor Mammedov <imammedo@redhat.com>
Date: Mon, 27 Dec 2021 14:31:18 -0500
Subject: [PATCH 2/6] tests: acpi: whitelist expected blobs before changing
them
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20211227193120.1084176-3-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
tests/data/acpi/q35/FACP.slic | Bin 0 -> 244 bytes
tests/data/acpi/q35/SLIC.slic | 0
tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
3 files changed, 2 insertions(+)
create mode 100644 tests/data/acpi/q35/FACP.slic
create mode 100644 tests/data/acpi/q35/SLIC.slic
diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..49dbf8fa3e 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,3 @@
/* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/FACP.slic",
+"tests/data/acpi/q35/SLIC.slic",
--
2.27.0

View File

@ -0,0 +1,110 @@
From c3ed2f4828e29f5ac82efd6a32117e9b17180dd1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= <philmd@redhat.com>
Date: Thu, 18 Nov 2021 12:57:33 +0100
Subject: [PATCH 6/6] tests/qtest/fdc-test: Add a regression test for
CVE-2021-3507
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add the reproducer from https://gitlab.com/qemu-project/qemu/-/issues/339
Without the previous commit, when running 'make check-qtest-i386'
with QEMU configured with '--enable-sanitizers' we get:
==4028352==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x619000062a00 at pc 0x5626d03c491a bp 0x7ffdb4199410 sp 0x7ffdb4198bc0
READ of size 786432 at 0x619000062a00 thread T0
#0 0x5626d03c4919 in __asan_memcpy (qemu-system-i386+0x1e65919)
#1 0x5626d1c023cc in flatview_write_continue softmmu/physmem.c:2787:13
#2 0x5626d1bf0c0f in flatview_write softmmu/physmem.c:2822:14
#3 0x5626d1bf0798 in address_space_write softmmu/physmem.c:2914:18
#4 0x5626d1bf0f37 in address_space_rw softmmu/physmem.c:2924:16
#5 0x5626d1bf14c8 in cpu_physical_memory_rw softmmu/physmem.c:2933:5
#6 0x5626d0bd5649 in cpu_physical_memory_write include/exec/cpu-common.h:82:5
#7 0x5626d0bd0a07 in i8257_dma_write_memory hw/dma/i8257.c:452:9
#8 0x5626d09f825d in fdctrl_transfer_handler hw/block/fdc.c:1616:13
#9 0x5626d0a048b4 in fdctrl_start_transfer hw/block/fdc.c:1539:13
#10 0x5626d09f4c3e in fdctrl_write_data hw/block/fdc.c:2266:13
#11 0x5626d09f22f7 in fdctrl_write hw/block/fdc.c:829:9
#12 0x5626d1c20bc5 in portio_write softmmu/ioport.c:207:17
0x619000062a00 is located 0 bytes to the right of 512-byte region [0x619000062800,0x619000062a00)
allocated by thread T0 here:
#0 0x5626d03c66ec in posix_memalign (qemu-system-i386+0x1e676ec)
#1 0x5626d2b988d4 in qemu_try_memalign util/oslib-posix.c:210:11
#2 0x5626d2b98b0c in qemu_memalign util/oslib-posix.c:226:27
#3 0x5626d09fbaf0 in fdctrl_realize_common hw/block/fdc.c:2341:20
#4 0x5626d0a150ed in isabus_fdc_realize hw/block/fdc-isa.c:113:5
#5 0x5626d2367935 in device_set_realized hw/core/qdev.c:531:13
SUMMARY: AddressSanitizer: heap-buffer-overflow (qemu-system-i386+0x1e65919) in __asan_memcpy
Shadow bytes around the buggy address:
0x0c32800044f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c3280004510: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c3280004520: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x0c3280004530: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c3280004540:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004550: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004560: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004570: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004580: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c3280004590: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Heap left redzone: fa
Freed heap region: fd
==4028352==ABORTING
[ kwolf: Added snapshot=on to prevent write file lock failure ]
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qtest/fdc-test.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c
index 8f6eee84a4..6f5850354f 100644
--- a/tests/qtest/fdc-test.c
+++ b/tests/qtest/fdc-test.c
@@ -583,6 +583,26 @@ static void test_cve_2021_20196(void)
qtest_quit(s);
}
+static void test_cve_2021_3507(void)
+{
+ QTestState *s;
+
+ s = qtest_initf("-nographic -m 32M -nodefaults "
+ "-drive file=%s,format=raw,if=floppy,snapshot=on",
+ test_image);
+ qtest_outl(s, 0x9, 0x0a0206);
+ qtest_outw(s, 0x3f4, 0x1600);
+ qtest_outw(s, 0x3f4, 0x0000);
+ qtest_outw(s, 0x3f4, 0x0000);
+ qtest_outw(s, 0x3f4, 0x0000);
+ qtest_outw(s, 0x3f4, 0x0200);
+ qtest_outw(s, 0x3f4, 0x0200);
+ qtest_outw(s, 0x3f4, 0x0000);
+ qtest_outw(s, 0x3f4, 0x0000);
+ qtest_outw(s, 0x3f4, 0x0000);
+ qtest_quit(s);
+}
+
int main(int argc, char **argv)
{
int fd;
@@ -614,6 +634,7 @@ int main(int argc, char **argv)
qtest_add_func("/fdc/read_no_dma_19", test_read_no_dma_19);
qtest_add_func("/fdc/fuzz-registers", fuzz_registers);
qtest_add_func("/fdc/fuzz/cve_2021_20196", test_cve_2021_20196);
+ qtest_add_func("/fdc/fuzz/cve_2021_3507", test_cve_2021_3507);
ret = g_test_run();
--
2.27.0