[Qemu-devel] [PULL 0/4] fw_cfg 20190104 patches

[Qemu-devel] [PULL 0/4] fw_cfg 20190104 patches

[Philippe Mathieu-Daudé]

Hi Peter,

The following changes since commit 6395fe0c2c7d9f336d87960a7c9924b630c57c91:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-01-04 13:22:51 +0000)

are available in the Git repository at:

  https://github.com/philmd/qemu.git tags/fw_cfg-20190104-pull-request

for you to fetch changes up to 19bcc4bc3213e78c303ad480a7a578f62258252d:

  fw_cfg: Make qemu_extra_params_fw locally (2019-01-04 15:30:52 +0100)

----------------------------------------------------------------

fw_cfg patches for 2019-01-04

Two fixes from Li Qiang:
- Improve error message when can't load splash file
- Fix boot bootsplash and reboot-timeout error checking

----------------------------------------------------------------

Li Qiang (4):
  fw_cfg: Improve error message when can't load splash file
  fw_cfg: Fix -boot bootsplash error checking
  fw_cfg: Fix -boot reboot-timeout error checking
  fw_cfg: Make qemu_extra_params_fw locally

 hw/nvram/fw_cfg.c       | 70 ++++++++++++++++++-----------------------
 include/sysemu/sysemu.h |  1 -
 vl.c                    |  5 ++-
 3 files changed, 32 insertions(+), 44 deletions(-)

-- 
2.17.2

[Qemu-devel] [PULL 1/4] fw_cfg: Improve error message when can't load splash file

[Philippe Mathieu-Daudé]

From: Li Qiang <liq3ea@gmail.com>

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 <liq3ea@gmail.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <1541052148-28752-1-git-send-email-liq3ea@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 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;
     }
-- 
2.17.2

[Qemu-devel] [PULL 2/4] fw_cfg: Fix -boot bootsplash error checking

[Philippe Mathieu-Daudé]

From: Li Qiang <liq3ea@gmail.com>

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 <liq3ea@gmail.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <1542777026-2788-2-git-send-email-liq3ea@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 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,
-- 
2.17.2

[Qemu-devel] [PULL 3/4] fw_cfg: Fix -boot reboot-timeout error checking

[Philippe Mathieu-Daudé]

From: Li Qiang <liq3ea@gmail.com>

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 <liq3ea@gmail.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <1542777026-2788-3-git-send-email-liq3ea@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 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,
-- 
2.17.2

[Qemu-devel] [PULL 4/4] fw_cfg: Make qemu_extra_params_fw locally

[Philippe Mathieu-Daudé]

From: Li Qiang <liq3ea@gmail.com>

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 <liq3ea@gmail.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
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é <philmd@redhat.com>
---
 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;
-- 
2.17.2

Re: [Qemu-devel] [PULL 0/4] fw_cfg 20190104 patches

[Peter Maydell]

On Fri, 4 Jan 2019 at 16:32, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Hi Peter,
>
> The following changes since commit 6395fe0c2c7d9f336d87960a7c9924b630c57c91:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-01-04 13:22:51 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/philmd/qemu.git tags/fw_cfg-20190104-pull-request
>
> for you to fetch changes up to 19bcc4bc3213e78c303ad480a7a578f62258252d:
>
>   fw_cfg: Make qemu_extra_params_fw locally (2019-01-04 15:30:52 +0100)
>
> ----------------------------------------------------------------
>
> fw_cfg patches for 2019-01-04
>
> Two fixes from Li Qiang:
> - Improve error message when can't load splash file
> - Fix boot bootsplash and reboot-timeout error checking
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0
for any user-visible changes.

-- PMM