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

[Vladimir Sementsov-Ogievskiy <v.sementsov-og@mail.ru>]

29.03.2022 00:15, Maxim Davydov 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.

So, immediately after initialization, h->bus is NULL?

The significant bit is, is the value which we calculate without h->bus is correct or not? That should be covered by commit message.

> 
> 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;


-- 
Best regards,
Vladimir