| Message ID | 20200730130156.35063-1-pasic@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/1] s390x/s390-virtio-ccw: fix off-by-one in loadparm getter | expand |
On Thu, 30 Jul 2020 at 14:02, 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 g_strndup instead! > > 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 | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 403d30e13b..e72c61d2ea 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -701,12 +701,9 @@ bool hpage_1m_allowed(void) > static char *machine_get_loadparm(Object *obj, Error **errp) > { > S390CcwMachineState *ms = S390_CCW_MACHINE(obj); > - char *loadparm_str; > > /* make a NUL-terminated string */ > - loadparm_str = g_memdup(ms->loadparm, sizeof(ms->loadparm) + 1); > - loadparm_str[sizeof(ms->loadparm)] = 0; > - return loadparm_str; > + return g_strndup((char *) ms->loadparm, sizeof(ms->loadparm)); > } Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 30.07.20 15:01, Halil Pasic 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 g_strndup instead! > > 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 | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 403d30e13b..e72c61d2ea 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -701,12 +701,9 @@ bool hpage_1m_allowed(void) > static char *machine_get_loadparm(Object *obj, Error **errp) > { > S390CcwMachineState *ms = S390_CCW_MACHINE(obj); > - char *loadparm_str; > > /* make a NUL-terminated string */ > - loadparm_str = g_memdup(ms->loadparm, sizeof(ms->loadparm) + 1); > - loadparm_str[sizeof(ms->loadparm)] = 0; > - return loadparm_str; > + return g_strndup((char *) ms->loadparm, sizeof(ms->loadparm)); > } > > static void machine_set_loadparm(Object *obj, const char *val, Error **errp) > > base-commit: 5772f2b1fc5d00e7e04e01fa28e9081d6550440a > Reviewed-by: David Hildenbrand <david@redhat.com>
On Thu, 30 Jul 2020 15:01:56 +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 g_strndup instead! > > 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 | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) Thanks, queued to s390-fixes.
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 403d30e13b..e72c61d2ea 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -701,12 +701,9 @@ bool hpage_1m_allowed(void) static char *machine_get_loadparm(Object *obj, Error **errp) { S390CcwMachineState *ms = S390_CCW_MACHINE(obj); - char *loadparm_str; /* make a NUL-terminated string */ - loadparm_str = g_memdup(ms->loadparm, sizeof(ms->loadparm) + 1); - loadparm_str[sizeof(ms->loadparm)] = 0; - return loadparm_str; + return g_strndup((char *) ms->loadparm, sizeof(ms->loadparm)); } static void machine_set_loadparm(Object *obj, const char *val, Error **errp)
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 g_strndup instead! 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 | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) base-commit: 5772f2b1fc5d00e7e04e01fa28e9081d6550440a