Especially with our use of __builtin_memset() to implement memset() the
compiler is free to eliminate instances when it can prove that the
affected object is dead. Introduce a small helper function accompanying
the memset() with a construct forcing the compiler to retain the
clearing of (stack) memory.
Fixes: c021c95498d9 ("x86: Replace our own specialised versions of memset and
memcpy with")
Signed-off-by: Jan Beulich <[email protected]>
---
The Fixes: tag names the commit which broke the wrong assumption made by
6deab1ae316b ("txt: perform per-domain (and frametable and xenheap) MAC
on entry into").
--- a/xen/arch/x86/tboot.c
+++ b/xen/arch/x86/tboot.c
@@ -179,6 +179,17 @@ static void update_iommu_mac(vmac_ctx_t
#define is_page_in_use(page) \
(page_state_is(page, inuse) || page_state_is(page, offlining))
+/* Wipe ctx to ensure key is not left in memory. */
+static void wipe_ctx(vmac_ctx_t *ctx)
+{
+ memset(ctx, 0, sizeof(*ctx));
+ /*
+ * Make sure the compiler won't optimize out the memset(), for the local
+ * variable (at the call sites) going out of scope right afterwards.
+ */
+ asm volatile ( "" :: "m" (*ctx) );
+}
+
static void tboot_gen_domain_integrity(const uint8_t key[TB_KEY_SIZE],
vmac_t *mac)
{
@@ -216,8 +227,7 @@ static void tboot_gen_domain_integrity(c
*mac = vmac(NULL, 0, nonce, NULL, &ctx);
- /* wipe ctx to ensure key is not left in memory */
- memset(&ctx, 0, sizeof(ctx));
+ wipe_ctx(&ctx);
}
/*
@@ -278,8 +288,7 @@ static void tboot_gen_xenheap_integrity(
}
*mac = vmac(NULL, 0, nonce, NULL, &ctx);
- /* wipe ctx to ensure key is not left in memory */
- memset(&ctx, 0, sizeof(ctx));
+ wipe_ctx(&ctx);
}
static void tboot_gen_frametable_integrity(const uint8_t key[TB_KEY_SIZE],
@@ -307,8 +316,7 @@ static void tboot_gen_frametable_integri
*mac = vmac(NULL, 0, nonce, NULL, &ctx);
- /* wipe ctx to ensure key is not left in memory */
- memset(&ctx, 0, sizeof(ctx));
+ wipe_ctx(&ctx);
}
void tboot_shutdown(uint32_t shutdown_type)
