hw/i386/pc.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
QEMU will crash when device-memory-region-size property is read if ms->device_memory
wasn't initialized yet.
Crash can be reproduced with:
$QEMU -preconfig -qmp unix:qmp_socket,server,nowait &
./scripts/qmp/qom-get -s qmp_socket /machine.device-memory-region-size
Instead of crashing return 0 if ms->device_memory hasn't been initialized.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
add reproducer to commit message
(Markus Armbruster <armbru@redhat.com>)
hw/i386/pc.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index edc240b..1b7ead9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2459,7 +2459,11 @@ pc_machine_get_device_memory_region_size(Object *obj, Visitor *v,
Error **errp)
{
MachineState *ms = MACHINE(obj);
- int64_t value = memory_region_size(&ms->device_memory->mr);
+ int64_t value = 0;
+
+ if (ms->device_memory) {
+ memory_region_size(&ms->device_memory->mr);
+ }
visit_type_int(v, name, &value, errp);
}
--
2.7.4
On 10/06/19 15:50, Igor Mammedov wrote: > QEMU will crash when device-memory-region-size property is read if ms->device_memory > wasn't initialized yet. > > Crash can be reproduced with: > $QEMU -preconfig -qmp unix:qmp_socket,server,nowait & > ./scripts/qmp/qom-get -s qmp_socket /machine.device-memory-region-size > > Instead of crashing return 0 if ms->device_memory hasn't been initialized. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v2: > add reproducer to commit message > (Markus Armbruster <armbru@redhat.com>) > > hw/i386/pc.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index edc240b..1b7ead9 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -2459,7 +2459,11 @@ pc_machine_get_device_memory_region_size(Object *obj, Visitor *v, > Error **errp) > { > MachineState *ms = MACHINE(obj); > - int64_t value = memory_region_size(&ms->device_memory->mr); > + int64_t value = 0; > + > + if (ms->device_memory) { > + memory_region_size(&ms->device_memory->mr); > + } > > visit_type_int(v, name, &value, errp); > } > Queued, thanks. Paolo
On 10/06/19 15:50, Igor Mammedov wrote: > QEMU will crash when device-memory-region-size property is read if ms->device_memory > wasn't initialized yet. > > Crash can be reproduced with: > $QEMU -preconfig -qmp unix:qmp_socket,server,nowait & > ./scripts/qmp/qom-get -s qmp_socket /machine.device-memory-region-size > > Instead of crashing return 0 if ms->device_memory hasn't been initialized. This patch breaks bios-tables-test /x86_64/acpi/piix64/cpuhp: acpi-test: Warning! SRAT binary file mismatch. Actual [aml:/tmp/aml-RIFK3Z], Expected [aml:tests/data/acpi/pc/SRAT.memhp]. acpi-test: Warning! SRAT mismatch. Actual [asl:/tmp/asl-TLFK3Z.dsl, aml:/tmp/aml-RIFK3Z], Expected [asl:/tmp/asl-JL5J3Z.dsl, aml:tests/data/acpi/pc/SRAT.memhp]. ** ERROR:/home/pbonzini/work/upstream/qemu/tests/bios-tables-test.c:434:test_acpi_asl: assertion failed: (all_tables_match) ERROR - Bail out! ERROR:/home/pbonzini/work/upstream/qemu/tests/bios-tables-test.c:434:test_acpi_asl: assertion failed: (all_tables_match) So I'm removing it from the pull request. Paolo > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > v2: > add reproducer to commit message > (Markus Armbruster <armbru@redhat.com>) > > hw/i386/pc.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index edc240b..1b7ead9 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -2459,7 +2459,11 @@ pc_machine_get_device_memory_region_size(Object *obj, Visitor *v, > Error **errp) > { > MachineState *ms = MACHINE(obj); > - int64_t value = memory_region_size(&ms->device_memory->mr); > + int64_t value = 0; > + > + if (ms->device_memory) { > + memory_region_size(&ms->device_memory->mr); > + } > > visit_type_int(v, name, &value, errp); > } >
On Fri, Jun 21, 2019 at 02:29:29AM +0200, Paolo Bonzini wrote: > On 10/06/19 15:50, Igor Mammedov wrote: > > QEMU will crash when device-memory-region-size property is read if ms->device_memory > > wasn't initialized yet. > > > > Crash can be reproduced with: > > $QEMU -preconfig -qmp unix:qmp_socket,server,nowait & > > ./scripts/qmp/qom-get -s qmp_socket /machine.device-memory-region-size > > > > Instead of crashing return 0 if ms->device_memory hasn't been initialized. > > This patch breaks bios-tables-test /x86_64/acpi/piix64/cpuhp: > > acpi-test: Warning! SRAT binary file mismatch. Actual [aml:/tmp/aml-RIFK3Z], Expected [aml:tests/data/acpi/pc/SRAT.memhp]. > acpi-test: Warning! SRAT mismatch. Actual [asl:/tmp/asl-TLFK3Z.dsl, aml:/tmp/aml-RIFK3Z], Expected [asl:/tmp/asl-JL5J3Z.dsl, aml:tests/data/acpi/pc/SRAT.memhp]. > ** > ERROR:/home/pbonzini/work/upstream/qemu/tests/bios-tables-test.c:434:test_acpi_asl: assertion failed: (all_tables_match) > ERROR - Bail out! ERROR:/home/pbonzini/work/upstream/qemu/tests/bios-tables-test.c:434:test_acpi_asl: assertion failed: (all_tables_match) > > So I'm removing it from the pull request. The patch makes all memory regions return 0 as its size. > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > v2: > > add reproducer to commit message > > (Markus Armbruster <armbru@redhat.com>) > > > > hw/i386/pc.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index edc240b..1b7ead9 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -2459,7 +2459,11 @@ pc_machine_get_device_memory_region_size(Object *obj, Visitor *v, > > Error **errp) > > { > > MachineState *ms = MACHINE(obj); > > - int64_t value = memory_region_size(&ms->device_memory->mr); > > + int64_t value = 0; > > + > > + if (ms->device_memory) { > > + memory_region_size(&ms->device_memory->mr); This was supposed to be: value = memory_region_size(&ms->device_memory->mr); > > + } > > > > visit_type_int(v, name, &value, errp); > > } > > > -- Eduardo
On Thu, 20 Jun 2019 22:46:15 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Fri, Jun 21, 2019 at 02:29:29AM +0200, Paolo Bonzini wrote: > > On 10/06/19 15:50, Igor Mammedov wrote: > > > QEMU will crash when device-memory-region-size property is read if ms->device_memory > > > wasn't initialized yet. > > > > > > Crash can be reproduced with: > > > $QEMU -preconfig -qmp unix:qmp_socket,server,nowait & > > > ./scripts/qmp/qom-get -s qmp_socket /machine.device-memory-region-size > > > > > > Instead of crashing return 0 if ms->device_memory hasn't been initialized. > > > > This patch breaks bios-tables-test /x86_64/acpi/piix64/cpuhp: > > > > acpi-test: Warning! SRAT binary file mismatch. Actual [aml:/tmp/aml-RIFK3Z], Expected [aml:tests/data/acpi/pc/SRAT.memhp]. > > acpi-test: Warning! SRAT mismatch. Actual [asl:/tmp/asl-TLFK3Z.dsl, aml:/tmp/aml-RIFK3Z], Expected [asl:/tmp/asl-JL5J3Z.dsl, aml:tests/data/acpi/pc/SRAT.memhp]. > > ** > > ERROR:/home/pbonzini/work/upstream/qemu/tests/bios-tables-test.c:434:test_acpi_asl: assertion failed: (all_tables_match) > > ERROR - Bail out! ERROR:/home/pbonzini/work/upstream/qemu/tests/bios-tables-test.c:434:test_acpi_asl: assertion failed: (all_tables_match) > > > > So I'm removing it from the pull request. > > The patch makes all memory regions return 0 as its size. Indeed, sorry for not testing before posting patch. I'll send properly tested v2. > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > --- > > > v2: > > > add reproducer to commit message > > > (Markus Armbruster <armbru@redhat.com>) > > > > > > hw/i386/pc.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > > index edc240b..1b7ead9 100644 > > > --- a/hw/i386/pc.c > > > +++ b/hw/i386/pc.c > > > @@ -2459,7 +2459,11 @@ pc_machine_get_device_memory_region_size(Object *obj, Visitor *v, > > > Error **errp) > > > { > > > MachineState *ms = MACHINE(obj); > > > - int64_t value = memory_region_size(&ms->device_memory->mr); > > > + int64_t value = 0; > > > + > > > + if (ms->device_memory) { > > > + memory_region_size(&ms->device_memory->mr); > > This was supposed to be: > > value = memory_region_size(&ms->device_memory->mr); > > > > + } > > > > > > visit_type_int(v, name, &value, errp); > > > } > > > > > >
© 2016 - 2024 Red Hat, Inc.