Hi,
On 11/09/2023 16:01, Jan Beulich wrote:
[1] specifies a long list of instructions which are intended to exhibit
timing behavior independent of the data they operate on. On certain
hardware this independence is optional, controlled by a bit in a new
MSR. Provide a command line option to control the mode Xen and its
guests are to operate in, with a build time control over the default.
Longer term we may want to allow guests to control this.
If I read correctly the discussion on the previous version. There was
some concern with this version of patch. I can't find anything public
suggesting that the concerned were dismissed. Do you have any pointer?
Since Arm64 supposedly also has such a control, put command line option
and Kconfig control in common files.
[1]
https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/best-practices/data-operand-independent-timing-isa-guidance.html
Requested-by: Demi Marie Obenour <d...@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeul...@suse.com>
---
This may be viewed as a new feature, and hence be too late for 4.18. It
may, however, also be viewed as security relevant, which is why I'd like
to propose to at least consider it.
Slightly RFC, in particular for whether the Kconfig option should
default to Y or N.
I am not entirely sure. I understand that DIT will help in the
cryptographic case but it is not clear to me what is the performance impact.
I would have wanted to invoke setup_doitm() from cpu_init(), but that
works only on the BSP. On APs cpu_init() runs before ucode loading.
Plus recheck_cpu_features() invoking identify_cpu() takes care of the
BSP during S3 resume.
---
v2: Introduce and use cpu_has_doitm. Add comment "borrowed" from the
XenServer patch queue patch providing similar functionality.
Re-base.
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -788,6 +788,14 @@ Specify the size of the console debug tr
additionally a trace buffer of the specified size is allocated per cpu.
The debug trace feature is only enabled in debugging builds of Xen.
+### dit (x86)
+> `= <boolean>`
+
+> Default: `CONFIG_DIT_DEFAULT`
+
+Specify whether Xen and guests should operate in Data Independent Timing
+mode.
+
### dma_bits
> `= <integer>`
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -15,6 +15,7 @@ config X86
select HAS_ALTERNATIVE
select HAS_COMPAT
select HAS_CPUFREQ
+ select HAS_DIT
select HAS_EHCI
select HAS_EX_TABLE
select HAS_FAST_MULTIPLY
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -204,6 +204,28 @@ void ctxt_switch_levelling(const struct
alternative_vcall(ctxt_switch_masking, next);
}
+static void setup_doitm(void)
+{
+ uint64_t msr;
+
+ if ( !cpu_has_doitm )
I am not very familiar with the feature. If it is not present, then can
we guarantee that the instructions will be executed in constant time?
If not, I think we should taint Xen and/or print a message if the admin
requested to use DIT. This would make clear that the admin is trying to
do something that doesn't work.
+ return;
+
+ /*
+ * We don't currently enumerate DOITM to guests. As a conseqeuence, guest
+ * kernels will believe they're safe even when they are not.
+ *
+ * For now, set it unilaterally. This prevents otherwise-correct crypto
+ * code from becoming vulnerable to timing sidechannels.
+ */
+
+ rdmsrl(MSR_UARCH_MISC_CTRL, msr);
+ msr |= UARCH_CTRL_DOITM;
+ if ( !opt_dit )
+ msr &= ~UARCH_CTRL_DOITM;
+ wrmsrl(MSR_UARCH_MISC_CTRL, msr);
+}
+
bool opt_cpu_info;
boolean_param("cpuinfo", opt_cpu_info);
@@ -589,6 +611,8 @@ void identify_cpu(struct cpuinfo_x86 *c)
mtrr_bp_init();
}
+
+ setup_doitm();
}
/* leaf 0xb SMT level */
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -201,6 +201,7 @@ static inline bool boot_cpu_has(unsigned
#define cpu_has_if_pschange_mc_no boot_cpu_has(X86_FEATURE_IF_PSCHANGE_MC_NO)
#define cpu_has_tsx_ctrl boot_cpu_has(X86_FEATURE_TSX_CTRL)
#define cpu_has_taa_no boot_cpu_has(X86_FEATURE_TAA_NO)
+#define cpu_has_doitm boot_cpu_has(X86_FEATURE_DOITM)
#define cpu_has_fb_clear boot_cpu_has(X86_FEATURE_FB_CLEAR)
#define cpu_has_rrsba boot_cpu_has(X86_FEATURE_RRSBA)
#define cpu_has_gds_ctrl boot_cpu_has(X86_FEATURE_GDS_CTRL)
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -41,6 +41,9 @@ config HAS_COMPAT
config HAS_DEVICE_TREE
bool
+config HAS_DIT # Data Independent Timing
+ bool
+
config HAS_EX_TABLE
bool
@@ -175,6 +178,18 @@ config SPECULATIVE_HARDEN_GUEST_ACCESS
endmenu
+config DIT_DEFAULT
+ bool "Data Independent Timing default"
+ depends on HAS_DIT
+ help
+ Hardware often surfaces instructions the timing of which is dependent
+ on the data they process. Some of these instructions may be used in
+ timing sensitive environments, e.g. cryptography. When such
+ instructions exist, hardware may further surface a control allowing
+ to make the behavior of such instructions independent of the data
+ they act upon. Choose the default here for when no "dit" command line
+ option is present.
+
config HYPFS
bool "Hypervisor file system support"
default y
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -28,6 +28,11 @@ CHECK_feature_info;
enum system_state system_state = SYS_STATE_early_boot;
+#ifdef CONFIG_HAS_DIT
+bool __ro_after_init opt_dit = IS_ENABLED(CONFIG_DIT_DEFAULT);
+boolean_param("dit", opt_dit);
+#endif
+
static xen_commandline_t saved_cmdline;
static const char __initconst opt_builtin_cmdline[] = CONFIG_CMDLINE;
--- a/xen/include/xen/param.h
+++ b/xen/include/xen/param.h
@@ -184,6 +184,8 @@ extern struct param_hypfs __paramhypfs_s
string_param(_name, _var); \
string_runtime_only_param(_name, _var)
+extern bool opt_dit;
+
static inline void no_config_param(const char *cfg, const char *param,
const char *s, const char *e)
{
Cheers,
[1]
https://lore.kernel.org/all/8f07c532-e742-fa02-27ee-b08c56299...@citrix.com/
--
Julien Grall
