linux-kernel - Re: [RFC PATCH] x86/acpi: Ignore invalid x2APIC entries

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]

Date:   Fri, 28 Jul 2023 16:47:40 +0000
From:   "Zhang, Rui" <rui.zhang@...el.com>
To:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "Wysocki, Rafael J" <rafael.j.wysocki@...el.com>,
        "bp@...en8.de" <bp@...en8.de>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Tang, Feng" <feng.tang@...el.com>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>
Subject: Re: [RFC PATCH] x86/acpi: Ignore invalid x2APIC entries

On Fri, 2023-07-28 at 14:51 +0200, Thomas Gleixner wrote:
> On Mon, Jul 03 2023 at 00:28, Zhang Rui wrote:
> >  
> > +static bool has_lapic_cpus;
> 
> Yet another random flag. Sigh.
> 
> I really hate this. Why not doing the obvious?
> 
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -2452,6 +2452,9 @@ int generic_processor_info(int apicid, i
>         bool boot_cpu_detected =
> physid_isset(boot_cpu_physical_apicid,
>                                 phys_cpu_present_map);
>  
> +       if (physid_isset(apicid, phys_cpu_present_map))
> +               return -EBUSY;
> +
>         /*
>          * boot_cpu_physical_apicid is designed to have the apicid
>          * returned by read_apic_id(), i.e, the apicid of the
> 
> As the call sites during MADT parsing ignore the return value anyway,
> there is no harm and this is a proper defense against broken tables
> which enumerate an APIC twice.

Yeah, this can fix the duplicate APIC ID issue.

But for x2APIC CPUs with unique APIC ID, but smaller than 255, should
we still enumerate them when we already have valid LAPIC entries?

For the Ivebridge-EP 2-socket system,

LAPIC: APIC ID from 0x0 - 0xB, 0x10 - 0x1B, 0x20 - 0x2B, 0x30 - 0x3B
x2APIC: APIC ID from 0x0 - 0x77

# cpuid -1 -l 0xb -s 1
CPU:
      --- level 1 (core) ---
      bits to shift APIC ID to get next = 0x5 (5)
      logical processors at this level  = 0x18 (24)
      level number                      = 0x1 (1)
      level type                        = core (2)
      extended APIC ID                  = 0

If we still enumerates all the x2APIC entries,
1. we got 72 extra possible CPUs from x2APIC
2. with the patch at https://lore.kernel.org/all/87edm36qqb.ffs@tglx/ ,
_max_logical_packages is set to 4 instead of 2.

this is still a problem, right?

thanks,
rui