linux-kernel - RE: [PATCH Part2 v6 06/49] x86/sev: Add helper functions for RMPUPDATE and PSMASH instruction

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <SN6PR12MB27675C337244C0A6848167AA8E9D9@SN6PR12MB2767.namprd12.prod.outlook.com>
Date:   Tue, 2 Aug 2022 04:49:51 +0000
From:   "Kalra, Ashish" <Ashish.Kalra@....com>
To:     Dov Murik <dovmurik@...ux.ibm.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>
CC:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "jroedel@...e.de" <jroedel@...e.de>,
        "Lendacky, Thomas" <Thomas.Lendacky@....com>,
        "hpa@...or.com" <hpa@...or.com>,
        "ardb@...nel.org" <ardb@...nel.org>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "seanjc@...gle.com" <seanjc@...gle.com>,
        "vkuznets@...hat.com" <vkuznets@...hat.com>,
        "jmattson@...gle.com" <jmattson@...gle.com>,
        "luto@...nel.org" <luto@...nel.org>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "slp@...hat.com" <slp@...hat.com>,
        "pgonda@...gle.com" <pgonda@...gle.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "srinivas.pandruvada@...ux.intel.com" 
        <srinivas.pandruvada@...ux.intel.com>,
        "rientjes@...gle.com" <rientjes@...gle.com>,
        "tobin@....com" <tobin@....com>, "bp@...en8.de" <bp@...en8.de>,
        "Roth, Michael" <Michael.Roth@....com>,
        "vbabka@...e.cz" <vbabka@...e.cz>,
        "kirill@...temov.name" <kirill@...temov.name>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>,
        "tony.luck@...el.com" <tony.luck@...el.com>,
        "marcorr@...gle.com" <marcorr@...gle.com>,
        "sathyanarayanan.kuppuswamy@...ux.intel.com" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        "alpergun@...gle.com" <alpergun@...gle.com>,
        "dgilbert@...hat.com" <dgilbert@...hat.com>,
        "jarkko@...nel.org" <jarkko@...nel.org>
Subject: RE: [PATCH Part2 v6 06/49] x86/sev: Add helper functions for
 RMPUPDATE and PSMASH instruction

[AMD Official Use Only - General]

Hello Dov,

>> +int rmp_make_private(u64 pfn, u64 gpa, enum pg_level level, int asid, 
>> +bool immutable) {
>> +	struct rmpupdate val;
>> +
>> +	if (!pfn_valid(pfn))
>> +		return -EINVAL;
>> +

>Should we add more checks on the arguments?

>1. asid must be > 0
>2. gpa must be aligned according to 'level'
>3. gpa must be below the maximal address for the guest

Ok, yes it surely makes sense to add more checks on the arguments.

>"Note that the guest physical address space is limited according to CPUID Fn80000008_EAX and thus the GPAs used by the firmware in measurement calculation are equally limited. Hypervisors should not attempt to map pages outside of this limit."
>(-SNP ABI spec page 86, section 8.17 SNP_LAUNCH_UPDATE)


>But note that in patch 28 of this series we have:

>+		/* Transition the VMSA page to a firmware state. */
>+		ret = rmp_make_private(pfn, -1, PG_LEVEL_4K, sev->asid, true);

>That (u64)(-1) value for the gpa argument violates conditions 2 and 3 from my list above.

>And indeed when calculating measurements we see that the GPA value for the VMSA pages is 0x0000FFFF_FFFFF000, and not (u64)(-1). [1] [2]

>Instead of checks, we can mask the gpa argument so that rmpupdate will get the correct value.  Not sure which approach is preferable.

Well, the firmware is anyway masking the gpa argument as you observe in the launch digest, so probably we should do the same here too.

Thanks,
Ashish