linux-kernel - Re: [PATCH v2 3/3] x86/sev: Add SEV-SNP CipherTextHiding support

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: <CAMkAt6qP+kuzsXYtnE4MRDUVx4sVpFoa+YwBtBRArMcnAfadkw@mail.gmail.com>
Date: Thu, 3 Oct 2024 08:04:44 -0600
From: Peter Gonda <pgonda@...gle.com>
To: "Kalra, Ashish" <ashish.kalra@....com>
Cc: seanjc@...gle.com, pbonzini@...hat.com, tglx@...utronix.de, 
	mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com, hpa@...or.com, 
	herbert@...dor.apana.org.au, x86@...nel.org, john.allen@....com, 
	davem@...emloft.net, thomas.lendacky@....com, michael.roth@....com, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-crypto@...r.kernel.org
Subject: Re: [PATCH v2 3/3] x86/sev: Add SEV-SNP CipherTextHiding support

> >> +static int max_snp_asid;
> >> +module_param(max_snp_asid, int, 0444);
> >> +MODULE_PARM_DESC(max_snp_asid, "  override MAX_SNP_ASID for Cipher Text Hiding");
> > My read of the spec is if Ciphertext hiding is not enabled there is no
> > additional split in the ASID space. Am I understanding that correctly?
> Yes that is correct.
> > If so, I don't think we want to enable ciphertext hiding by default
> > because it might break whatever management of ASIDs systems already
> > have. For instance right now we have to split SEV-ES and SEV ASIDS,
> > and SNP guests need SEV-ES ASIDS. This change would half the # of SNP
> > enable ASIDs on a system.
>
> My thought here is that we probably want to enable Ciphertext hiding by default as that should fix any security issues and concerns around SNP encryption as .Ciphertext hiding prevents host accesses from reading the ciphertext of SNP guest private memory.
>
> This patch does add a new CCP module parameter, max_snp_asid, which can be used to dedicate all SEV-ES ASIDs to SNP guests.
>
> >
> > Also should we move the ASID splitting code to be all in one place?
> > Right now KVM handles it in sev_hardware_setup().
>
> Yes, but there is going to be a separate set of patches to move all ASID handling code to CCP module.
>
> This refactoring won't be part of the SNP ciphertext hiding support patches.

Makes sense. I see Tom has asked you to split this patch into ccp and KVM.

Maybe add a line to the description so more are aware of the impending
changes to asids?

I tested these patches a bit with the selftests / manually by
backporting to 6.11-rc7. When you send a V3 I'll redo for a tag. BTW
for some reason 6.12-rc1 and kvm/queue both fail to init SNP for me,
then the kernel segfaults. Not sure whats going on there...