linux-kernel - Re: [PATCH 2/2] x86/cpu: clear SME/SEV/SEV

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: <d705b64d-2866-e2bf-2323-dfe0cfe73da8@amd.com>
Date:   Fri, 11 Feb 2022 13:54:01 -0600
From:   "Limonciello, Mario" <mario.limonciello@....com>
To:     Tom Lendacky <thomas.lendacky@....com>,
        Borislav Petkov <bp@...en8.de>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>
Cc:     Kees Cook <keescook@...omium.org>, hughsient@...il.com,
        Martin Fernandez <martin.fernandez@...ypsium.com>,
        linux-kernel@...r.kernel.org, Brijesh Singh <brijesh.singh@....com>
Subject: Re: [PATCH 2/2] x86/cpu: clear SME/SEV/SEV_ES features when not in
 use

On 2/11/2022 08:46, Tom Lendacky wrote:
> +Brijesh (please copy him, too, on all SEV related things)
> 
> On 2/10/22 23:36, Mario Limonciello wrote:
>> Currently SME/SEV/SEV_ES features are reflective of whether the CPU
>> supports the features but not whether they have been activated by the
>> kernel.
>>
>> Change this around to clear the features if the kernel is not using
>> them so userspace can determine if they are available and in use
>> from `/proc/cpuinfo`.
>>
>> Signed-off-by: Mario Limonciello <mario.limonciello@....com>
>> ---
>>   arch/x86/kernel/cpu/amd.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
>> index 4edb6f0f628c..4a7d4801fa0b 100644
>> --- a/arch/x86/kernel/cpu/amd.c
>> +++ b/arch/x86/kernel/cpu/amd.c
>> @@ -611,12 +611,20 @@ static void early_detect_mem_encrypt(struct 
>> cpuinfo_x86 *c)
>>           if (!(msr & MSR_K7_HWCR_SMMLOCK))
>>               goto clear_sev;
>> +        if (!cc_platform_has(CC_ATTR_MEM_ENCRYPT))
>> +            goto clear_all;
>> +        if (!cc_platform_has(CC_ATTR_GUEST_MEM_ENCRYPT))
>> +            goto clear_sev;
>> +        if (!cc_platform_has(CC_ATTR_GUEST_STATE_ENCRYPT))
>> +            goto clear_sev_es;
>> +
> 
> A host/hypervisor will never see GUEST related attributes return true, 
> you need to verify all uses of X86_FEATURE_SEV* in the kernel. I see two 
> files where this change will break SEV hypervisor support:
> 

Maybe it's best then to just check for/clear the SME feature at this 
time and let Brijesh handle plumbing the applicable removals/additions 
for SEV/SEV_ES separately.

> - arch/x86/kvm/svm/sev.c / sev_hardware_setup()
> - drivers/crypto/ccp/sev-dev.c / sev_dev_init()
> 
> I imagine that some of this has to be re-thought a bit. The current SEV 
> and SEV-ES feature attributes are intended for hypervsior side usage.  
> For example, MSR MSR_K7_HWCR_SMMLOCK, which is likely never going to be 
> provided to a guest, needs to be checked to be certain that an SEV guest 
> can be launched, even though SEV is advertised in CPUID.
> 
> Once in the guest, it is the cc_platform_has() support that determines 
> feature support and not X86_FEATURE_SEV*
> 
> Thanks,
> Tom
> 
>>           return;
>>   clear_all:
>>           setup_clear_cpu_cap(X86_FEATURE_SME);
>>   clear_sev:
>>           setup_clear_cpu_cap(X86_FEATURE_SEV);
>> +clear_sev_es:
>>           setup_clear_cpu_cap(X86_FEATURE_SEV_ES);
>>       }
>>   }