On 14/12/2016 18:00, Eduardo Habkost wrote: > On Wed, Dec 14, 2016 at 05:55:07PM +0100, Paolo Bonzini wrote: >> >> >> On 14/12/2016 17:19, Vlad Lungu wrote: >>> get_opt_value() truncates the value at the first comma. >>> Use memcpy() instead. >> >> Looks good since get_opt_value is already used by the caller. Have you >> tested this with multiple initrd modules too? > > get_opt_value() is used by the caller, but with NULL buf > argument. This means the caller doesn't handle ",," escaping. > (See my reply to the previous submission of this patch)
Hmm, wait. When NULL is passed, ",," escaping is handled correctly in that next_initrd points to the next lone comma. The lone comma is replaced with a '\0' by the caller. So you need to use get_opt_value again in mb_add_cmdline to do the unescaping, because mb_add_cmdline only receives double commas. This was actually my first reaction to the patch, and it was correct. Then I overthought it. :) So the patch is wrong. Paolo > However this only means the caller is buggy, not > mb_add_cmdline(). So the change still looks good to me. > >> >> Paolo >> >>> Signed-off-by: Vlad Lungu <vlad.lu...@windriver.com> >>> --- >>> 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); >>> + memcpy(b, cmdline, strlen(cmdline) + 1); >>> s->offset_cmdlines += strlen(b) + 1; >>> return s->mb_buf_phys + p; >>> } >>> >
