| Message ID | 1431953244-29304-1-git-send-email-somlo@cmu.edu |
|---|---|
| State | New |
| Headers | show |
ping ? On Mon, May 18, 2015 at 08:47:24AM -0400, Gabriel L. Somlo wrote: > It has been reported that sometimes the .rodata section of SeaBIOS, > containing the constant string against which the SMBIOS signature > ends up being compared, also falls within the guest f-segment. In > that case, the test obviously fails, unless we continue searching > for the *real* SMBIOS entry point. > > Rather than stopping at the first match for the SMBIOS signature > ("_SM_") in the f-segment (0xF0000-0xFFFFF), continue scanning > until either a valid entry point table is found, or the f-segment > has been exhausted. > > Reported-by: Bruce Rogers <brogers@suse.com> > Signed-off-by: Gabriel Somlo <somlo@cmu.edu> > Tested-by: Bruce Rogers <brogers@suse.com> > --- > > - Added motivation for patch in commit log, as requested by mst > (whom I forgot to cc on v2 -- D'OH!!! ) :) > > - Fixed s/withing/within/ as pointed out by eblake > > Thanks, > Gabriel > > tests/bios-tables-test.c | 76 ++++++++++++++++++++++++++++-------------------- > 1 file changed, 44 insertions(+), 32 deletions(-) > > diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c > index 7e85dc4..0de1742 100644 > --- a/tests/bios-tables-test.c > +++ b/tests/bios-tables-test.c > @@ -599,35 +599,15 @@ static void test_acpi_asl(test_data *data) > free_test_data(&exp_data); > } > > -static void test_smbios_ep_address(test_data *data) > -{ > - uint32_t off; > - > - /* find smbios entry point structure */ > - for (off = 0xf0000; off < 0x100000; off += 0x10) { > - uint8_t sig[] = "_SM_"; > - int i; > - > - for (i = 0; i < sizeof sig - 1; ++i) { > - sig[i] = readb(off + i); > - } > - > - if (!memcmp(sig, "_SM_", sizeof sig)) { > - break; > - } > - } > - > - g_assert_cmphex(off, <, 0x100000); > - data->smbios_ep_addr = off; > -} > - > -static void test_smbios_ep_table(test_data *data) > +static bool smbios_ep_table_ok(test_data *data) > { > struct smbios_entry_point *ep_table = &data->smbios_ep_table; > uint32_t addr = data->smbios_ep_addr; > > ACPI_READ_ARRAY(ep_table->anchor_string, addr); > - g_assert(!memcmp(ep_table->anchor_string, "_SM_", 4)); > + if (memcmp(ep_table->anchor_string, "_SM_", 4)) { > + return false; > + } > ACPI_READ_FIELD(ep_table->checksum, addr); > ACPI_READ_FIELD(ep_table->length, addr); > ACPI_READ_FIELD(ep_table->smbios_major_version, addr); > @@ -636,17 +616,50 @@ static void test_smbios_ep_table(test_data *data) > ACPI_READ_FIELD(ep_table->entry_point_revision, addr); > ACPI_READ_ARRAY(ep_table->formatted_area, addr); > ACPI_READ_ARRAY(ep_table->intermediate_anchor_string, addr); > - g_assert(!memcmp(ep_table->intermediate_anchor_string, "_DMI_", 5)); > + if (memcmp(ep_table->intermediate_anchor_string, "_DMI_", 5)) { > + return false; > + } > ACPI_READ_FIELD(ep_table->intermediate_checksum, addr); > ACPI_READ_FIELD(ep_table->structure_table_length, addr); > - g_assert_cmpuint(ep_table->structure_table_length, >, 0); > + if (ep_table->structure_table_length == 0) { > + return false; > + } > ACPI_READ_FIELD(ep_table->structure_table_address, addr); > ACPI_READ_FIELD(ep_table->number_of_structures, addr); > - g_assert_cmpuint(ep_table->number_of_structures, >, 0); > + if (ep_table->number_of_structures == 0) { > + return false; > + } > ACPI_READ_FIELD(ep_table->smbios_bcd_revision, addr); > - g_assert(!acpi_checksum((uint8_t *)ep_table, sizeof *ep_table)); > - g_assert(!acpi_checksum((uint8_t *)ep_table + 0x10, > - sizeof *ep_table - 0x10)); > + if (acpi_checksum((uint8_t *)ep_table, sizeof *ep_table) || > + acpi_checksum((uint8_t *)ep_table + 0x10, sizeof *ep_table - 0x10)) { > + return false; > + } > + return true; > +} > + > +static void test_smbios_entry_point(test_data *data) > +{ > + uint32_t off; > + > + /* find smbios entry point structure */ > + for (off = 0xf0000; off < 0x100000; off += 0x10) { > + uint8_t sig[] = "_SM_"; > + int i; > + > + for (i = 0; i < sizeof sig - 1; ++i) { > + sig[i] = readb(off + i); > + } > + > + if (!memcmp(sig, "_SM_", sizeof sig)) { > + /* signature match, but is this a valid entry point? */ > + data->smbios_ep_addr = off; > + if (smbios_ep_table_ok(data)) { > + break; > + } > + } > + } > + > + g_assert_cmphex(off, <, 0x100000); > } > > static inline bool smbios_single_instance(uint8_t type) > @@ -767,8 +780,7 @@ static void test_acpi_one(const char *params, test_data *data) > } > } > > - test_smbios_ep_address(data); > - test_smbios_ep_table(data); > + test_smbios_entry_point(data); > test_smbios_structs(data); > > qtest_quit(global_qtest); > -- > 2.1.0 >
On Mo, 2015-05-18 at 08:47 -0400, Gabriel L. Somlo wrote: > It has been reported that sometimes the .rodata section of SeaBIOS, > containing the constant string against which the SMBIOS signature > ends up being compared, also falls within the guest f-segment. In > that case, the test obviously fails, unless we continue searching > for the *real* SMBIOS entry point. > > Rather than stopping at the first match for the SMBIOS signature > ("_SM_") in the f-segment (0xF0000-0xFFFFF), continue scanning > until either a valid entry point table is found, or the f-segment > has been exhausted. added to fw_cfg queue too. cheers, Gerd
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c index 7e85dc4..0de1742 100644 --- a/tests/bios-tables-test.c +++ b/tests/bios-tables-test.c @@ -599,35 +599,15 @@ static void test_acpi_asl(test_data *data) free_test_data(&exp_data); } -static void test_smbios_ep_address(test_data *data) -{ - uint32_t off; - - /* find smbios entry point structure */ - for (off = 0xf0000; off < 0x100000; off += 0x10) { - uint8_t sig[] = "_SM_"; - int i; - - for (i = 0; i < sizeof sig - 1; ++i) { - sig[i] = readb(off + i); - } - - if (!memcmp(sig, "_SM_", sizeof sig)) { - break; - } - } - - g_assert_cmphex(off, <, 0x100000); - data->smbios_ep_addr = off; -} - -static void test_smbios_ep_table(test_data *data) +static bool smbios_ep_table_ok(test_data *data) { struct smbios_entry_point *ep_table = &data->smbios_ep_table; uint32_t addr = data->smbios_ep_addr; ACPI_READ_ARRAY(ep_table->anchor_string, addr); - g_assert(!memcmp(ep_table->anchor_string, "_SM_", 4)); + if (memcmp(ep_table->anchor_string, "_SM_", 4)) { + return false; + } ACPI_READ_FIELD(ep_table->checksum, addr); ACPI_READ_FIELD(ep_table->length, addr); ACPI_READ_FIELD(ep_table->smbios_major_version, addr); @@ -636,17 +616,50 @@ static void test_smbios_ep_table(test_data *data) ACPI_READ_FIELD(ep_table->entry_point_revision, addr); ACPI_READ_ARRAY(ep_table->formatted_area, addr); ACPI_READ_ARRAY(ep_table->intermediate_anchor_string, addr); - g_assert(!memcmp(ep_table->intermediate_anchor_string, "_DMI_", 5)); + if (memcmp(ep_table->intermediate_anchor_string, "_DMI_", 5)) { + return false; + } ACPI_READ_FIELD(ep_table->intermediate_checksum, addr); ACPI_READ_FIELD(ep_table->structure_table_length, addr); - g_assert_cmpuint(ep_table->structure_table_length, >, 0); + if (ep_table->structure_table_length == 0) { + return false; + } ACPI_READ_FIELD(ep_table->structure_table_address, addr); ACPI_READ_FIELD(ep_table->number_of_structures, addr); - g_assert_cmpuint(ep_table->number_of_structures, >, 0); + if (ep_table->number_of_structures == 0) { + return false; + } ACPI_READ_FIELD(ep_table->smbios_bcd_revision, addr); - g_assert(!acpi_checksum((uint8_t *)ep_table, sizeof *ep_table)); - g_assert(!acpi_checksum((uint8_t *)ep_table + 0x10, - sizeof *ep_table - 0x10)); + if (acpi_checksum((uint8_t *)ep_table, sizeof *ep_table) || + acpi_checksum((uint8_t *)ep_table + 0x10, sizeof *ep_table - 0x10)) { + return false; + } + return true; +} + +static void test_smbios_entry_point(test_data *data) +{ + uint32_t off; + + /* find smbios entry point structure */ + for (off = 0xf0000; off < 0x100000; off += 0x10) { + uint8_t sig[] = "_SM_"; + int i; + + for (i = 0; i < sizeof sig - 1; ++i) { + sig[i] = readb(off + i); + } + + if (!memcmp(sig, "_SM_", sizeof sig)) { + /* signature match, but is this a valid entry point? */ + data->smbios_ep_addr = off; + if (smbios_ep_table_ok(data)) { + break; + } + } + } + + g_assert_cmphex(off, <, 0x100000); } static inline bool smbios_single_instance(uint8_t type) @@ -767,8 +780,7 @@ static void test_acpi_one(const char *params, test_data *data) } } - test_smbios_ep_address(data); - test_smbios_ep_table(data); + test_smbios_entry_point(data); test_smbios_structs(data); qtest_quit(global_qtest);
It has been reported that sometimes the .rodata section of SeaBIOS, containing the constant string against which the SMBIOS signature ends up being compared, also falls within the guest f-segment. In that case, the test obviously fails, unless we continue searching for the *real* SMBIOS entry point. Rather than stopping at the first match for the SMBIOS signature ("_SM_") in the f-segment (0xF0000-0xFFFFF), continue scanning until either a valid entry point table is found, or the f-segment has been exhausted. Reported-by: Bruce Rogers <brogers@suse.com> Signed-off-by: Gabriel Somlo <somlo@cmu.edu> Tested-by: Bruce Rogers <brogers@suse.com> --- - Added motivation for patch in commit log, as requested by mst (whom I forgot to cc on v2 -- D'OH!!! ) :) - Fixed s/withing/within/ as pointed out by eblake Thanks, Gabriel tests/bios-tables-test.c | 76 ++++++++++++++++++++++++++++-------------------- 1 file changed, 44 insertions(+), 32 deletions(-)