Hi All,

On Wednesday 11 October 2017 03:11 PM, Punit Agrawal wrote:
> The kernel crashes while iterating over a redistributor that is
> in-correctly sized by the platform firmware or doesn't contain the last
> record.
> 
> Prevent the crash by checking accesses against the size of the region
> provided by the firmware. While we are at it, warn the user about
> incorrect region size.
> 
> Signed-off-by: Punit Agrawal <punit.agra...@arm.com>
> Cc: Marc Zyngier <marc.zyng...@arm.com>

Sorry to bring up an old thread. Just wanted to check what is the status
on this series.

This will also be useful when we try to boot linux + hypervisor with
less number of cores than the SoC supports. For example:
- SoC has 4 cores and Linux tries to boot with 2 cores.
- then a type-2 hypervisor gets installed.
- Hypervisor tries to boot a VM with linux on core 1.

Now the VM boot will fail while it iterates over all the GICR regions
till GICR_TYPER is found. Hypervisor will trap any accesses to GICR
regions of any invalid cpus(cpu 2, cpu 3 in this case).

If the $patch is not the right approach, can you suggest on how to
handle the above scenario?

Thanks and regards,
Lokesh

> ---
>  drivers/irqchip/irq-gic-v3.c | 48 
> ++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 881d327c53fa..754d936c95e5 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -429,11 +429,21 @@ static int gic_iterate_rdists(int (*fn)(struct 
> redist_region *, void __iomem *))
>       int i;
>  
>       for (i = 0; i < gic_data.nr_redist_regions; i++) {
> -             void __iomem *ptr = gic_data.redist_regions[i].redist_base;
>               struct resource *res = &gic_data.redist_regions[i].res;
> -             u64 typer;
> +             void __iomem *ptr, *base;
> +             u64 typer, size, stride;
>               u32 reg;
>  
> +             ptr = base = gic_data.redist_regions[i].redist_base;
> +             size = resource_size(res);
> +
> +             stride = gic_data.redist_stride ?: SZ_64K * 2;
> +             if (ptr + stride > base + size) {
> +                     pr_warn("Insufficient size for redistributor region 
> @%llx. Skipping\n",
> +                             res->start);
> +                     continue;
> +             }
> +
>               reg = readl_relaxed(ptr + GICR_PIDR2) & GIC_PIDR2_ARCH_MASK;
>               if (reg != GIC_PIDR2_ARCH_GICv3 &&
>                   reg != GIC_PIDR2_ARCH_GICv4) { /* We're in trouble... */
> @@ -442,7 +452,28 @@ static int gic_iterate_rdists(int (*fn)(struct 
> redist_region *, void __iomem *))
>               }
>  
>               do {
> +                     /*
> +                      * We can access GICR_TYPER as we have already
> +                      * checked that we have atleast 128kB or
> +                      * redist_stride
> +                      */
>                       typer = gic_read_typer(ptr + GICR_TYPER);
> +                     if (!gic_data.redist_stride &&
> +                         (typer & GICR_TYPER_VLPIS)) {
> +                             /* VLPI_base + reserved page */
> +                             stride += SZ_64K * 2;
> +
> +                             /*
> +                              * We are larger than we thought, do
> +                              * we still fit?
> +                              */
> +                             if (ptr + stride > base + size) {
> +                                     pr_warn("No last record found in 
> redistributor region @%llx\n",
> +                                             
> gic_data.redist_regions[i].res.start);
> +                                     break;
> +                             }
> +                     }
> +
>                       ret = fn(gic_data.redist_regions + i, ptr);
>                       if (!ret)
>                               return 0;
> @@ -450,12 +481,13 @@ static int gic_iterate_rdists(int (*fn)(struct 
> redist_region *, void __iomem *))
>                       if (gic_data.redist_regions[i].single_redist)
>                               break;
>  
> -                     if (gic_data.redist_stride) {
> -                             ptr += gic_data.redist_stride;
> -                     } else {
> -                             ptr += SZ_64K * 2; /* Skip RD_base + SGI_base */
> -                             if (typer & GICR_TYPER_VLPIS)
> -                                     ptr += SZ_64K * 2; /* Skip VLPI_base + 
> reserved page */
> +                     ptr += stride;
> +
> +                     stride = gic_data.redist_stride ?: SZ_64K * 2;
> +                     if (ptr + stride > base + size) {
> +                             pr_warn("No last record found in redistributor 
> region @%llx\n",
> +                                     gic_data.redist_regions[i].res.start);
> +                             break;
>                       }
>               } while (!(typer & GICR_TYPER_LAST));
>       }
> 

Reply via email to