* [PATCH 1/1] s390x/s390-virtio-ccw: fix loadparm property getter
@ 2020-07-23 16:27 Halil Pasic
2020-07-23 16:37 ` Thomas Huth
0 siblings, 1 reply; 4+ messages in thread
From: Halil Pasic @ 2020-07-23 16:27 UTC (permalink / raw)
To: Cornelia Huck, qemu-s390x, qemu-devel
Cc: Thomas Huth, David Hildenbrand, Halil Pasic,
Christian Borntraeger, Claudio Imbrenda, Richard Henderson
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(-)
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)
base-commit: 53ce7b47b5bf47db067b81c18c786ed7b792d031
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/1] s390x/s390-virtio-ccw: fix loadparm property getter
2020-07-23 16:27 [PATCH 1/1] s390x/s390-virtio-ccw: fix loadparm property getter Halil Pasic
@ 2020-07-23 16:37 ` Thomas Huth
2020-07-24 6:52 ` Cornelia Huck
2020-07-27 10:22 ` Halil Pasic
0 siblings, 2 replies; 4+ messages in thread
From: Thomas Huth @ 2020-07-23 16:37 UTC (permalink / raw)
To: Halil Pasic, Cornelia Huck, qemu-s390x, qemu-devel
Cc: Claudio Imbrenda, Christian Borntraeger, David Hildenbrand,
Richard Henderson
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>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1/1] s390x/s390-virtio-ccw: fix loadparm property getter
2020-07-23 16:37 ` Thomas Huth
@ 2020-07-24 6:52 ` Cornelia Huck
2020-07-27 10:22 ` Halil Pasic
1 sibling, 0 replies; 4+ messages in thread
From: Cornelia Huck @ 2020-07-24 6:52 UTC (permalink / raw)
To: Thomas Huth
Cc: David Hildenbrand, qemu-devel, Halil Pasic, Christian Borntraeger,
qemu-s390x, Claudio Imbrenda, Richard Henderson
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.)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1/1] s390x/s390-virtio-ccw: fix loadparm property getter
2020-07-23 16:37 ` Thomas Huth
2020-07-24 6:52 ` Cornelia Huck
@ 2020-07-27 10:22 ` Halil Pasic
1 sibling, 0 replies; 4+ messages in thread
From: Halil Pasic @ 2020-07-27 10:22 UTC (permalink / raw)
To: Thomas Huth
Cc: David Hildenbrand, Cornelia Huck, qemu-devel,
Christian Borntraeger, qemu-s390x, Claudio Imbrenda,
Richard Henderson
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-27 10:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-23 16:27 [PATCH 1/1] s390x/s390-virtio-ccw: fix loadparm property getter Halil Pasic
2020-07-23 16:37 ` Thomas Huth
2020-07-24 6:52 ` Cornelia Huck
2020-07-27 10:22 ` Halil Pasic
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).