[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim |
Date: |
Wed, 14 Dec 2016 12:38:23 -0200 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Wed, Dec 14, 2016 at 03:35:29PM +0200, Vlad Lungu wrote:
>
> get_opt_value() truncates the value at the first comma
> Use memcpy() instead
>
> Signed-off-by: Vlad Lungu <address@hidden>
Your patch is corrupted. I suggest using git-send-email instead
of Thunderbird, but you might want to take a look at:
http://kb.mozillazine.org/Plain_text_e-mail_-_Thunderbird#Completely_plain_email
> ---
> hw/i386/multiboot.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
> index 387caa6..b4495ad 100644
> --- a/hw/i386/multiboot.c
> +++ b/hw/i386/multiboot.c
> @@ -109,7 +109,7 @@ static uint32_t mb_add_cmdline(MultibootState *s, const
> char *cmdline)
> hwaddr p = s->offset_cmdlines;
> char *b = (char *)s->mb_buf + p;
>
> - get_opt_value(b, strlen(cmdline) + 1, cmdline);
One caller of mb_add_cmdline() relies on get_opt_value() to allow
multiple initrd modules to be specified on the command-line. But:
* The caller already splits the string manually by adding a null
terminator;
* The caller is already broken if filenames contain "," (because
it doesn't handle ",," escaping)
so it won't be affected by the change, and it would need to
handle escaping before calling mb_add_cmdline() anyway.
The other caller of mb_add_cmdline() (kcmdline) doesn't need
option splitting/parsing.
So, not using get_opt_value() here makes sense.
But:
> + memcpy(b, cmdline, strlen(cmdline) + 1);
This is equivalent to strcpy(), and it is ignoring the size of
mb_buf.
The existing code looks very dangerous: the size of mb_buf is
calculated based on the image contents, and then it doesn't check
the buffer size before writing to it.
> s->offset_cmdlines += strlen(b) + 1;
> return s->mb_buf_phys + p;
> }
> --
> 1.9.1
>
>
--
Eduardo
- [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim, Vlad Lungu, 2016/12/14
- Re: [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim, Paolo Bonzini, 2016/12/14
- Re: [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim, Eduardo Habkost, 2016/12/14
- Re: [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim, Paolo Bonzini, 2016/12/14
- Re: [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim, Eduardo Habkost, 2016/12/14
- Re: [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim, Paolo Bonzini, 2016/12/14
- Re: [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim, Eduardo Habkost, 2016/12/14
- Re: [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim, Paolo Bonzini, 2016/12/14
- Re: [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim, Vlad Lungu, 2016/12/15