diff mbox series

[v9,12/19] arm64: acpi: Harden get_cpu_for_acpi_id() against missing CPU entry

Message ID 20240430142434.10471-13-Jonathan.Cameron@huawei.com (mailing list archive)
State New, archived
Headers show
Series ACPI/arm64: add support for virtual cpu hotplug | expand

Commit Message

Jonathan Cameron April 30, 2024, 2:24 p.m. UTC
In a review discussion of the changes to support vCPU hotplug where
a check was added on the GICC being enabled if was was online, it was
noted that there is need to map back to the cpu and use that to index
into a cpumask. As such, a valid ID is needed.

If an MPIDR check fails in acpi_map_gic_cpu_interface() it is possible
for the entry in cpu_madt_gicc[cpu] == NULL.  This function would
then cause a NULL pointer dereference.   Whilst a path to trigger
this has not been established, harden this caller against the
possibility.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
v9: New patch in response to a question from Marc Zyngier.
    Taking the easy way out - harden against a possible condition rather
    than establishing it never happens!
---
 arch/arm64/include/asm/acpi.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Gavin Shan May 1, 2024, 11:10 a.m. UTC | #1
On 5/1/24 00:24, Jonathan Cameron wrote:
> In a review discussion of the changes to support vCPU hotplug where
> a check was added on the GICC being enabled if was was online, it was
                                                  ^^^^^^^

                                                  typo

> noted that there is need to map back to the cpu and use that to index
> into a cpumask. As such, a valid ID is needed.
> 
> If an MPIDR check fails in acpi_map_gic_cpu_interface() it is possible
> for the entry in cpu_madt_gicc[cpu] == NULL.  This function would
> then cause a NULL pointer dereference.   Whilst a path to trigger
> this has not been established, harden this caller against the
> possibility.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v9: New patch in response to a question from Marc Zyngier.
>      Taking the easy way out - harden against a possible condition rather
>      than establishing it never happens!
> ---
>   arch/arm64/include/asm/acpi.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 

With the typo corrected:

Reviewed-by: Gavin Shan <gshan@redhat.com>

> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index bc9a6656fc0c..a407f9cd549e 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -124,7 +124,8 @@ static inline int get_cpu_for_acpi_id(u32 uid)
>   	int cpu;
>   
>   	for (cpu = 0; cpu < nr_cpu_ids; cpu++)
> -		if (uid == get_acpi_id_for_cpu(cpu))
> +		if (acpi_cpu_get_madt_gicc(cpu) &&
> +		    uid == get_acpi_id_for_cpu(cpu))
>   			return cpu;
>   
>   	return -EINVAL;
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index bc9a6656fc0c..a407f9cd549e 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -124,7 +124,8 @@  static inline int get_cpu_for_acpi_id(u32 uid)
 	int cpu;
 
 	for (cpu = 0; cpu < nr_cpu_ids; cpu++)
-		if (uid == get_acpi_id_for_cpu(cpu))
+		if (acpi_cpu_get_madt_gicc(cpu) &&
+		    uid == get_acpi_id_for_cpu(cpu))
 			return cpu;
 
 	return -EINVAL;