| 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
| ||
|
Message-ID: <ba6dc90a-64a7-4124-a9b9-c600f3bdbd19@intel.com>
Date: Fri, 1 Dec 2023 15:01:38 +0800
From: "Yang, Weijiang" <weijiang.yang@...el.com>
To: Maxim Levitsky <mlevitsk@...hat.com>
CC: <linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>,
<dave.hansen@...el.com>, <pbonzini@...hat.com>,
<seanjc@...gle.com>, <peterz@...radead.org>, <chao.gao@...el.com>,
<rick.p.edgecombe@...el.com>, <john.allen@....com>
Subject: Re: [PATCH v7 03/26] x86/fpu/xstate: Add CET supervisor mode state
support
On 12/1/2023 1:27 AM, Maxim Levitsky wrote:
> On Fri, 2023-11-24 at 00:53 -0500, Yang Weijiang wrote:
>> Add supervisor mode state support within FPU xstate management framework.
>> Although supervisor shadow stack is not enabled/used today in kernel,KVM
>> requires the support because when KVM advertises shadow stack feature to
>> guest, architecturally it claims the support for both user and supervisor
>> modes for guest OSes(Linux or non-Linux).
>>
>> CET supervisor states not only includes PL{0,1,2}_SSP but also IA32_S_CET
>> MSR, but the latter is not xsave-managed. In virtualization world, guest
>> IA32_S_CET is saved/stored into/from VM control structure. With supervisor
>> xstate support, guest supervisor mode shadow stack state can be properly
>> saved/restored when 1) guest/host FPU context is swapped 2) vCPU
>> thread is sched out/in.
>>
>> The alternative is to enable it in KVM domain, but KVM maintainers NAKed
>> the solution. The external discussion can be found at [*], it ended up
>> with adding the support in kernel instead of KVM domain.
>>
>> Note, in KVM case, guest CET supervisor state i.e., IA32_PL{0,1,2}_MSRs,
>> are preserved after VM-Exit until host/guest fpstates are swapped, but
>> since host supervisor shadow stack is disabled, the preserved MSRs won't
>> hurt host.
>>
>> [*]: https://lore.kernel.org/all/806e26c2-8d21-9cc9-a0b7-7787dd231729@intel.com/
>>
>> Signed-off-by: Yang Weijiang <weijiang.yang@...el.com>
>> ---
>> arch/x86/include/asm/fpu/types.h | 14 ++++++++++++--
>> arch/x86/include/asm/fpu/xstate.h | 6 +++---
>> arch/x86/kernel/fpu/xstate.c | 6 +++++-
>> 3 files changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
>> index eb810074f1e7..c6fd13a17205 100644
>> --- a/arch/x86/include/asm/fpu/types.h
>> +++ b/arch/x86/include/asm/fpu/types.h
>> @@ -116,7 +116,7 @@ enum xfeature {
>> XFEATURE_PKRU,
>> XFEATURE_PASID,
>> XFEATURE_CET_USER,
>> - XFEATURE_CET_KERNEL_UNUSED,
>> + XFEATURE_CET_KERNEL,
>> XFEATURE_RSRVD_COMP_13,
>> XFEATURE_RSRVD_COMP_14,
>> XFEATURE_LBR,
>> @@ -139,7 +139,7 @@ enum xfeature {
>> #define XFEATURE_MASK_PKRU (1 << XFEATURE_PKRU)
>> #define XFEATURE_MASK_PASID (1 << XFEATURE_PASID)
>> #define XFEATURE_MASK_CET_USER (1 << XFEATURE_CET_USER)
>> -#define XFEATURE_MASK_CET_KERNEL (1 << XFEATURE_CET_KERNEL_UNUSED)
>> +#define XFEATURE_MASK_CET_KERNEL (1 << XFEATURE_CET_KERNEL)
>> #define XFEATURE_MASK_LBR (1 << XFEATURE_LBR)
>> #define XFEATURE_MASK_XTILE_CFG (1 << XFEATURE_XTILE_CFG)
>> #define XFEATURE_MASK_XTILE_DATA (1 << XFEATURE_XTILE_DATA)
>> @@ -264,6 +264,16 @@ struct cet_user_state {
>> u64 user_ssp;
>> };
>>
>> +/*
>> + * State component 12 is Control-flow Enforcement supervisor states
>> + */
>> +struct cet_supervisor_state {
>> + /* supervisor ssp pointers */
>> + u64 pl0_ssp;
>> + u64 pl1_ssp;
>> + u64 pl2_ssp;
>> +};
>> +
>> /*
>> * State component 15: Architectural LBR configuration state.
>> * The size of Arch LBR state depends on the number of LBRs (lbr_depth).
>> diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
>> index d4427b88ee12..3b4a038d3c57 100644
>> --- a/arch/x86/include/asm/fpu/xstate.h
>> +++ b/arch/x86/include/asm/fpu/xstate.h
>> @@ -51,7 +51,8 @@
>>
>> /* All currently supported supervisor features */
>> #define XFEATURE_MASK_SUPERVISOR_SUPPORTED (XFEATURE_MASK_PASID | \
>> - XFEATURE_MASK_CET_USER)
>> + XFEATURE_MASK_CET_USER | \
>> + XFEATURE_MASK_CET_KERNEL)
>>
>> /*
>> * A supervisor state component may not always contain valuable information,
>> @@ -78,8 +79,7 @@
>> * Unsupported supervisor features. When a supervisor feature in this mask is
>> * supported in the future, move it to the supported supervisor feature mask.
>> */
>> -#define XFEATURE_MASK_SUPERVISOR_UNSUPPORTED (XFEATURE_MASK_PT | \
>> - XFEATURE_MASK_CET_KERNEL)
>> +#define XFEATURE_MASK_SUPERVISOR_UNSUPPORTED (XFEATURE_MASK_PT)
>>
>> /* All supervisor states including supported and unsupported states. */
>> #define XFEATURE_MASK_SUPERVISOR_ALL (XFEATURE_MASK_SUPERVISOR_SUPPORTED | \
>> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
>> index 6e50a4251e2b..b57d909facca 100644
>> --- a/arch/x86/kernel/fpu/xstate.c
>> +++ b/arch/x86/kernel/fpu/xstate.c
>> @@ -51,7 +51,7 @@ static const char *xfeature_names[] =
>> "Protection Keys User registers",
>> "PASID state",
>> "Control-flow User registers",
>> - "Control-flow Kernel registers (unused)",
>> + "Control-flow Kernel registers",
>> "unknown xstate feature",
>> "unknown xstate feature",
>> "unknown xstate feature",
>> @@ -73,6 +73,7 @@ static unsigned short xsave_cpuid_features[] __initdata = {
>> [XFEATURE_PT_UNIMPLEMENTED_SO_FAR] = X86_FEATURE_INTEL_PT,
>> [XFEATURE_PKRU] = X86_FEATURE_OSPKE,
>> [XFEATURE_PASID] = X86_FEATURE_ENQCMD,
>> + [XFEATURE_CET_KERNEL] = X86_FEATURE_SHSTK,
>> [XFEATURE_XTILE_CFG] = X86_FEATURE_AMX_TILE,
>> [XFEATURE_XTILE_DATA] = X86_FEATURE_AMX_TILE,
>> };
>> @@ -277,6 +278,7 @@ static void __init print_xstate_features(void)
>> print_xstate_feature(XFEATURE_MASK_PKRU);
>> print_xstate_feature(XFEATURE_MASK_PASID);
>> print_xstate_feature(XFEATURE_MASK_CET_USER);
>> + print_xstate_feature(XFEATURE_MASK_CET_KERNEL);
>> print_xstate_feature(XFEATURE_MASK_XTILE_CFG);
>> print_xstate_feature(XFEATURE_MASK_XTILE_DATA);
>> }
>> @@ -346,6 +348,7 @@ static __init void os_xrstor_booting(struct xregs_state *xstate)
>> XFEATURE_MASK_BNDCSR | \
>> XFEATURE_MASK_PASID | \
>> XFEATURE_MASK_CET_USER | \
>> + XFEATURE_MASK_CET_KERNEL | \
>> XFEATURE_MASK_XTILE)
>>
>> /*
>> @@ -546,6 +549,7 @@ static bool __init check_xstate_against_struct(int nr)
>> case XFEATURE_PASID: return XCHECK_SZ(sz, nr, struct ia32_pasid_state);
>> case XFEATURE_XTILE_CFG: return XCHECK_SZ(sz, nr, struct xtile_cfg);
>> case XFEATURE_CET_USER: return XCHECK_SZ(sz, nr, struct cet_user_state);
>> + case XFEATURE_CET_KERNEL: return XCHECK_SZ(sz, nr, struct cet_supervisor_state);
>> case XFEATURE_XTILE_DATA: check_xtile_data_against_struct(sz); return true;
>> default:
>> XSTATE_WARN_ON(1, "No structure for xstate: %d\n", nr);
> Any reason why my reviewed-by was not added to this patch?
My apology again! I missed the Reviewed-by tag for this patch!
Appreciated for your careful review for this series!
> Reviewed-by: Maxim Levitsky <mlevitsk@...hat.com>
>
> Best regards,
> Maxim Levitsky
>
>
>
Powered by blists - more mailing lists