Skip to content

Commit

Permalink
x86/apic: Fix signedness bug in APIC ID validity checks
Browse files Browse the repository at this point in the history
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@baidu.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Cc: jgross@suse.com
Cc: Dou Liyang <douly.fnst@cn.fujitsu.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: hpa@zytor.com
Link: https://lkml.kernel.org/r/1523322966-10296-1-git-send-email-lirongqing@baidu.com
  • Loading branch information
Li RongQing authored and Thomas Gleixner committed Apr 10, 2018
1 parent d94a155 commit a774635
Show file tree
Hide file tree
Showing 8 changed files with 16 additions and 13 deletions.
4 changes: 2 additions & 2 deletions arch/x86/include/asm/apic.h
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ struct apic {
/* Probe, setup and smpboot functions */
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);

bool (*check_apicid_used)(physid_mask_t *map, int apicid);
Expand Down Expand Up @@ -492,7 +492,7 @@ static inline unsigned int read_apic_id(void)
return apic->get_apic_id(reg);
}

extern int default_apic_id_valid(int apicid);
extern int default_apic_id_valid(u32 apicid);
extern int default_acpi_madt_oem_check(char *, char *);
extern void default_setup_apic_routing(void);

Expand Down
13 changes: 8 additions & 5 deletions arch/x86/kernel/acpi/boot.c
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
{
struct acpi_madt_local_x2apic *processor = NULL;
#ifdef CONFIG_X86_X2APIC
int apic_id;
u32 apic_id;
u8 enabled;
#endif

Expand All @@ -222,10 +222,13 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
* 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, processor->uid, enabled);
if (!apic->apic_id_valid(apic_id)) {
if (enabled)
pr_warn(PREFIX "x2apic entry ignored\n");
return 0;
}

acpi_register_lapic(apic_id, processor->uid, enabled);
#else
printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
#endif
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/kernel/apic/apic_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ int default_check_phys_apicid_present(int phys_apicid)
return physid_isset(phys_apicid, phys_cpu_present_map);
}

int default_apic_id_valid(int apicid)
int default_apic_id_valid(u32 apicid)
{
return (apicid < 255);
}
2 changes: 1 addition & 1 deletion arch/x86/kernel/apic/apic_numachip.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ static u32 numachip2_set_apic_id(unsigned int id)
return id << 24;
}

static int numachip_apic_id_valid(int apicid)
static int numachip_apic_id_valid(u32 apicid)
{
/* Trust what bootloader passes in MADT */
return 1;
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/kernel/apic/x2apic.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* Common bits for X2APIC cluster/physical modes. */

int x2apic_apic_id_valid(int apicid);
int x2apic_apic_id_valid(u32 apicid);
int x2apic_apic_id_registered(void);
void __x2apic_send_IPI_dest(unsigned int apicid, int vector, unsigned int dest);
unsigned int x2apic_get_apic_id(unsigned long id);
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/kernel/apic/x2apic_phys.c
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ static int x2apic_phys_probe(void)
}

/* Common x2apic functions, also used by x2apic_cluster */
int x2apic_apic_id_valid(int apicid)
int x2apic_apic_id_valid(u32 apicid)
{
return 1;
}
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/kernel/apic/x2apic_uv_x.c
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,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;
}
Expand Down
2 changes: 1 addition & 1 deletion arch/x86/xen/apic.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ static int xen_madt_oem_check(char *oem_id, char *oem_table_id)
return xen_pv_domain();
}

static int xen_id_always_valid(int apicid)
static int xen_id_always_valid(u32 apicid)
{
return 1;
}
Expand Down

0 comments on commit a774635

Please sign in to comment.