Re: [Qemu-devel] [PATCH] multiboot: copy the cmdline verbatim

[Eduardo Habkost]

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)

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 <address@hidden>
> > ---
> >  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;
> >  }
> > 

-- 
Eduardo