linux-kernel - Re: [PATCH v2] x86/cpuinfo: Clear X86_FEATURE

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: <CAKgze5azQG1mnOASbpcrs9jTejdXGkXYmezz9bTKuWQoZp5EFg@mail.gmail.com>
Date:   Tue, 5 Jul 2022 10:21:47 -0300
From:   Martin Fernandez <martin.fernandez@...ypsium.com>
To:     Kai Huang <kai.huang@...el.com>
Cc:     linux-kernel@...r.kernel.org, bp@...en8.de,
        dave.hansen@...ux.intel.com, x86@...nel.org, mingo@...hat.com,
        tglx@...utronix.de, kirill.shutemov@...ux.intel.com,
        daniel.gutson@...ypsium.com, hughsient@...il.com,
        alex.bazhaniuk@...ypsium.com
Subject: Re: [PATCH v2] x86/cpuinfo: Clear X86_FEATURE_TME if TME/MKTME is
 disabled by BIOS

On 7/5/22, Kai Huang <kai.huang@...el.com> wrote:
> On Mon, 2022-07-04 at 11:22 -0300, Martin Fernandez wrote:
>> Right now the only way to check this is by greping the kernel logs,
>> which is inconvenient. This is currently checked for fwupd for
>> example.
>>
>> I understand that cpuinfo is supposed to report every feature in the
>> cpu but since AMD is doing the same (and it also broke backwards
>> compatibility [1]) for sme/sev I think it's good to have this for
>> Intel too.
>>
>> Another option to prevent greping the logs would be a file in
>> sysfs. I'm open to suggestions to where to place this infomartion. I
>> saw a proposal about a firmware security filesystem [2]; that would
>> fit perfectly.
>>
>> [1]
>> https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/patch/?id=08f253ec3767bcfafc5d32617a92cee57c63968e
>>
>> [2]
>> https://lore.kernel.org/all/20220622215648.96723-3-nayna@linux.ibm.com/
>
> Leave above to others, but...
>>
>> Changelog since v1
>>
>> Clear the flag not only for BSP but for every cpu in the system.
>
> ... the changelog history shouldn't be in the commit message.
>
> You can put one additional '---' after your 'Signed-off-by' and add your
> changelog after it.  The content between two '---'s will be stripped away
> by
> 'git am' when maintainer takes the patch:
>
> 	Signed-off-by: Martin Fernandez <martin.fernandez@...ypsium.com>
> 	---
> 	v1->v2:
> 		xxx
> 	---
> 	arch/x86/kernel/cpu/intel.c | 1 +
> 	1 file changed, 1 insertion(+)'

Thanks!, didn't know about it, makes sense.

>>
>> Signed-off-by: Martin Fernandez <martin.fernandez@...ypsium.com>
>> ---
>>  arch/x86/kernel/cpu/intel.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
>> index fd5dead8371c..17f23e23f911 100644
>> --- a/arch/x86/kernel/cpu/intel.c
>> +++ b/arch/x86/kernel/cpu/intel.c
>> @@ -570,6 +570,7 @@ static void detect_tme(struct cpuinfo_x86 *c)
>>
>>  	if (!TME_ACTIVATE_LOCKED(tme_activate) ||
>> !TME_ACTIVATE_ENABLED(tme_activate)) {
>>  		pr_info_once("x86/tme: not enabled by BIOS\n");
>> +		clear_cpu_cap(c, X86_FEATURE_TME);
>>  		mktme_status = MKTME_DISABLED;
>>  		return;
>
> This code change itself looks good to me.
>
> But, TME actually supports bypassing TME encryption/decryption by setting 1
> to
> bit 31 to IA32_TME_ACTIVATE MSR.  See 'Table 4-2 IA32_TME_ACTIVATE MSR' in
> MKTME
> spec below:
>
> https://edc.intel.com/content/www/us/en/design/ipla/software-development-platforms/client/platforms/alder-lake-desktop/12th-generation-intel-core-processors-datasheet-volume-1-of-2/002/intel-multi-key-total-memory-encryption/
>
> When bit 31 is set, the TME is bypassed (no encryption/decryption for KeyID
> 0).
>
> So looks userspace also needs to check this if it wants to truly check
> whether
> "TME memory encryption" is active.
>
> But perhaps it's arguable whether we can also clear TME flag in this case.

Yep, that's what I thought.

> So:
>
> Acked-by: Kai Huang <kai.huang@...el.com>
>
>
> --
> Thanks,
> -Kai
>
>
>