Re: [Qemu-devel] [PATCH RFC 4/7] hw/apci: fix pcihp io initialization

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 31 May 2016 20:48:35 +0300
Marcel Apfelbaum <marcel@redhat.com> wrote:

> The pm initialization code assigns the pcihp IO base and length to -1 on error,
> but the later code will assume 0 as invalid value.
> 
> Fix it initializing the above value to 0 as expected.
> 
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
> ---
>  hw/i386/acpi-build.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 0c329fb..2097c4c 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -124,17 +124,19 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
>      Object *lpc = ich9_lpc_find();
>      Object *obj = NULL;
>      QObject *o;
> +    int pcihp_io_len, pcihp_io_base;
>  
>      pm->cpu_hp_io_base = 0;
> -    pm->pcihp_io_base = 0;
> -    pm->pcihp_io_len = 0;
this introduces uninitialized memory access in q35 case

>      if (piix) {
>          obj = piix;
>          pm->cpu_hp_io_base = PIIX4_CPU_HOTPLUG_IO_BASE;
> -        pm->pcihp_io_base =
> +        pcihp_io_base =
>              object_property_get_int(obj, ACPI_PCIHP_IO_BASE_PROP, NULL);
> -        pm->pcihp_io_len =
> +        pcihp_io_len =
>              object_property_get_int(obj, ACPI_PCIHP_IO_LEN_PROP, NULL);
> +
> +        pm->pcihp_io_base = (pcihp_io_base == -1) ? 0 : pcihp_io_base;
> +        pm->pcihp_io_len = (pcihp_io_len == -1) ? 0 : pcihp_io_len;
>      }
>      if (lpc) {
>          obj = lpc;

how about something like that:

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4f9aec6..d753e25 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -125,7 +125,7 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
 
     pm->cpu_hp_io_base = 0;
     pm->pcihp_io_base = 0;
-    pm->pcihp_io_len = 0;
+    pm->pcihp_io_len = UINT16_MAX;
     if (piix) {
         obj = piix;
         pm->cpu_hp_io_base = PIIX4_CPU_HOTPLUG_IO_BASE;
@@ -2053,7 +2053,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
     g_ptr_array_free(mem_ranges, true);
 
     /* reserve PCIHP resources */
-    if (pm->pcihp_io_len) {
+    if (pm->pcihp_io_len != UINT16_MAX) {
         dev = aml_device("PHPR");
         aml_append(dev, aml_name_decl("_HID", aml_string("PNP0A06")));
         aml_append(dev,