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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <lsq.1539530741.111236195@decadent.org.uk>
Date:   Sun, 14 Oct 2018 16:25:41 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, hpa@...or.com,
        "Thomas Gleixner" <tglx@...utronix.de>,
        "Peter Zijlstra" <peterz@...radead.org>,
        "Li RongQing" <lirongqing@...du.com>, jgross@...e.com,
        "Dou Liyang" <douly.fnst@...fujitsu.com>
Subject: [PATCH 3.16 130/366] x86/apic: Fix signedness bug in APIC ID
 validity checks

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

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

From: Li RongQing <lirongqing@...du.com>

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@...du.com>
Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
Cc: jgross@...e.com
Cc: Dou Liyang <douly.fnst@...fujitsu.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: hpa@...or.com
Link: https://lkml.kernel.org/r/1523322966-10296-1-git-send-email-lirongqing@baidu.com
[bwh: Backported to 3.16:
 - Drop change to xen_id_always_valid()
 - Adjust filenames, context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- 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;
 }