1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
|
Gentoo bug: https://bugs.gentoo.org/830170
Upstream bug: https://gitlab.com/qemu-project/qemu/-/issues/786
Patches taken from
https://lore.kernel.org/qemu-devel/20211227193120.1084176-1-imammedo@redhat.com/
commit dce6c86f54eab61028e110497c222e73381379df
Author: Igor Mammedov <imammedo@redhat.com>
Date: Mon Dec 27 14:31:17 2021 -0500
acpi: fix QEMU crash when started with SLIC table
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>
diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 1e004d0078..3e811bf03c 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -345,8 +345,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 a99c6e4fe3..570f82997b 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)
commit a22de122ad03ea40953ad0328b2c3e31002d8052
Author: Igor Mammedov <imammedo@redhat.com>
Date: Mon Dec 27 14:31:18 2021 -0500
tests: acpi: whitelist expected blobs before changing them
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
diff --git a/tests/data/acpi/q35/FACP.slic b/tests/data/acpi/q35/FACP.slic
new file mode 100644
index 0000000000..f6a864cc86
Binary files /dev/null and b/tests/data/acpi/q35/FACP.slic differ
diff --git a/tests/data/acpi/q35/SLIC.slic b/tests/data/acpi/q35/SLIC.slic
new file mode 100644
index 0000000000..e69de29bb2
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",
commit cb913395d76f8fdfd7f1d0c8ea77d4710821bbd3
Author: Igor Mammedov <imammedo@redhat.com>
Date: Mon Dec 27 14:31:19 2021 -0500
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>
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 258874167e..ae7ef13ec7 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1567,6 +1567,19 @@ static void test_acpi_oem_fields_virt(void)
g_free(args);
}
+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);
+}
int main(int argc, char *argv[])
{
@@ -1639,6 +1652,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);
commit ffba261306370e0ad8506401b104be5fa4749ade
Author: Igor Mammedov <imammedo@redhat.com>
Date: Mon Dec 27 14:31:20 2021 -0500
tests: acpi: SLIC: update expected blobs
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
diff --git a/tests/data/acpi/q35/FACP.slic b/tests/data/acpi/q35/FACP.slic
index f6a864cc86..891fd4b784 100644
Binary files a/tests/data/acpi/q35/FACP.slic and b/tests/data/acpi/q35/FACP.slic differ
diff --git a/tests/data/acpi/q35/SLIC.slic b/tests/data/acpi/q35/SLIC.slic
index e69de29bb2..fd26592e24 100644
Binary files a/tests/data/acpi/q35/SLIC.slic and b/tests/data/acpi/q35/SLIC.slic differ
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",
|
