linux-kernel - Re: [PATCH 05/21] x86/mm/pat: mirror direct map changes to ASI

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]

Message-ID: <19e5012a-3c58-4696-9e4e-39e2b7d2b5af@intel.com>
Date: Thu, 2 Oct 2025 09:40:27 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: Brendan Jackman <jackmanb@...gle.com>, Andy Lutomirski <luto@...nel.org>,
 Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 "Liam R. Howlett" <Liam.Howlett@...cle.com>,
 Suren Baghdasaryan <surenb@...gle.com>, Michal Hocko <mhocko@...e.com>,
 Johannes Weiner <hannes@...xchg.org>, Zi Yan <ziy@...dia.com>,
 Axel Rasmussen <axelrasmussen@...gle.com>, Yuanchu Xie <yuanchu@...gle.com>,
 Roman Gushchin <roman.gushchin@...ux.dev>
Cc: peterz@...radead.org, bp@...en8.de, dave.hansen@...ux.intel.com,
 mingo@...hat.com, tglx@...utronix.de, akpm@...ux-foundation.org,
 david@...hat.com, derkling@...gle.com, junaids@...gle.com,
 linux-kernel@...r.kernel.org, linux-mm@...ck.org, reijiw@...gle.com,
 rientjes@...gle.com, rppt@...nel.org, vbabka@...e.cz, x86@...nel.org,
 Yosry Ahmed <yosry.ahmed@...ux.dev>
Subject: Re: [PATCH 05/21] x86/mm/pat: mirror direct map changes to ASI

On 10/2/25 07:31, Brendan Jackman wrote:
> On Wed Oct 1, 2025 at 8:50 PM UTC, Dave Hansen wrote:
...
>> But let's say someone is doing something silly like:
>>
>> 	set_memory_np(addr, size);
>> 	set_memory_p(addr, size);
>>
>> Won't that end up in here and make the "unrestricted PGD" have
>> _PAGE_PRESENT==1 entries?
> 
> Er, yes, that's a bug, thanks for pointing this out. I guess this is
> actually broken under debug_pagealloc or something? I should check that.
> 
> This code should only mirror the bits that are irrelevant to ASI. 

It's actually anything that has _PAGE_PRESENT in cpa->mask_set. There
are a number of those. Some of them are irrelevant like the execmem
code, but there are quite a few more that look troublesome outside of
debugging environments.

>> Also, could we try and make the nomenclature consistent? We've got
>> "unrestricted direct map" and "asi_nonsensitive_pgd" being used (at
>> least). Could the terminology be made more consistent?
> 
> Hm. It is actually consistent: "unrestricted" is a property of the
> address space / execution context. "nonsensitive" is a property of the
> memory. Nonsensitive memory is mapped into the unrestricted address
> space. asi_nonsensitive_pgd isn't an address space we enter it's just a
> holding area (like if we never actually pointed CR3 at init_mm.pgd but
> just useed it as a source to clone from).
> 
> However.. just because it's consistent doesn't mean it's not confusing.
> Do you think we should just squash these two words and call the whole
> thing "nonsensitive"? I don't know if "nonsensitive address space" makes
> much sense... Is it possible I can fix this by just adding more
> comments?

It makes sense to me that a "nonsensitive address space" would not map
any sensitive data and that a "asi_nonsensitive_pgd" is the root of that
address space.

>>>  static int __change_page_attr_set_clr(struct cpa_data *cpa, int primary)
>>>  {
>>>  	unsigned long numpages = cpa->numpages;
>>> @@ -2007,6 +2033,8 @@ static int __change_page_attr_set_clr(struct cpa_data *cpa, int primary)
>>>  		if (!debug_pagealloc_enabled())
>>>  			spin_lock(&cpa_lock);
>>>  		ret = __change_page_attr(cpa, primary);
>>> +		if (!ret)
>>> +			ret = mirror_asi_direct_map(cpa, primary);
>>>  		if (!debug_pagealloc_enabled())
>>>  			spin_unlock(&cpa_lock);
>>>  		if (ret)
>>>
>>
>> Is cpa->pgd ever have any values other than NULL or init_mm->pgd? I
>> didn't see anything in a quick grep.
> 
> It can also be efi_mm.pgd via sev_es_efi_map_ghcbs_cas().

It would be _nice_ if the ASI exclusion wasn't so magic.

Like, instead of hooking in to __change_page_attr_set_clr() and
filtering on init_mm if we had the callers declare explicitly whether
their changes get reflected into the ASI nonsensitive PGD.

Maybe that looks like a new flag: CPA_DIRECT_MAP or something. Once you
pass that flag in, the cpa code knows that you're working on init_mm.pgd
and mirror_asi_direct_map() can look for *that* instead of init_mm.