[PATCH 3.16 130/366] x86/apic: Fix signedness bug in APIC ID validity checks

From: Ben Hutchings
Date: Sun Oct 14 2018 - 11:47:45 EST


3.16.60-rc1 review patch. If anyone has any objections, please let me know.

------------------

From: Li RongQing <lirongqing@xxxxxxxxx>

commit a774635db5c430cbf21fa5d2f2df3d23aaa8e782 upstream.

The APIC ID as parsed from ACPI MADT is validity checked with the
apic->apic_id_valid() callback, which depends on the selected APIC type.

For non X2APIC types APIC IDs >= 0xFF are invalid, but values > 0x7FFFFFFF
are detected as valid. This happens because the 'apicid' argument of the
apic_id_valid() callback is type 'int'. So the resulting comparison

apicid < 0xFF

evaluates to true for all unsigned int values > 0x7FFFFFFF which are handed
to default_apic_id_valid(). As a consequence, invalid APIC IDs in !X2APIC
mode are considered valid and accounted as possible CPUs.

Change the apicid argument type of the apic_id_valid() callback to u32 so
the evaluation is unsigned and returns the correct result.

[ tglx: Massaged changelog ]

Signed-off-by: Li RongQing <lirongqing@xxxxxxxxx>
Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: jgross@xxxxxxxx
Cc: Dou Liyang <douly.fnst@xxxxxxxxxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: hpa@xxxxxxxxx
Link: https://lkml.kernel.org/r/1523322966-10296-1-git-send-email-lirongqing@xxxxxxxxx
[bwh: Backported to 3.16:
- Drop change to xen_id_always_valid()
- Adjust filenames, context]
Signed-off-by: Ben Hutchings <ben@xxxxxxxxxxxxxxx>
---
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -288,7 +288,7 @@ struct apic {

int (*probe)(void);
int (*acpi_madt_oem_check)(char *oem_id, char *oem_table_id);
- int (*apic_id_valid)(int apicid);
+ int (*apic_id_valid)(u32 apicid);
int (*apic_id_registered)(void);

u32 irq_delivery_mode;
@@ -547,7 +547,7 @@ static inline unsigned int read_apic_id(
return apic->get_apic_id(reg);
}

-static inline int default_apic_id_valid(int apicid)
+static inline int default_apic_id_valid(u32 apicid)
{
return (apicid < 255);
}
--- a/arch/x86/include/asm/x2apic.h
+++ b/arch/x86/include/asm/x2apic.h
@@ -9,7 +9,7 @@
#include <asm/ipi.h>
#include <linux/cpumask.h>

-static int x2apic_apic_id_valid(int apicid)
+static int x2apic_apic_id_valid(u32 apicid)
{
return 1;
}
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -215,7 +215,7 @@ static int __init
acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
{
struct acpi_madt_local_x2apic *processor = NULL;
- int apic_id;
+ u32 apic_id;
u8 enabled;

processor = (struct acpi_madt_local_x2apic *)header;
@@ -235,10 +235,13 @@ acpi_parse_x2apic(struct acpi_subtable_h
* to not preallocating memory for all NR_CPUS
* when we use CPU hotplug.
*/
- if (!apic->apic_id_valid(apic_id) && enabled)
- printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
- else
- acpi_register_lapic(apic_id, enabled);
+ if (!apic->apic_id_valid(apic_id)) {
+ if (enabled)
+ pr_warn(PREFIX "x2apic entry ignored\n");
+ return 0;
+ }
+
+ acpi_register_lapic(apic_id, enabled);
#else
printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
#endif
--- a/arch/x86/kernel/apic/apic_numachip.c
+++ b/arch/x86/kernel/apic/apic_numachip.c
@@ -58,7 +58,7 @@ static unsigned int read_xapic_id(void)
return get_apic_id(apic_read(APIC_ID));
}

-static int numachip_apic_id_valid(int apicid)
+static int numachip_apic_id_valid(u32 apicid)
{
/* Trust what bootloader passes in MADT */
return 1;
--- a/arch/x86/kernel/apic/x2apic_uv_x.c
+++ b/arch/x86/kernel/apic/x2apic_uv_x.c
@@ -272,7 +272,7 @@ static void uv_send_IPI_all(int vector)
uv_send_IPI_mask(cpu_online_mask, vector);
}

-static int uv_apic_id_valid(int apicid)
+static int uv_apic_id_valid(u32 apicid)
{
return 1;
}