From 3a6a60ff269da56e9af3bfa7e9b58247f22d3e0d Mon Sep 17 00:00:00 2001 From: dehanj Date: Fri, 2 Feb 2024 14:20:21 +0100 Subject: [PATCH] fw: Protect zeroisation against compiler optimisation. The memset() responsible for the zeroisation of the secure_ctx under the compute_cdi() function in FW's main.c, was optimised away by the compiler. Instead of using memset(), secure_wipe() is introduced which uses a volatile keyword to prevent the compiler to try to optimise it. Secure_wipe() is now used on all locations handling removal of sensitive data. --- hw/application_fpga/fw/tk1/lib.c | 7 +++++++ hw/application_fpga/fw/tk1/lib.h | 2 +- hw/application_fpga/fw/tk1/main.c | 4 ++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/hw/application_fpga/fw/tk1/lib.c b/hw/application_fpga/fw/tk1/lib.c index ac8b69a..61289a2 100644 --- a/hw/application_fpga/fw/tk1/lib.c +++ b/hw/application_fpga/fw/tk1/lib.c @@ -155,3 +155,10 @@ int memeq(void *dest, const void *src, size_t n) return res; } + +void secure_wipe(void *v, size_t n) +{ + volatile uint8_t *p = (volatile uint8_t *)v; + while (n--) + *p++ = 0; +} diff --git a/hw/application_fpga/fw/tk1/lib.h b/hw/application_fpga/fw/tk1/lib.h index 72dc795..b7944b3 100644 --- a/hw/application_fpga/fw/tk1/lib.h +++ b/hw/application_fpga/fw/tk1/lib.h @@ -28,5 +28,5 @@ void *memset(void *dest, int c, unsigned n); void memcpy_s(void *dest, size_t destsize, const void *src, size_t n); void wordcpy_s(void *dest, size_t destsize, const void *src, size_t n); int memeq(void *dest, const void *src, size_t n); - +void secure_wipe(void *v, size_t n); #endif diff --git a/hw/application_fpga/fw/tk1/main.c b/hw/application_fpga/fw/tk1/main.c index 8022cff..f819ad7 100644 --- a/hw/application_fpga/fw/tk1/main.c +++ b/hw/application_fpga/fw/tk1/main.c @@ -115,7 +115,7 @@ static void compute_cdi(const uint8_t *digest, const uint8_t use_uss, // while on the firmware stack which is in the special fw_ram. wordcpy_s(local_uds, 8, (void *)uds, 8); blake2s_update(&secure_ctx, (const void *)local_uds, 32); - (void)memset(local_uds, 0, 32); + (void)secure_wipe(local_uds, sizeof(local_uds)); // Update with TKey program digest blake2s_update(&secure_ctx, digest, 32); @@ -130,7 +130,7 @@ static void compute_cdi(const uint8_t *digest, const uint8_t use_uss, // Clear secure_ctx of any residue of UDS. Don't want to keep // that for long even though fw_ram is cleared later. - (void)memset(&secure_ctx, 0, sizeof(secure_ctx)); + (void)secure_wipe(&secure_ctx, sizeof(secure_ctx)); // CDI only word writable wordcpy_s((void *)cdi, 8, &local_cdi, 8);