| Message ID | 20200723162717.88485-1-pasic@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] s390x/s390-virtio-ccw: fix loadparm property getter | expand |
On 23/07/2020 18.27, Halil Pasic wrote: > The function machine_get_loadparm() is supposed to produce as C-string, sed "s/ as / a /" > that is a null-terminated one, but it does not. ElectricFence can detect maybe: sed "s/null/NUL/" > this problem if the loadparm machine property is used. > > Let us make the returned string a null-terminated one. dito > Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > Fixes: 7104bae9de ("hw/s390x: provide loadparm property for the machine") > --- > hw/s390x/s390-virtio-ccw.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 8cc2f25d8a..e0e4a69ac8 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -701,8 +701,12 @@ bool hpage_1m_allowed(void) > static char *machine_get_loadparm(Object *obj, Error **errp) > { > S390CcwMachineState *ms = S390_CCW_MACHINE(obj); > + char *loadparm_str; > > - return g_memdup(ms->loadparm, sizeof(ms->loadparm)); > + /* make a null-terminated string */ maybe: sed "s/null/NUL/" > + loadparm_str = g_memdup(ms->loadparm, sizeof(ms->loadparm) + 1); Wrong indentation. > + loadparm_str[sizeof(ms->loadparm)] = 0; > + return loadparm_str; > } With the cosmetics fixed: Reviewed-by: Thomas Huth <thuth@redhat.com>
On Thu, 23 Jul 2020 18:37:50 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 23/07/2020 18.27, Halil Pasic wrote: > > The function machine_get_loadparm() is supposed to produce as C-string, > > sed "s/ as / a /" > > > that is a null-terminated one, but it does not. ElectricFence can detect > > maybe: sed "s/null/NUL/" "NUL" seems to be the more commonly used form in QEMU, so I went ahead and changed it. > > > this problem if the loadparm machine property is used. > > > > Let us make the returned string a null-terminated one. > > dito > > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > > Fixes: 7104bae9de ("hw/s390x: provide loadparm property for the machine") > > --- > > hw/s390x/s390-virtio-ccw.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > > index 8cc2f25d8a..e0e4a69ac8 100644 > > --- a/hw/s390x/s390-virtio-ccw.c > > +++ b/hw/s390x/s390-virtio-ccw.c > > @@ -701,8 +701,12 @@ bool hpage_1m_allowed(void) > > static char *machine_get_loadparm(Object *obj, Error **errp) > > { > > S390CcwMachineState *ms = S390_CCW_MACHINE(obj); > > + char *loadparm_str; > > > > - return g_memdup(ms->loadparm, sizeof(ms->loadparm)); > > + /* make a null-terminated string */ > > maybe: sed "s/null/NUL/" > > > + loadparm_str = g_memdup(ms->loadparm, sizeof(ms->loadparm) + 1); > > Wrong indentation. > > > + loadparm_str[sizeof(ms->loadparm)] = 0; > > + return loadparm_str; > > } > > With the cosmetics fixed: > > Reviewed-by: Thomas Huth <thuth@redhat.com> Thanks, queued to s390-fixes (with the nits fixed.)
On Thu, 23 Jul 2020 18:37:50 +0200 Thomas Huth <thuth@redhat.com> wrote: > On 23/07/2020 18.27, Halil Pasic wrote: > > The function machine_get_loadparm() is supposed to produce as C-string, > > sed "s/ as / a /" > Nod. > > that is a null-terminated one, but it does not. ElectricFence can detect > > maybe: sed "s/null/NUL/" > https://en.wikipedia.org/wiki/Null-terminated_string but it does not matter to me all that much. > > this problem if the loadparm machine property is used. > > > > Let us make the returned string a null-terminated one. > > dito > > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > > Fixes: 7104bae9de ("hw/s390x: provide loadparm property for the machine") > > --- > > hw/s390x/s390-virtio-ccw.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > > index 8cc2f25d8a..e0e4a69ac8 100644 > > --- a/hw/s390x/s390-virtio-ccw.c > > +++ b/hw/s390x/s390-virtio-ccw.c > > @@ -701,8 +701,12 @@ bool hpage_1m_allowed(void) > > static char *machine_get_loadparm(Object *obj, Error **errp) > > { > > S390CcwMachineState *ms = S390_CCW_MACHINE(obj); > > + char *loadparm_str; > > > > - return g_memdup(ms->loadparm, sizeof(ms->loadparm)); > > + /* make a null-terminated string */ > > maybe: sed "s/null/NUL/" > > > + loadparm_str = g_memdup(ms->loadparm, sizeof(ms->loadparm) + 1); > > Wrong indentation. Nod. > > > + loadparm_str[sizeof(ms->loadparm)] = 0; > > + return loadparm_str; > > } > > With the cosmetics fixed: > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > Thanks! Regards, Halil
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 8cc2f25d8a..e0e4a69ac8 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -701,8 +701,12 @@ bool hpage_1m_allowed(void) static char *machine_get_loadparm(Object *obj, Error **errp) { S390CcwMachineState *ms = S390_CCW_MACHINE(obj); + char *loadparm_str; - return g_memdup(ms->loadparm, sizeof(ms->loadparm)); + /* make a null-terminated string */ + loadparm_str = g_memdup(ms->loadparm, sizeof(ms->loadparm) + 1); + loadparm_str[sizeof(ms->loadparm)] = 0; + return loadparm_str; } static void machine_set_loadparm(Object *obj, const char *val, Error **errp)
The function machine_get_loadparm() is supposed to produce as C-string, that is a null-terminated one, but it does not. ElectricFence can detect this problem if the loadparm machine property is used. Let us make the returned string a null-terminated one. Signed-off-by: Halil Pasic <pasic@linux.ibm.com> Fixes: 7104bae9de ("hw/s390x: provide loadparm property for the machine") --- hw/s390x/s390-virtio-ccw.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) base-commit: 53ce7b47b5bf47db067b81c18c786ed7b792d031