From bed66336771ecdcb788d394bdd081a78b843e509 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Wed, 31 Oct 2018 23:02:28 -0700 Subject: fw_cfg: Improve error message when can't load splash file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit read_splashfile() reports "failed to read splash file" without further details. Get the details from g_file_get_contents(), and include them in the error message. Also remove unnecessary 'res' variable. Signed-off-by: Li Qiang Reviewed-by: Markus Armbruster Reviewed-by: Philippe Mathieu-Daudé Message-Id: <1541052148-28752-1-git-send-email-liq3ea@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/nvram/fw_cfg.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 3cb726ff68..9c4409be5e 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -68,15 +68,14 @@ static char *read_splashfile(char *filename, gsize *file_sizep, int *file_typep) { GError *err = NULL; - gboolean res; gchar *content; int file_type; unsigned int filehead; int bmp_bpp; - res = g_file_get_contents(filename, &content, file_sizep, &err); - if (res == FALSE) { - error_report("failed to read splash file '%s'", filename); + if (!g_file_get_contents(filename, &content, file_sizep, &err)) { + error_report("failed to read splash file '%s': %s", + filename, err->message); g_error_free(err); return NULL; } -- cgit v1.2.3 From 6912bb0b3d3b140c70d8cdfd2dff77f9890d7f12 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Tue, 20 Nov 2018 21:10:24 -0800 Subject: fw_cfg: Fix -boot bootsplash error checking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fw_cfg_bootsplash() gets option parameter "splash-time" with qemu_opt_get(), then converts it to an integer by hand. It neglects to check that conversion for errors. This is needlessly complicated and error-prone. But as "splash-time not specified" is not the same as "splash-time=T" for any T, we need use qemu_opt_get() to check if splash time exists. This patch also make the qemu exit when finding or loading splash file failed. Signed-off-by: Li Qiang Reviewed-by: Markus Armbruster Reviewed-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Message-Id: <1542777026-2788-2-git-send-email-liq3ea@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/nvram/fw_cfg.c | 35 +++++++++++++---------------------- vl.c | 2 +- 2 files changed, 14 insertions(+), 23 deletions(-) diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index 9c4409be5e..ad91abdbb7 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -117,47 +117,38 @@ error: static void fw_cfg_bootsplash(FWCfgState *s) { - int boot_splash_time = -1; const char *boot_splash_filename = NULL; - char *p; + const char *boot_splash_time = NULL; char *filename, *file_data; gsize file_size; int file_type; - const char *temp; /* get user configuration */ QemuOptsList *plist = qemu_find_opts("boot-opts"); QemuOpts *opts = QTAILQ_FIRST(&plist->head); - if (opts != NULL) { - temp = qemu_opt_get(opts, "splash"); - if (temp != NULL) { - boot_splash_filename = temp; - } - temp = qemu_opt_get(opts, "splash-time"); - if (temp != NULL) { - p = (char *)temp; - boot_splash_time = strtol(p, &p, 10); - } - } + boot_splash_filename = qemu_opt_get(opts, "splash"); + boot_splash_time = qemu_opt_get(opts, "splash-time"); /* insert splash time if user configurated */ - if (boot_splash_time >= 0) { + if (boot_splash_time) { + int64_t bst_val = qemu_opt_get_number(opts, "splash-time", -1); /* validate the input */ - if (boot_splash_time > 0xffff) { - error_report("splash time is big than 65535, force it to 65535."); - boot_splash_time = 0xffff; + if (bst_val < 0 || bst_val > 0xffff) { + error_report("splash-time is invalid," + "it should be a value between 0 and 65535"); + exit(1); } /* use little endian format */ - qemu_extra_params_fw[0] = (uint8_t)(boot_splash_time & 0xff); - qemu_extra_params_fw[1] = (uint8_t)((boot_splash_time >> 8) & 0xff); + qemu_extra_params_fw[0] = (uint8_t)(bst_val & 0xff); + qemu_extra_params_fw[1] = (uint8_t)((bst_val >> 8) & 0xff); fw_cfg_add_file(s, "etc/boot-menu-wait", qemu_extra_params_fw, 2); } /* insert splash file if user configurated */ - if (boot_splash_filename != NULL) { + if (boot_splash_filename) { filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, boot_splash_filename); if (filename == NULL) { - error_report("failed to find file '%s'.", boot_splash_filename); + error_report("failed to find file '%s'", boot_splash_filename); return; } diff --git a/vl.c b/vl.c index 8353d3c718..a24e5e076d 100644 --- a/vl.c +++ b/vl.c @@ -338,7 +338,7 @@ static QemuOptsList qemu_boot_opts = { .type = QEMU_OPT_STRING, }, { .name = "splash-time", - .type = QEMU_OPT_STRING, + .type = QEMU_OPT_NUMBER, }, { .name = "reboot-timeout", .type = QEMU_OPT_STRING, -- cgit v1.2.3 From ee5d0f89de3e53cdb0dcf51acc1502b310ed3bd2 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Tue, 20 Nov 2018 21:10:25 -0800 Subject: fw_cfg: Fix -boot reboot-timeout error checking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fw_cfg_reboot() gets option parameter "reboot-timeout" with qemu_opt_get(), then converts it to an integer by hand. It neglects to check that conversion for errors, and fails to reject negative values. Positive values above the limit get reported and replaced by the limit. This patch checks for conversion errors properly, and reject all values outside 0...0xffff. Signed-off-by: Li Qiang Reviewed-by: Markus Armbruster Reviewed-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Message-Id: <1542777026-2788-3-git-send-email-liq3ea@gmail.com> Signed-off-by: Philippe Mathieu-Daudé --- hw/nvram/fw_cfg.c | 27 +++++++++++++-------------- vl.c | 2 +- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index ad91abdbb7..cba58344f5 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -176,26 +176,25 @@ static void fw_cfg_bootsplash(FWCfgState *s) static void fw_cfg_reboot(FWCfgState *s) { - int reboot_timeout = -1; - char *p; - const char *temp; + const char *reboot_timeout = NULL; + int64_t rt_val = -1; /* get user configuration */ QemuOptsList *plist = qemu_find_opts("boot-opts"); QemuOpts *opts = QTAILQ_FIRST(&plist->head); - if (opts != NULL) { - temp = qemu_opt_get(opts, "reboot-timeout"); - if (temp != NULL) { - p = (char *)temp; - reboot_timeout = strtol(p, &p, 10); + reboot_timeout = qemu_opt_get(opts, "reboot-timeout"); + + if (reboot_timeout) { + rt_val = qemu_opt_get_number(opts, "reboot-timeout", -1); + /* validate the input */ + if (rt_val < 0 || rt_val > 0xffff) { + error_report("reboot timeout is invalid," + "it should be a value between 0 and 65535"); + exit(1); } } - /* validate the input */ - if (reboot_timeout > 0xffff) { - error_report("reboot timeout is larger than 65535, force it to 65535."); - reboot_timeout = 0xffff; - } - fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(&reboot_timeout, 4), 4); + + fw_cfg_add_file(s, "etc/boot-fail-wait", g_memdup(&rt_val, 4), 4); } static void fw_cfg_write(FWCfgState *s, uint8_t value) diff --git a/vl.c b/vl.c index a24e5e076d..efa4ff9da5 100644 --- a/vl.c +++ b/vl.c @@ -341,7 +341,7 @@ static QemuOptsList qemu_boot_opts = { .type = QEMU_OPT_NUMBER, }, { .name = "reboot-timeout", - .type = QEMU_OPT_STRING, + .type = QEMU_OPT_NUMBER, }, { .name = "strict", .type = QEMU_OPT_BOOL, -- cgit v1.2.3 From 19bcc4bc3213e78c303ad480a7a578f62258252d Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Tue, 20 Nov 2018 21:10:26 -0800 Subject: fw_cfg: Make qemu_extra_params_fw locally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qemu_extra_params_fw[] has external linkage, but is used only in fw_cfg_bootsplash(), it makes sense to make it locally. Signed-off-by: Li Qiang Reviewed-by: Markus Armbruster Reviewed-by: Gerd Hoffmann Reviewed-by: Philippe Mathieu-Daudé Message-Id: <1542777026-2788-4-git-send-email-liq3ea@gmail.com> [PMD: Removed qemu_extra_params_fw declaration in vl.c] Signed-off-by: Philippe Mathieu-Daudé --- hw/nvram/fw_cfg.c | 1 + include/sysemu/sysemu.h | 1 - vl.c | 1 - 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c index cba58344f5..de58c7be46 100644 --- a/hw/nvram/fw_cfg.c +++ b/hw/nvram/fw_cfg.c @@ -119,6 +119,7 @@ static void fw_cfg_bootsplash(FWCfgState *s) { const char *boot_splash_filename = NULL; const char *boot_splash_time = NULL; + uint8_t qemu_extra_params_fw[2]; char *filename, *file_data; gsize file_size; int file_type; diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index e0d15da937..85877b7e43 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -116,7 +116,6 @@ extern uint8_t *boot_splash_filedata; extern size_t boot_splash_filedata_size; extern bool enable_mlock; extern bool enable_cpu_pm; -extern uint8_t qemu_extra_params_fw[2]; extern QEMUClockType rtc_clock; extern const char *mem_path; extern int mem_prealloc; diff --git a/vl.c b/vl.c index efa4ff9da5..0db5ad0246 100644 --- a/vl.c +++ b/vl.c @@ -191,7 +191,6 @@ int boot_menu; bool boot_strict; uint8_t *boot_splash_filedata; size_t boot_splash_filedata_size; -uint8_t qemu_extra_params_fw[2]; bool wakeup_suspend_enabled; int icount_align_option; -- cgit v1.2.3