linux-kernel - Re: [tip:x86/apic] x86/acpi: Introduce persistent storage for cpuid <-> apicid mapping

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 for Android: free password hash cracker in your pocket

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

Message-ID: <alpine.DEB.2.20.1610041258410.7712@nanos>
Date:   Wed, 5 Oct 2016 16:04:01 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Yinghai Lu <yinghai@...nel.org>
cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Gu Zheng <guz.fnst@...fujitsu.com>,
        Tang Chen <tangchen@...fujitsu.com>,
        Ingo Molnar <mingo@...nel.org>, douly.fnst@...fujitsu.com,
        zhugh.fnst@...fujitsu.com, "H. Peter Anvin" <hpa@...or.com>,
        "linux-tip-commits@...r.kernel.org" 
        <linux-tip-commits@...r.kernel.org>,
        Tony Luck <tony.luck@...el.com>
Subject: Re: [tip:x86/apic] x86/acpi: Introduce persistent storage for cpuid
 <-> apicid mapping

On Mon, 3 Oct 2016, Yinghai Lu wrote:
> init_cpu_node become:
> [   55.477160] init_cpu_to_node:
> [   55.483280] cpu 0 -> apicid 0x0 -> node 0
> [   55.491558] cpu 1 -> apicid 0xff -> node 1
> [   55.500017] cpu 2 -> apicid 0x2 -> node 0
> [   55.508296] cpu 3 -> apicid 0x4 -> node 0
> [   55.516575] cpu 4 -> apicid 0x6 -> node 0
> ...
> 
> looks like problem is
> 
> acpi_parse_lapic==>acpi_register_lapic==>__generic_processor_info==>allocate_logical_cpuid
> 
> it will take lapic_id[0xff] take cpu index 1.
> 
> Then will have not /dev/cpu/1/msr, that will make the MLC not happy.

That's pretty irrelevant whether that MLC thing - whatever it is - is
unhappy or not. If it cannot deal with something missing, it's surely not
the kernels problem. Unplug the cpu and the file will be missing as well.

> -static int acpi_register_lapic(int id, u32 acpiid, u8 enabled)
> +static int acpi_register_lapic(int id, u32 acpiid, u8 enabled, int disabled_id)
>  {
>      unsigned int ver = 0;
>      int cpu;

This is whitespace damaged. The patch does not apply. Can you finaly after
doing 10 years of kernel development start being more careful?

> @@ -176,6 +177,11 @@ static int acpi_register_lapic(int id, u
>          return -EINVAL;
>      }
> 
> +    if (!enabled && (id == disabled_id)) {
> +        ++disabled_cpus;
> +        return -EINVAL;
> +    }

Why would you need that disabled_id thing at all? The proper fix is to let
the apic driver detect the issue and this boils down to a 5 lines
change. Does the patch below fix the issue for you?

Thanks,

	tglx

8<----------------
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2076,6 +2076,11 @@ int __generic_processor_info(int apicid,
 	bool boot_cpu_detected = physid_isset(boot_cpu_physical_apicid,
 				phys_cpu_present_map);
 
+	if (!apic->apic_id_valid(apicid)) {
+		disabled_cpus++;
+		return -EINVAL;
+	}
+
 	/*
 	 * boot_cpu_physical_apicid is designed to have the apicid
 	 * returned by read_apic_id(), i.e, the apicid of the