diff mbox

[2/3] multiboot: Fix offset of bootloader name

Message ID 1421321204-17764-3-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf Jan. 15, 2015, 11:26 a.m. UTC
This fixes a bug introduced in commit 5eba5a66 ('Add bootloader name to
multiboot implementation').

The calculation of the bootloader name offset didn't consider space
occupied by module command lines, so some unlucky module got its command
line partially overwritten with a "qemu" string.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 hw/i386/multiboot.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

Comments

Max Reitz Jan. 15, 2015, 5:08 p.m. UTC | #1
On 2015-01-15 at 06:26, Kevin Wolf wrote:
> This fixes a bug introduced in commit 5eba5a66 ('Add bootloader name to
> multiboot implementation').
>
> The calculation of the bootloader name offset didn't consider space
> occupied by module command lines, so some unlucky module got its command
> line partially overwritten with a "qemu" string.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   hw/i386/multiboot.c | 16 +++++++++-------
>   1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
> index f86d351..1adbe9e 100644
> --- a/hw/i386/multiboot.c
> +++ b/hw/i386/multiboot.c
> @@ -156,6 +156,7 @@ int load_multiboot(FWCfgState *fw_cfg,
>       MultibootState mbs;
>       uint8_t bootinfo[MBI_SIZE];
>       uint8_t *mb_bootinfo_data;
> +    uint32_t cmdline_len;
>   
>       /* Ok, let's see if it is a multiboot image.
>          The header is 12x32bit long, so the latest entry may be 8192 - 48. */
> @@ -258,27 +259,28 @@ int load_multiboot(FWCfgState *fw_cfg,
>       mbs.offset_mbinfo = mbs.mb_buf_size;
>   
>       /* Calculate space for cmdlines, bootloader name, and mb_mods */
> -    mbs.mb_buf_size += strlen(kernel_filename) + 1;
> -    mbs.mb_buf_size += strlen(kernel_cmdline) + 1;
> -    mbs.mb_buf_size += strlen(bootloader_name) + 1;
> +    cmdline_len = strlen(kernel_filename) + 1;
> +    cmdline_len += strlen(kernel_cmdline) + 1;
>       if (initrd_filename) {
>           const char *r = initrd_filename;
> -        mbs.mb_buf_size += strlen(r) + 1;
> +        cmdline_len += strlen(r) + 1;
>           mbs.mb_mods_avail = 1;
>           while (*(r = get_opt_value(NULL, 0, r))) {
>              mbs.mb_mods_avail++;
>              r++;
>           }
> -        mbs.mb_buf_size += MB_MOD_SIZE * mbs.mb_mods_avail;
>       }
>   
> +    mbs.mb_buf_size += cmdline_len;
> +    mbs.mb_buf_size += MB_MOD_SIZE * mbs.mb_mods_avail;
> +    mbs.mb_buf_size += strlen(bootloader_name) + 1;

I would have swapped these two lines to reflect their order in mb_buf, 
but it's fine either way.

> +
>       mbs.mb_buf_size = TARGET_PAGE_ALIGN(mbs.mb_buf_size);
>   
>       /* enlarge mb_buf to hold cmdlines, bootloader, mb-info structs */
>       mbs.mb_buf            = g_realloc(mbs.mb_buf, mbs.mb_buf_size);
>       mbs.offset_cmdlines   = mbs.offset_mbinfo + mbs.mb_mods_avail * MB_MOD_SIZE;
> -    mbs.offset_bootloader = mbs.offset_cmdlines + strlen(kernel_filename) + 1
> -                            + strlen(kernel_cmdline) + 1;
> +    mbs.offset_bootloader = mbs.offset_cmdlines + cmdline_len;
>   
>       if (initrd_filename) {
>           char *next_initrd, not_last;

Reviewed-by: Max Reitz <mreitz@redhat.com>
diff mbox

Patch

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index f86d351..1adbe9e 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -156,6 +156,7 @@  int load_multiboot(FWCfgState *fw_cfg,
     MultibootState mbs;
     uint8_t bootinfo[MBI_SIZE];
     uint8_t *mb_bootinfo_data;
+    uint32_t cmdline_len;
 
     /* Ok, let's see if it is a multiboot image.
        The header is 12x32bit long, so the latest entry may be 8192 - 48. */
@@ -258,27 +259,28 @@  int load_multiboot(FWCfgState *fw_cfg,
     mbs.offset_mbinfo = mbs.mb_buf_size;
 
     /* Calculate space for cmdlines, bootloader name, and mb_mods */
-    mbs.mb_buf_size += strlen(kernel_filename) + 1;
-    mbs.mb_buf_size += strlen(kernel_cmdline) + 1;
-    mbs.mb_buf_size += strlen(bootloader_name) + 1;
+    cmdline_len = strlen(kernel_filename) + 1;
+    cmdline_len += strlen(kernel_cmdline) + 1;
     if (initrd_filename) {
         const char *r = initrd_filename;
-        mbs.mb_buf_size += strlen(r) + 1;
+        cmdline_len += strlen(r) + 1;
         mbs.mb_mods_avail = 1;
         while (*(r = get_opt_value(NULL, 0, r))) {
            mbs.mb_mods_avail++;
            r++;
         }
-        mbs.mb_buf_size += MB_MOD_SIZE * mbs.mb_mods_avail;
     }
 
+    mbs.mb_buf_size += cmdline_len;
+    mbs.mb_buf_size += MB_MOD_SIZE * mbs.mb_mods_avail;
+    mbs.mb_buf_size += strlen(bootloader_name) + 1;
+
     mbs.mb_buf_size = TARGET_PAGE_ALIGN(mbs.mb_buf_size);
 
     /* enlarge mb_buf to hold cmdlines, bootloader, mb-info structs */
     mbs.mb_buf            = g_realloc(mbs.mb_buf, mbs.mb_buf_size);
     mbs.offset_cmdlines   = mbs.offset_mbinfo + mbs.mb_mods_avail * MB_MOD_SIZE;
-    mbs.offset_bootloader = mbs.offset_cmdlines + strlen(kernel_filename) + 1 
-                            + strlen(kernel_cmdline) + 1;
+    mbs.offset_bootloader = mbs.offset_cmdlines + cmdline_len;
 
     if (initrd_filename) {
         char *next_initrd, not_last;