boot: Simplify object erasure

This erase_obj() machinery looks like voodoo and creates an awful lot of
noise as soon as we get back to building with -O0. We can do this in a
more simple way by introducing a struct that holds the information we
need on cleanup. When building with optimization enabled, all this gets
inlined and the eraser vanishes.

diff --git a/.github/codeql-queries/UninitializedVariableWithCleanup.ql b/.github/codeql-queries/UninitializedVariableWithCleanup.ql
index dadc6cb1b5ee3c45360dc870cb95958be2f4e90d..e514111f282c0f2639a72f8eb3fda06a43f61293 100644 (file)
--- a/.github/codeql-queries/UninitializedVariableWithCleanup.ql
+++ b/.github/codeql-queries/UninitializedVariableWithCleanup.ql
@@ -20,7 +20,7 @@ import semmle.code.cpp.controlflow.StackVariableReachability
   * since they don't do anything illegal even when the variable is uninitialized
   */
 predicate cleanupFunctionDenyList(string fun) {
-  fun = "erase_char" or fun = "erase_obj"
+  fun = "erase_char"
 }
 
 /**

diff --git a/src/basic/memory-util.c b/src/basic/memory-util.c
index 298376211761b4dd43e59b8f9f901b15b4492545..c4f54c7b4ed97196d9fdb1391497b4e7230db80f 100644 (file)
--- a/src/basic/memory-util.c
+++ b/src/basic/memory-util.c
@@ -38,21 +38,3 @@ bool memeqbyte(uint8_t byte, const void *data, size_t length) {
         /* Now we know first 16 bytes match, memcmp() with self.  */
         return memcmp(data, p + 16, length) == 0;
 }
-
-#if !HAVE_EXPLICIT_BZERO
-/*
- * The pointer to memset() is volatile so that compiler must de-reference the pointer and can't assume that
- * it points to any function in particular (such as memset(), which it then might further "optimize"). This
- * approach is inspired by openssl's crypto/mem_clr.c.
- */
-typedef void *(*memset_t)(void *,int,size_t);
-
-static volatile memset_t memset_func = memset;
-
-void* explicit_bzero_safe(void *p, size_t l) {
-        if (l > 0)
-                memset_func(p, '\0', l);
-
-        return p;
-}
-#endif

diff --git a/src/basic/memory-util.h b/src/basic/memory-util.h
index 6e3280b9dfe98f48f920f079152df4391e95e3c8..428ccc210cdb61615efdf9b8fe2922956d4afa85 100644 (file)
--- a/src/basic/memory-util.h
+++ b/src/basic/memory-util.h
@@ -9,6 +9,7 @@
 
 #include "alloc-util.h"
 #include "macro.h"
+#include "memory-util-fundamental.h"
 
 size_t page_size(void) _pure_;
 #define PAGE_ALIGN(l) ALIGN_TO((l), page_size())
@@ -91,17 +92,6 @@ static inline void *mempmem_safe(const void *haystack, size_t haystacklen, const
         return (uint8_t*) p + needlelen;
 }
 
-#if HAVE_EXPLICIT_BZERO
-static inline void* explicit_bzero_safe(void *p, size_t l) {
-        if (l > 0)
-                explicit_bzero(p, l);
-
-        return p;
-}
-#else
-void *explicit_bzero_safe(void *p, size_t l);
-#endif
-
 static inline void* erase_and_free(void *p) {
         size_t l;
 

diff --git a/src/boot/efi/efi-string.h b/src/boot/efi/efi-string.h
index e12add0b19908b6cca1ac2157e3db95b3404a9f3..25931a7d6e52566e57674ead7f6425336a7aa1be 100644 (file)
--- a/src/boot/efi/efi-string.h
+++ b/src/boot/efi/efi-string.h
@@ -125,12 +125,6 @@ static inline void *mempcpy(void * restrict dest, const void * restrict src, siz
         return (uint8_t *) dest + n;
 }
 
-static inline void explicit_bzero_safe(void *bytes, size_t len) {
-        if (!bytes || len == 0)
-                return;
-        memset(bytes, 0, len);
-        __asm__ __volatile__("": :"r"(bytes) :"memory");
-}
 #else
 /* For unit testing. */
 int efi_memcmp(const void *p1, const void *p2, size_t n);

diff --git a/src/boot/efi/random-seed.c b/src/boot/efi/random-seed.c
index 5b0693dfe1182c9f9555bc53ebc0e05347102123..332f537d91c74a44be168fc2e7fb30a107a7c058 100644 (file)
--- a/src/boot/efi/random-seed.c
+++ b/src/boot/efi/random-seed.c
@@ -3,6 +3,7 @@
 #include <efi.h>
 #include <efilib.h>
 
+#include "memory-util-fundamental.h"
 #include "missing_efi.h"
 #include "random-seed.h"
 #include "secure-boot.h"
@@ -117,19 +118,23 @@ static void validate_sha256(void) {
 }
 
 EFI_STATUS process_random_seed(EFI_FILE *root_dir) {
-        _cleanup_erase_ uint8_t random_bytes[DESIRED_SEED_SIZE], hash_key[HASH_VALUE_SIZE];
+        uint8_t random_bytes[DESIRED_SEED_SIZE], hash_key[HASH_VALUE_SIZE];
         _cleanup_free_ struct linux_efi_random_seed *new_seed_table = NULL;
         struct linux_efi_random_seed *previous_seed_table = NULL;
         _cleanup_free_ void *seed = NULL, *system_token = NULL;
         _cleanup_(file_closep) EFI_FILE *handle = NULL;
         _cleanup_free_ EFI_FILE_INFO *info = NULL;
-        _cleanup_erase_ struct sha256_ctx hash;
+        struct sha256_ctx hash;
         uint64_t uefi_monotonic_counter = 0;
         size_t size, rsize, wsize;
         bool seeded_by_efi = false;
         EFI_STATUS err;
         EFI_TIME now;
 
+        CLEANUP_ERASE(random_bytes);
+        CLEANUP_ERASE(hash_key);
+        CLEANUP_ERASE(hash);
+
         assert(root_dir);
         assert_cc(DESIRED_SEED_SIZE == HASH_VALUE_SIZE);
 

diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h
index cc750d7cca3ff6c2a044e445eb477e97832f82b0..836f223cc053a44179b9d9558c6375ef2d4d106a 100644 (file)
--- a/src/boot/efi/util.h
+++ b/src/boot/efi/util.h
@@ -10,17 +10,6 @@
 #define UINTN_MAX (~(UINTN)0)
 #define INTN_MAX ((INTN)(UINTN_MAX>>1))
 
-#ifndef __has_attribute
-#define __has_attribute(x) 0
-#endif
-#if __has_attribute(__error__)
-__attribute__((noreturn)) extern void __assert_cl_failure__(void) __attribute__((__error__("compile-time assertion failed")));
-#else
-__attribute__((noreturn)) extern void __assert_cl_failure__(void);
-#endif
-/* assert_cl generates a later-stage compile-time assertion when constant folding occurs. */
-#define assert_cl(condition) ({ if (!(condition)) __assert_cl_failure__(); })
-
 /* gnu-efi format specifiers for integers are fixed to either 64bit with 'l' and 32bit without a size prefix.
  * We rely on %u/%d/%x to format regular ints, so ensure the size is what we expect. At the same time, we also
  * need specifiers for (U)INTN which are native (pointer) sized. */
@@ -54,20 +43,6 @@ static inline void freep(void *p) {
 
 #define _cleanup_free_ _cleanup_(freep)
 
-static __always_inline void erase_obj(void *p) {
-#ifdef __OPTIMIZE__
-        size_t l;
-        assert_cl(p);
-        l = __builtin_object_size(p, 0);
-        assert_cl(l != (size_t) -1);
-        explicit_bzero_safe(p, l);
-#else
-#warning "Object will not be erased with -O0; do not release to production."
-#endif
-}
-
-#define _cleanup_erase_ _cleanup_(erase_obj)
-
 _malloc_ _alloc_(1) _returns_nonnull_ _warn_unused_result_
 static inline void *xmalloc(size_t size) {
         void *p;

diff --git a/src/fundamental/memory-util-fundamental.h b/src/fundamental/memory-util-fundamental.h
new file mode 100644 (file)
index 0000000..9015300
--- /dev/null
+++ b/src/fundamental/memory-util-fundamental.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+#pragma once
+
+#include <stddef.h>
+
+#ifdef SD_BOOT
+#  include "efi-string.h"
+#else
+#  include <string.h>
+#endif
+
+#include "macro-fundamental.h"
+
+#if defined(HAVE_EXPLICIT_BZERO)
+static inline void *explicit_bzero_safe(void *p, size_t l) {
+        if (p && l > 0)
+                explicit_bzero(p, l);
+
+        return p;
+}
+#else
+static inline void *explicit_bzero_safe(void *p, size_t l) {
+        if (p && l > 0) {
+                memset(p, 0, l);
+                __asm__ __volatile__("" : : "r"(p) : "memory");
+        }
+        return p;
+}
+#endif
+
+struct VarEraser {
+        void *p;
+        size_t size;
+};
+
+static inline void erase_var(struct VarEraser *e) {
+        explicit_bzero_safe(e->p, e->size);
+}
+
+/* Mark var to be erased when leaving scope. */
+#define CLEANUP_ERASE(var) \
+        _cleanup_(erase_var) _unused_ struct VarEraser CONCATENATE(_eraser_, UNIQ) = { .p = &var, .size = sizeof(var) }

diff --git a/src/fundamental/meson.build b/src/fundamental/meson.build
index 3810d6b456de6146ac112f168f7d5927b2265dce..4b8e32337ddd5b726f4abf45ebbbe94a43cf114b 100644 (file)
--- a/src/fundamental/meson.build
+++ b/src/fundamental/meson.build
@@ -6,6 +6,7 @@ fundamental_headers = files(
         'bootspec-fundamental.h',
         'efivars-fundamental.h',
         'macro-fundamental.h',
+        'memory-util-fundamental.h',
         'sha256.h',
         'string-util-fundamental.h',
         'tpm-pcr.h',