From: "Michael S. Tsirkin" <mst@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
Igor Mammedov <imammedo@redhat.com>,
Ani Sinha <anisinha@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Richard Henderson <richard.henderson@linaro.org>,
Eduardo Habkost <eduardo@habkost.net>,
Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Subject: [PULL 15/24] smbios: handle errors consistently
Date: Mon, 18 Mar 2024 12:16:23 -0400 [thread overview]
Message-ID: <643e1c9ef9d90a6e80b82998d41c91302fef506b.1710778506.git.mst@redhat.com> (raw)
In-Reply-To: <cover.1710778506.git.mst@redhat.com>
From: Igor Mammedov <imammedo@redhat.com>
Current code uses mix of error_report()+exit(1)
and error_setg() to handle errors.
Use newer error_setg() everywhere, beside consistency
it will allow to detect error condition without killing
QEMU and attempt switch-over to SMBIOS3.x tables/entrypoint
in follow up patch.
while at it, clear smbios_tables pointer after freeing.
that will avoid double free if smbios_get_tables() is called
multiple times.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Ani Sinha <anisinha@redhat.com>
Message-Id: <20240314152302.2324164-13-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
include/hw/firmware/smbios.h | 4 ++--
hw/i386/fw_cfg.c | 3 ++-
hw/smbios/smbios.c | 34 ++++++++++++++++++++++------------
hw/smbios/smbios_legacy.c | 22 ++++++++++++++--------
4 files changed, 40 insertions(+), 23 deletions(-)
diff --git a/include/hw/firmware/smbios.h b/include/hw/firmware/smbios.h
index ccc51e72f5..d4b91d5a14 100644
--- a/include/hw/firmware/smbios.h
+++ b/include/hw/firmware/smbios.h
@@ -326,7 +326,7 @@ struct smbios_type_127 {
struct smbios_structure_header header;
} QEMU_PACKED;
-void smbios_validate_table(void);
+bool smbios_validate_table(Error **errp);
void smbios_add_usr_blob_size(size_t size);
void smbios_entry_add(QemuOpts *opts, Error **errp);
void smbios_set_cpuid(uint32_t version, uint32_t features);
@@ -334,7 +334,7 @@ void smbios_set_defaults(const char *manufacturer, const char *product,
const char *version,
bool uuid_encoded, SmbiosEntryPointType ep_type);
void smbios_set_default_processor_family(uint16_t processor_family);
-uint8_t *smbios_get_table_legacy(size_t *length);
+uint8_t *smbios_get_table_legacy(size_t *length, Error **errp);
void smbios_get_tables(MachineState *ms,
const struct smbios_phys_mem_area *mem_array,
const unsigned int mem_array_size,
diff --git a/hw/i386/fw_cfg.c b/hw/i386/fw_cfg.c
index d1281066f4..e387bf50d0 100644
--- a/hw/i386/fw_cfg.c
+++ b/hw/i386/fw_cfg.c
@@ -71,7 +71,8 @@ void fw_cfg_build_smbios(PCMachineState *pcms, FWCfgState *fw_cfg)
smbios_set_cpuid(cpu->env.cpuid_version, cpu->env.features[FEAT_1_EDX]);
if (pcmc->smbios_legacy_mode) {
- smbios_tables = smbios_get_table_legacy(&smbios_tables_len);
+ smbios_tables = smbios_get_table_legacy(&smbios_tables_len,
+ &error_fatal);
fw_cfg_add_bytes(fw_cfg, FW_CFG_SMBIOS_ENTRIES,
smbios_tables, smbios_tables_len);
return;
diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 242106fd87..f7ca7d77e3 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -19,7 +19,6 @@
#include "qemu/units.h"
#include "qapi/error.h"
#include "qemu/config-file.h"
-#include "qemu/error-report.h"
#include "qemu/module.h"
#include "qemu/option.h"
#include "sysemu/sysemu.h"
@@ -511,23 +510,25 @@ opts_init(smbios_register_config);
*/
#define SMBIOS_21_MAX_TABLES_LEN 0xffff
-static void smbios_check_type4_count(uint32_t expected_t4_count)
+static bool smbios_check_type4_count(uint32_t expected_t4_count, Error **errp)
{
if (smbios_type4_count && smbios_type4_count != expected_t4_count) {
- error_report("Expected %d SMBIOS Type 4 tables, got %d instead",
- expected_t4_count, smbios_type4_count);
- exit(1);
+ error_setg(errp, "Expected %d SMBIOS Type 4 tables, got %d instead",
+ expected_t4_count, smbios_type4_count);
+ return false;
}
+ return true;
}
-void smbios_validate_table(void)
+bool smbios_validate_table(Error **errp)
{
if (smbios_ep_type == SMBIOS_ENTRY_POINT_TYPE_32 &&
smbios_tables_len > SMBIOS_21_MAX_TABLES_LEN) {
- error_report("SMBIOS 2.1 table length %zu exceeds %d",
- smbios_tables_len, SMBIOS_21_MAX_TABLES_LEN);
- exit(1);
+ error_setg(errp, "SMBIOS 2.1 table length %zu exceeds %d",
+ smbios_tables_len, SMBIOS_21_MAX_TABLES_LEN);
+ return false;
}
+ return true;
}
bool smbios_skip_table(uint8_t type, bool required_table)
@@ -1151,15 +1152,18 @@ void smbios_get_tables(MachineState *ms,
smbios_build_type_41_table(errp);
smbios_build_type_127_table();
- smbios_check_type4_count(ms->smp.sockets);
- smbios_validate_table();
+ if (!smbios_check_type4_count(ms->smp.sockets, errp)) {
+ goto err_exit;
+ }
+ if (!smbios_validate_table(errp)) {
+ goto err_exit;
+ }
smbios_entry_point_setup();
/* return tables blob and entry point (anchor), and their sizes */
*tables = smbios_tables;
*tables_len = smbios_tables_len;
*anchor = (uint8_t *)&ep;
-
/* calculate length based on anchor string */
if (!strncmp((char *)&ep, "_SM_", 4)) {
*anchor_len = sizeof(struct smbios_21_entry_point);
@@ -1168,6 +1172,12 @@ void smbios_get_tables(MachineState *ms,
} else {
abort();
}
+
+ return;
+err_exit:
+ g_free(smbios_tables);
+ smbios_tables = NULL;
+ return;
}
static void save_opt(const char **dest, QemuOpts *opts, const char *name)
diff --git a/hw/smbios/smbios_legacy.c b/hw/smbios/smbios_legacy.c
index 21f143e738..a6544bf55a 100644
--- a/hw/smbios/smbios_legacy.c
+++ b/hw/smbios/smbios_legacy.c
@@ -19,7 +19,7 @@
#include "qemu/bswap.h"
#include "hw/firmware/smbios.h"
#include "sysemu/sysemu.h"
-#include "qemu/error-report.h"
+#include "qapi/error.h"
struct smbios_header {
uint16_t length;
@@ -128,7 +128,7 @@ static void smbios_build_type_1_fields(void)
}
}
-uint8_t *smbios_get_table_legacy(size_t *length)
+uint8_t *smbios_get_table_legacy(size_t *length, Error **errp)
{
int i;
size_t usr_offset;
@@ -136,15 +136,15 @@ uint8_t *smbios_get_table_legacy(size_t *length)
/* complain if fields were given for types > 1 */
if (find_next_bit(smbios_have_fields_bitmap,
SMBIOS_MAX_TYPE + 1, 2) < SMBIOS_MAX_TYPE + 1) {
- error_report("can't process fields for smbios "
+ error_setg(errp, "can't process fields for smbios "
"types > 1 on machine versions < 2.1!");
- exit(1);
+ goto err_exit;
}
if (test_bit(4, smbios_have_binfile_bitmap)) {
- error_report("can't process table for smbios "
- "type 4 on machine versions < 2.1!");
- exit(1);
+ error_setg(errp, "can't process table for smbios "
+ "type 4 on machine versions < 2.1!");
+ goto err_exit;
}
g_free(smbios_entries);
@@ -173,7 +173,13 @@ uint8_t *smbios_get_table_legacy(size_t *length)
smbios_build_type_0_fields();
smbios_build_type_1_fields();
- smbios_validate_table();
+ if (!smbios_validate_table(errp)) {
+ goto err_exit;
+ }
+
*length = smbios_entries_len;
return smbios_entries;
+err_exit:
+ g_free(smbios_entries);
+ return NULL;
}
--
MST
next prev parent reply other threads:[~2024-03-18 16:16 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-18 16:15 [PULL 00/24] virtio,pc,pci: bugfixes Michael S. Tsirkin
2024-03-18 16:15 ` [PULL 01/24] SMBIOS: fix long lines Michael S. Tsirkin
2024-03-18 16:15 ` [PULL 02/24] qapi: document PCIe Gen5/Gen6 speeds since 9.0 Michael S. Tsirkin
2024-03-18 16:15 ` [PULL 03/24] docs/specs/pvpanic: mark shutdown event as not implemented Michael S. Tsirkin
2024-03-18 16:15 ` [PULL 04/24] tests: smbios: make it possible to write SMBIOS only test Michael S. Tsirkin
2024-03-18 16:15 ` [PULL 05/24] tests: smbios: add test for -smbios type=11 option Michael S. Tsirkin
2024-03-18 16:15 ` [PULL 06/24] tests: smbios: add test for legacy mode CLI options Michael S. Tsirkin
2024-03-18 16:15 ` [PULL 07/24] smbios: cleanup smbios_get_tables() from legacy handling Michael S. Tsirkin
2024-03-18 16:15 ` [PULL 08/24] smbios: get rid of smbios_smp_sockets global Michael S. Tsirkin
2024-03-18 16:15 ` [PULL 09/24] smbios: get rid of smbios_legacy global Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 10/24] smbios: avoid mangling user provided tables Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 11/24] smbios: don't check type4 structures in legacy mode Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 12/24] smbios: add smbios_add_usr_blob_size() helper Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 13/24] smbios: rename/expose structures/bitmaps used by both legacy and modern code Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 14/24] smbios: build legacy mode code only for 'pc' machine Michael S. Tsirkin
2024-03-18 16:16 ` Michael S. Tsirkin [this message]
2024-03-18 16:16 ` [PULL 16/24] smbios: get rid of global smbios_ep_type Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 17/24] smbios: clear smbios_type4_count before building tables Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 18/24] smbios: extend smbios-entry-point-type with 'auto' value Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 19/24] smbios: in case of entry point is 'auto' try to build v2 tables 1st Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 20/24] smbios: error out when building type 4 table is not possible Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 21/24] tests: acpi/smbios: whitelist expected blobs Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 22/24] pc/q35: set SMBIOS entry point type to 'auto' by default Michael S. Tsirkin
2024-03-18 16:16 ` [PULL 23/24] tests: acpi: update expected SSDT.dimmpxm blob Michael S. Tsirkin
2024-03-18 16:17 ` [PULL 24/24] smbios: add extra comments to smbios_get_table_legacy() Michael S. Tsirkin
2024-03-19 14:25 ` [PULL 00/24] virtio,pc,pci: bugfixes Peter Maydell
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=643e1c9ef9d90a6e80b82998d41c91302fef506b.1710778506.git.mst@redhat.com \
--to=mst@redhat.com \
--cc=anisinha@redhat.com \
--cc=eduardo@habkost.net \
--cc=imammedo@redhat.com \
--cc=marcel.apfelbaum@gmail.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
/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 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).