Re: [PATCH v1 2/9] pci: add null-pointer check

[Igor Mammedov]

On Tue, 29 Mar 2022 00:15:32 +0300
Maxim Davydov <maxim.davydov@openvz.org> wrote:

> Call pci_bus_get_w64_range can fail with the segmentation fault. For
> example, this can happen during attempt to get pci-hole64-end

>" immediately after initialization"
this too vague, pls provide a better description 
and is possible a reproducer.

> 
> Signed-off-by: Maxim Davydov <maxim.davydov@openvz.org>
> ---
>  hw/pci-host/i440fx.c | 17 +++++++++++------
>  hw/pci-host/q35.c    | 17 +++++++++++------
>  2 files changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
> index e08716142b..71a114e551 100644
> --- a/hw/pci-host/i440fx.c
> +++ b/hw/pci-host/i440fx.c
> @@ -158,10 +158,12 @@ static uint64_t 
> i440fx_pcihost_get_pci_hole64_start_value(Object *obj)
>      PCIHostState *h = PCI_HOST_BRIDGE(obj);
>      I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
>      Range w64;
> -    uint64_t value;
> +    uint64_t value = 0;
>  
> -    pci_bus_get_w64_range(h->bus, &w64);
> -    value = range_is_empty(&w64) ? 0 : range_lob(&w64);
> +    if (h->bus) {
> +        pci_bus_get_w64_range(h->bus, &w64);
> +        value = range_is_empty(&w64) ? 0 : range_lob(&w64);
> +    }
>      if (!value && s->pci_hole64_fix) {
>          value = pc_pci_hole64_start();
>      }
> @@ -191,10 +193,13 @@ static void i440fx_pcihost_get_pci_hole64_end(Object 
> *obj, Visitor *v,
>      I440FXState *s = I440FX_PCI_HOST_BRIDGE(obj);
>      uint64_t hole64_start = i440fx_pcihost_get_pci_hole64_start_value(obj);
>      Range w64;
> -    uint64_t value, hole64_end;
> +    uint64_t value = 0;
> +    uint64_t hole64_end;
>  
> -    pci_bus_get_w64_range(h->bus, &w64);
> -    value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1;
> +    if (h->bus) {
> +        pci_bus_get_w64_range(h->bus, &w64);
> +        value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1;
> +    }
>      hole64_end = ROUND_UP(hole64_start + s->pci_hole64_size, 1ULL << 30);
>      if (s->pci_hole64_fix && value < hole64_end) {
>          value = hole64_end;
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index ab5a47aff5..d679fd85ef 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -124,10 +124,12 @@ static uint64_t 
> q35_host_get_pci_hole64_start_value(Object *obj)
>      PCIHostState *h = PCI_HOST_BRIDGE(obj);
>      Q35PCIHost *s = Q35_HOST_DEVICE(obj);
>      Range w64;
> -    uint64_t value;
> +    uint64_t value = 0;
>  
> -    pci_bus_get_w64_range(h->bus, &w64);
> -    value = range_is_empty(&w64) ? 0 : range_lob(&w64);
> +    if (h->bus) {
> +        pci_bus_get_w64_range(h->bus, &w64);
> +        value = range_is_empty(&w64) ? 0 : range_lob(&w64);
> +    }
>      if (!value && s->pci_hole64_fix) {
>          value = pc_pci_hole64_start();
>      }
> @@ -157,10 +159,13 @@ static void q35_host_get_pci_hole64_end(Object *obj, 
> Visitor *v,
>      Q35PCIHost *s = Q35_HOST_DEVICE(obj);
>      uint64_t hole64_start = q35_host_get_pci_hole64_start_value(obj);
>      Range w64;
> -    uint64_t value, hole64_end;
> +    uint64_t value = 0;
> +    uint64_t hole64_end;
>  
> -    pci_bus_get_w64_range(h->bus, &w64);
> -    value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1;
> +    if (h->bus) {
> +        pci_bus_get_w64_range(h->bus, &w64);
> +        value = range_is_empty(&w64) ? 0 : range_upb(&w64) + 1;
> +    }
>      hole64_end = ROUND_UP(hole64_start + s->mch.pci_hole64_size, 1ULL << 30);
>      if (s->pci_hole64_fix && value < hole64_end) {
>          value = hole64_end;