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: <DD80374YZEWM.1L31D2VUKK80C@google.com>
Date: Thu, 02 Oct 2025 17:08:59 +0000
From: Brendan Jackman <jackmanb@...gle.com>
To: Dave Hansen <dave.hansen@...el.com>, 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 Thu Oct 2, 2025 at 4:40 PM UTC, Dave Hansen wrote:
> On 10/2/25 07:31, Brendan Jackman wrote:
> 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.

OK, then it probably just sounds wrong to me because I'm steeped in the
current jargon. For v2 I'll try just dropping "[un]restricted".

>>>>  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.

Sounds good to me. If "The Direct Map" is a gonna be a special thing
then having it be a flag instead of a certain magic pgd_t * makes sense
to me. I'll try this out.