linux-kernel - Re: [PATCH] x86/cpu: Fix x86_match_cpu() to match just X86_VENDOR

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] [day] [month] [year] [list]

Message-ID: <20240516071759.GAZkWzJ7t83HW6BECA@fat_crate.local>
Date: Thu, 16 May 2024 09:17:59 +0200
From: Borislav Petkov <bp@...en8.de>
To: "Luck, Tony" <tony.luck@...el.com>
Cc: "x86@...nel.org" <x86@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"patches@...ts.linux.dev" <patches@...ts.linux.dev>
Subject: Re: [PATCH] x86/cpu: Fix x86_match_cpu() to match just
 X86_VENDOR_INTEL

On Wed, May 15, 2024 at 10:07:56PM +0000, Luck, Tony wrote:
> >> There's a wildcard match in arch/x86/kernel/smpboot.c that wants
> >> to hit on any CPU made by Intel. The match used to work because
> >
> > intel_cod_cpu[] or which one?
> >
> > Please be more specific.
> 
> intel_cod_cpu[] is the only one in arch/x86/kernel/smpboot.c

Sorry Tony, commit messages are not a guessing game but a precise
explanation as to what the problem is. Please try again.

Also, this change looks like "oh, lemme change the Intel vendor number
so that my conditional works".

But the Intel vendor has been 0 for what, 30 years?

Are you sure no other code in the tree relies on that? Audited?

Also, those x86_match_cpu functions with for-loop termination
conditionals checking whether stuff is not 0 - i.e., that:

        for (m = match;
             m->vendor | m->family | m->model | m->steppings | m->feature;
	     ^^^^^^^^^^^
             m++) {

are technically wrong when a vendor 0 is valid.

So this has all worked by chance and you've hit the one case where it
didn't.

Because it's not like we don't have a "invalid vendor" define:

#define X86_VENDOR_UNKNOWN      0xff

So, *actually*, all those terminators at the end of those arrays should
have those invalid values.

And there are a gazillion of those in the tree... :-\

Lovely.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette