Received-SPF: pass (google.com: domain of linux-kernel+bounces-180691-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 16 May 2024 09:17:59 +0200
From: Borislav Petkov <bp@alien8.de>
To: "Luck, Tony" <tony.luck@intel.com>
Cc: "x86@kernel.org" <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"patches@lists.linux.dev" <patches@lists.linux.dev>
Subject: Re: [PATCH] x86/cpu: Fix x86_match_cpu() to match just
 X86_VENDOR_INTEL
Message-ID: <20240516071759.GAZkWzJ7t83HW6BECA@fat_crate.local>
References: <20240515214357.73798-1-tony.luck@intel.com>
 <20240515220058.GEZkUwmtNazjQaLdDn@fat_crate.local>
 <SJ1PR11MB6083D455CE90AFC3251703B8FCEC2@SJ1PR11MB6083.namprd11.prod.outlook.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <SJ1PR11MB6083D455CE90AFC3251703B8FCEC2@SJ1PR11MB6083.namprd11.prod.outlook.com>

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