Re: [PATCH v1] kasan: Fix false-positive wild-memory-access on x86 under 5-level paging

[Ihor Solodrai]

On 6/23/26 7:57 PM, Borislav Petkov wrote:
> On Mon, Jun 22, 2026 at 05:29:12PM -0700, Ihor Solodrai wrote:
>> On 6/18/26 11:38 AM, Borislav Petkov wrote:
>>> On Thu, Jun 18, 2026 at 11:12:09AM -0700, Dave Hansen wrote:
>>>> On 6/18/26 10:09, Borislav Petkov wrote:
>>>>> On Wed, Jun 17, 2026 at 03:13:33PM -0700, Ihor Solodrai wrote:
>>>>>> So my question to maintainers is what approach seems best?
>>>>> The CPUID stuff is being rewritten currently and it should address your issue
>>>>> too. If not, then we need to rewrite it better.
>>>>>
>>>>> Can you reproduce with this set applied ontop:
>>>>>
>>>>> https://lore.kernel.org/r/20260528153923.403473-1-darwi@xxxxxxxxxxxxx
>>>>
>>>> Thinking about this a bit more... If Ahmed's series does fix this, I
>>>> think it will be accidental. It still uses identify_cpu() and also does
>>>> a memset() of the new c->cpuid structure in addition to the old
>>>> c->x86_capability structure.
>>>>
>>>> I'm not knocking Ahmed's series by any means. It just probably won't fix
>>>> this issue.
>>>>
>>>> In a perfect world early_identify_cpu() and identify_cpu() would either
>>>> get consolidated into one thing. Or at least become two discrete things
>>>> that initialize two completely disjoint sets of data. That way,
>>>> identify_cpu() wouldn't memset() anything.
>>>>
>>>> Isn't that the _real_ fix? Instead of trying to hide the inconsistency
>>>> when good data is blown away, we stop blowing it away in the first place?
>>>
>>> early_ is only on the BSP and you want to scan all CPUs.
>>>
>>> AFAIR, the last time I was looking at how we scan the CPUID leafs, we do have
>>> cases where there's blips in time when cap bits get disappeared to be
>>> rescanned again. The toggling of MSR bits which control feature flags is one
>>> thing that comes to mind.
>>>
>>> But I'm with you on the consolidation approach. I think this is what we should
>>
>> Hello Dave, Boris. Thank you for the input.
>>
>> I tried a slight refactoring of the identify_cpu() machinery to
>> eliminate the memset(x86_capability) from the boot cpu, and it fixes
>> the splat that I reported.
>>
>> identify_cpu() is split into two parts:
>>   * identify_cpu_scan() - the top part, up to and including the
>>     generic_identify() call
>>   * identify_cpu() proper with the rest, no memset here
>>
>> Then (gated on x86_64) identify_boot_cpu() *skips* the
>> identify_cpu_scan() call, only executing the bottom part of current
>> identify_cpu(). We can do that because boot cpu already did the _scan
>> part in early_identify_cpu(), when interrupts are still disabled.
>>
>> One caveat: get_model_name() is moved from generic_identify() into
>> identify_cpu(), otherwise it wouldn't be called on boot cpu. Same for
>> c->loops_per_jiffy = loops_per_jiffy; - it's moved to the "bottom"
>> identify_cpu().
>>
>> The behavior for secondary cpus is unchanged: identify_secondary_cpu()
>> calls identify_cpu_scan() then immediately identify_cpu().
>>
>> Please take a look at the diff below and let me know if this is a good
>> way to proceed. I don't know exactly what I'm doing, so concerns and
>> corrections are welcome.
>>
>>
>> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
>> index a4268c47f2bc..4a655109cacf 100644
>> --- a/arch/x86/kernel/cpu/common.c
>> +++ b/arch/x86/kernel/cpu/common.c
>> @@ -1978,8 +1978,6 @@ static void generic_identify(struct cpuinfo_x86 *c)
>>  
>>  	get_cpu_address_sizes(c);
>>  
>> -	get_model_name(c); /* Default name */
>> -
>>  	/*
>>  	 * ESPFIX is a strange bug.  All real CPUs have it.  Paravirt
>>  	 * systems that run Linux at CPL > 0 may or may not have the
>> @@ -1999,13 +1997,12 @@ static void generic_identify(struct cpuinfo_x86 *c)
>>  }
>>  
>>  /*
>> - * This does the hard work of actually picking apart the CPU stuff...
>> + * Rebuild capability set from CPUID and re-apply the forced-cap overrides
>> + * through generic_identify(). This *should* run with interrupts off, otherwise
>> + * an interrupt handler might see the capability bits cleared.
> 
> And yet it doesn't run with IRQs off. So that comment doesn't need to be here.
> The point of the whole exercise is to not disable ugly interrupts in that path
> at all.
> 
>>   */
>> -static void identify_cpu(struct cpuinfo_x86 *c)
>> +static void identify_cpu_scan(struct cpuinfo_x86 *c)
>>  {
>> -	int i;
>> -
>> -	c->loops_per_jiffy = loops_per_jiffy;
>>  	c->x86_cache_size = 0;
>>  	c->x86_vendor = X86_VENDOR_UNKNOWN;
>>  	c->x86_model = c->x86_stepping = 0;	/* So far unknown... */
>> @@ -2028,6 +2025,19 @@ static void identify_cpu(struct cpuinfo_x86 *c)
>>  #endif
>>  
>>  	generic_identify(c);
>> +}
>> +
>> +/*
>> + * This is safe to run with interrupts on.
>> + * The boot CPU can run just this from arch_cpu_finalize_init(), because
>> + * the scan part already happened in early_identify_cpu().
>> + */
>> +static void identify_cpu(struct cpuinfo_x86 *c)
>> +{
>> +	int i;
>> +
>> +	c->loops_per_jiffy = loops_per_jiffy;
>> +	get_model_name(c); /* Default name */
>>  
>>  	cpu_parse_topology(c);
>>  
>> @@ -2154,6 +2164,17 @@ void enable_sep_cpu(void)
>>  
>>  static __init void identify_boot_cpu(void)
>>  {
>> +	/*
>> +	 * The boot CPU's capabilities were already scanned by early_identify_cpu().
>> +	 * Here identify_cpu() only finalizes them.
>> +	 *
>> +	 * 32-bit still needs the full scan here: it sets X86_BUG_ESPFIX (via
>> +	 * generic_identify()) and the no-CPUID cpuid_level default, which the early
>> +	 * path does not.
>> +	 */
>> +#ifndef CONFIG_X86_64
>> +	identify_cpu_scan(&boot_cpu_data);
>> +#endif
>>  	identify_cpu(&boot_cpu_data);
>>  	if (HAS_KERNEL_IBT && cpu_feature_enabled(X86_FEATURE_IBT))
>>  		pr_info("CET detected: Indirect Branch Tracking enabled\n");
>> @@ -2178,6 +2199,7 @@ void identify_secondary_cpu(unsigned int cpu)
>>  		*c = boot_cpu_data;
>>  	c->cpu_index = cpu;
>>  
>> +	identify_cpu_scan(c);
>>  	identify_cpu(c);
>>  #ifdef CONFIG_X86_32
>>  	enable_sep_cpu();
>> -- 
> 
> Ok, thanks for that, that's a good first try. I did poke at it a bit and
> here's what I think should happen:
> 
> 1. The part which initializes struct cpuinfo_x86 up and including the memsets
>    should be one function called init_cpu_info() or so.
> 
> 2. Then, generic_identify() should be merged into identify_cpu() basically
>    turning the latter into a single function which does a generic CPU
>    identification both on the BSP and on the APs.
> 
> 3. #ifdef CONFIG_X86_32
>         enable_sep_cpu();
> #endif 
> 	that gunk should be stuck into a function called identify_cpu_32() and
> 	you'll have at the beginning of it
> 
> 	if (!IS_ENABLED(CONFIG_X86_32))
> 		return;
> 
>    and then you call that function both in identify_boot_cpu() and
>    identify_secondary_cpu(). This way you streamline the paths and drop all
>    that ugly ifdeffery.
> 
> 4. When you do 3. above, you should be able to move this gunk
> 
> 	#ifdef CONFIG_X86_32
>         set_cpu_bug(c, X86_BUG_ESPFIX);
> 	#endif
> 
> to it too. And now everything is nicely encapsulated and straight-forward.
> 
> 5. The above will allow you to have the init_cpu_info() only on 32-bit.
> 
> Oh and, looking at the ifdeffery on identify_cpu():
> 
> 	#ifdef CONFIG_X86_64
> 	        c->x86_clflush_size = 64;
> 	        c->x86_phys_bits = 36;
> 	        c->x86_virt_bits = 48;
> 	#else
> 
> you could probably add a identify_cpu_64() too. Or maybe the init parts should
> be called separately init_cpu_info_32() and init_cpu_info_64(). You probably
> need to see how it looks like when you write it.
> 
> Oh, and each 1., 2., ...step above is a single patch. This'll make the review
> very easy too.
> 
> How does that sound? Willing to give it a try? :-)

The instructions are quite detailed, thank you.

I'll try it and will send a separate series as soon as I can (likely next week).

> 
> If not, I could try to find some time and do it myself but I thought you might
> be willing to try it...
> 
> :-)
> 
> Thx.
>