linux-kernel - Re: [PATCH v1 24/26] crypto: ccp: Add the SNP_PLATFORM

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]

Date: Thu, 25 Jan 2024 21:32:22 -0600
From: Michael Roth <michael.roth@....com>
To: Borislav Petkov <bp@...en8.de>
CC: <x86@...nel.org>, <kvm@...r.kernel.org>, <linux-coco@...ts.linux.dev>,
	<linux-mm@...ck.org>, <linux-crypto@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <tglx@...utronix.de>, <mingo@...hat.com>,
	<jroedel@...e.de>, <thomas.lendacky@....com>, <hpa@...or.com>,
	<ardb@...nel.org>, <pbonzini@...hat.com>, <seanjc@...gle.com>,
	<vkuznets@...hat.com>, <jmattson@...gle.com>, <luto@...nel.org>,
	<dave.hansen@...ux.intel.com>, <slp@...hat.com>, <pgonda@...gle.com>,
	<peterz@...radead.org>, <srinivas.pandruvada@...ux.intel.com>,
	<rientjes@...gle.com>, <tobin@....com>, <vbabka@...e.cz>,
	<kirill@...temov.name>, <ak@...ux.intel.com>, <tony.luck@...el.com>,
	<sathyanarayanan.kuppuswamy@...ux.intel.com>, <alpergun@...gle.com>,
	<jarkko@...nel.org>, <ashish.kalra@....com>, <nikunj.dadhania@....com>,
	<pankaj.gupta@....com>, "liam.merwick@...cle.com Brijesh Singh"
	<brijesh.singh@....com>
Subject: Re: [PATCH v1 24/26] crypto: ccp: Add the SNP_PLATFORM_STATUS command

On Sun, Jan 21, 2024 at 01:29:20PM +0100, Borislav Petkov wrote:
> On Sat, Dec 30, 2023 at 10:19:52AM -0600, Michael Roth wrote:
> > +	/* Change the page state before accessing it */
> > +	if (snp_reclaim_pages(__pa(data), 1, true)) {
> > +		snp_leak_pages(__pa(data) >> PAGE_SHIFT, 1);
> > +		return -EFAULT;
> > +	}
> 
> This looks weird and it doesn't explain why this needs to happen.
> SNP_PLATFORM_STATUS text doesn't explain either.
> 
> So, what's up?

I've adding some clarifying comment in v2, but the page that firmware
writes needs to first be switched to the Firmware-owned state, and
after successful completion it will be put in Reclaim state. But it's
possible a failure might occur before that transition is made by
firmware, maybe the command fails somewhere in the callstack before it
even reaches firmware.

If that happens the page might still be in firmware-owned state, and
need to go through snp_reclaim_pages()/SNP_PAGE_RECLAIM before it can
be switched back to Default state.

Rather than trying to special-case all these possibilities, it's simpler
to just always use snp_reclaim_pages(), which will handle both Reclaim
and Firmware-owned pages.

However, snp_reclaim_pages() will already leak the page when necessary,
so I've dropped that bit.

-Mike

> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette