Igor Mammedov <[email protected]> writes:
> QEMU will crash when device-memory-region-size property is read if
> ms->device_memory
> wasn't initialized yet (ex: property being inspected during preconfig time).
Reproduced:
$ qemu-system-x86_64 -nodefaults -S -display none -preconfig -qmp stdio
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 0, "major": 4},
"package": "v4.0.0-828-ga7b21f6762"}, "capabilities": ["oob"]}}
{"execute": "qmp_capabilities"}
{"return": {}}
{"execute": "qom-get", "arguments": {"path": "/machine", "property":
"device-memory-region-size"}}
Segmentation fault (core dumped)
First time I started looking at this series, I went "I'll need a
reproducer to fully understand what's up, and I don't feel like finding
one now; next series, please". Second time, I had to spend a few
minutes on the reproducer. Wasn't hard, since you provided a clue.
Still: make review easy, include a reproducer whenever you can.
> Instead of crashing return 0 if ms->device_memory hasn't been initialized.
>
> Signed-off-by: Igor Mammedov <[email protected]>
> ---
> 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 d98b737..de91e90 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -2461,7 +2461,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);
> }
This makes qom-get return 0 for the size of memory that doesn't exist,
yet.
A possible alternative would be setting an error.
Opinions?