| 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: <aC4ELHF73K4KIY27@google.com>
Date: Wed, 21 May 2025 09:49:48 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Chao Gao <chao.gao@...el.com>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
tglx@...utronix.de, dave.hansen@...el.com, pbonzini@...hat.com,
peterz@...radead.org, rick.p.edgecombe@...el.com, weijiang.yang@...el.com,
john.allen@....com, bp@...en8.de, chang.seok.bae@...el.com, xin3.li@...el.com,
Ingo Molnar <mingo@...hat.com>, Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>, Samuel Holland <samuel.holland@...ive.com>,
Mitchell Levy <levymitchell0@...il.com>, Kees Cook <kees@...nel.org>,
Stanislav Spassov <stanspas@...zon.de>, Eric Biggers <ebiggers@...gle.com>,
Nikolay Borisov <nik.borisov@...e.com>, Oleg Nesterov <oleg@...hat.com>,
Vignesh Balasubramanian <vigbalas@....com>
Subject: Re: [PATCH v7 1/6] x86/fpu/xstate: Differentiate default features for
host and guest FPUs
On Mon, May 12, 2025, Chao Gao wrote:
> @@ -772,6 +776,21 @@ static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
> fpstate_reset(x86_task_fpu(current));
> }
>
> +static void __init init_default_features(u64 kernel_max_features, u64 user_max_features)
> +{
> + u64 kfeatures = kernel_max_features;
> + u64 ufeatures = user_max_features;
> +
> + /* Default feature sets should not include dynamic xfeatures. */
> + kfeatures &= ~XFEATURE_MASK_USER_DYNAMIC;
> + ufeatures &= ~XFEATURE_MASK_USER_DYNAMIC;
> +
> + fpu_kernel_cfg.default_features = kfeatures;
> + fpu_user_cfg.default_features = ufeatures;
> +
> + guest_default_cfg.features = kfeatures;
> +}
> +
> /*
> * Enable and initialize the xsave feature.
> * Called once per system bootup.
> @@ -854,12 +873,8 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
> fpu_user_cfg.max_features = fpu_kernel_cfg.max_features;
> fpu_user_cfg.max_features &= XFEATURE_MASK_USER_SUPPORTED;
>
> - /* Clean out dynamic features from default */
> - fpu_kernel_cfg.default_features = fpu_kernel_cfg.max_features;
> - fpu_kernel_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
> -
> - fpu_user_cfg.default_features = fpu_user_cfg.max_features;
> - fpu_user_cfg.default_features &= ~XFEATURE_MASK_USER_DYNAMIC;
> + /* Now, given maximum feature set, determine default values */
> + init_default_features(fpu_kernel_cfg.max_features, fpu_user_cfg.max_features);
Passing in max_features is rather odd. I assume the intent is to capture the
dependencies, but that falls apart by the end of series as the guest features
are initialized as:
guest_default_cfg.features = kfeatures | xfeatures_mask_guest_supervisor();
where xfeatures_mask_guest_supervisor() sneakily consumes fpu_kernel_cfg.max_features,
the very field this patch deliberately avoids consuming directly.
static inline u64 xfeatures_mask_guest_supervisor(void)
{
return fpu_kernel_cfg.max_features & XFEATURE_MASK_GUEST_SUPERVISOR;
}
Rather than providing a helper to initialize the defaults, what if we provide
helpers to provide the default *masks*? Then the dependencies on max_features
are super clear.
E.g. spread over multiple patches (completely untested)
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index be1cdfa9b00d..e52c7517df5f 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -780,27 +780,22 @@ static void __init fpu__init_disable_system_xstate(unsigned int legacy_size)
fpstate_reset(x86_task_fpu(current));
}
-static void __init init_default_features(u64 kernel_max_features, u64 user_max_features)
+static u64 __init host_default_mask(void)
{
- u64 kfeatures = kernel_max_features;
- u64 ufeatures = user_max_features;
-
/*
- * Default feature sets should not include dynamic and guest-only
- * xfeatures at all.
+ * Exclude dynamic features (require userspace opt-in) and features
+ * that are supported only for KVM guests.
*/
- kfeatures &= ~(XFEATURE_MASK_USER_DYNAMIC | XFEATURE_MASK_GUEST_SUPERVISOR);
- ufeatures &= ~XFEATURE_MASK_USER_DYNAMIC;
-
- fpu_kernel_cfg.default_features = kfeatures;
- fpu_user_cfg.default_features = ufeatures;
+ return ~((u64)XFEATURE_MASK_USER_DYNAMIC | XFEATURE_MASK_GUEST_SUPERVISOR);
+}
+static u64 __init guest_default_mask(void)
+{
/*
- * Ensure VCPU FPU container only reserves a space for guest-only
- * xfeatures. This distinction can save kernel memory by
- * maintaining a necessary amount of XSAVE buffer.
+ * Exclude dynamic features, which require userspace opt-in even for
+ * KVM guests.
*/
- guest_default_cfg.features = kfeatures | xfeatures_mask_guest_supervisor();
+ return ~(u64)XFEATURE_MASK_USER_DYNAMIC;
}
/*
@@ -886,7 +881,9 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
fpu_user_cfg.max_features &= XFEATURE_MASK_USER_SUPPORTED;
/* Now, given maximum feature set, determine default values */
- init_default_features(fpu_kernel_cfg.max_features, fpu_user_cfg.max_features);
+ fpu_kernel_cfg.default_features = fpu_kernel_cfg.max_features & host_default_mask();
+ fpu_user_cfg.default_features = fpu_user_cfg.max_features & host_default_mask();
+ guest_default_cfg.features = fpu_kernel_cfg.max_features & guest_default_mask();
/* Store it for paranoia check at the end */
xfeatures = fpu_kernel_cfg.max_features;
diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
index 9e496391b5f0..52ce19289989 100644
--- a/arch/x86/kernel/fpu/xstate.h
+++ b/arch/x86/kernel/fpu/xstate.h
@@ -62,11 +62,6 @@ static inline u64 xfeatures_mask_supervisor(void)
return fpu_kernel_cfg.max_features & XFEATURE_MASK_SUPERVISOR_SUPPORTED;
}
-static inline u64 xfeatures_mask_guest_supervisor(void)
-{
- return fpu_kernel_cfg.max_features & XFEATURE_MASK_GUEST_SUPERVISOR;
-}
-
static inline u64 xfeatures_mask_independent(void)
{
if (!cpu_feature_enabled(X86_FEATURE_ARCH_LBR))
Powered by blists - more mailing lists