Re: [PATCH 1/1] s390x/s390-virtio-ccw: fix off-by-one in loadparm getter

[Halil Pasic <pasic@linux.ibm.com>]

On Thu, 30 Jul 2020 12:26:56 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Wed, 29 Jul 2020 15:02:22 +0200
> Halil Pasic <pasic@linux.ibm.com> wrote:
> 
> > As pointed out by Peter, g_memdup(ms->loadparm, sizeof(ms->loadparm) + 1)
> > reads one past of the end of ms->loadparm, so g_memdup() can not be used
> > here.
> > 
> > Let's use malloc and memcpy instead!
> 
> Hm, an alternative would be to use g_strndup(). What do you think?

Sure. It is more concise and does exactly what we want. I'm not too
familiar with the string utility funcitons of glib, so it didn't jup
at me.

Shall I spin a v2?

Halil

> 
> > 
> > Fixes: d664548328 ("s390x/s390-virtio-ccw: fix loadparm property getter")
> > Fixes: Coverity CID 1431058
> > Reported-by: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Halil Pasic <pasic@linux.ibm.com>
> > ---
> >  hw/s390x/s390-virtio-ccw.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index 403d30e13b..8b7bac0392 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -704,8 +704,8 @@ static char *machine_get_loadparm(Object *obj, Error **errp)
> >      char *loadparm_str;
> >  
> >      /* make a NUL-terminated string */
> > -    loadparm_str = g_memdup(ms->loadparm, sizeof(ms->loadparm) + 1);
> > -    loadparm_str[sizeof(ms->loadparm)] = 0;
> > +    loadparm_str = g_malloc0(sizeof(ms->loadparm) + 1);
> > +    memcpy(loadparm_str, ms->loadparm, sizeof(ms->loadparm));
> >      return loadparm_str;
> >  }
> >  
> > 
> > base-commit: 5772f2b1fc5d00e7e04e01fa28e9081d6550440a
> 
>