[PATCH 2/3] Do exploit constructors for registering drivers

[Jan Kiszka]

From: Jan Kiszka <jan.k...@siemens.com>

Unfortunately, it turned out that we are creating holes in the final EFI
binaries by using sections that are not know to the gnu-efi linker
script. These wholes cause troubles when signing and specifically later
on verifying the signatures.

Avoid this by moving to a classic driver registration pattern where
constructor functions call a central services which adds the passed
driver descriptor to a list. This list is then later on walked for the
actual probing.

That works nicely for gnu-efi 3.0.16 and later after it added support
for calling constructors during init. For older versions, we have to do
that ourselves, though. We are using part of the old pattern for it
which annotated start and end of the init_array and manually walked it,
calling the listed functions.

Signed-off-by: Jan Kiszka <jan.k...@siemens.com>
---
Makefile.am | 2 +-
configure.ac | 1 +
drivers/watchdog/wdfuncs_end.c | 10 ++++++---
drivers/watchdog/wdfuncs_start.c | 10 ++++++---
include/utils.h | 19 ++++++++++++-----
main.c | 35 ++++++++++++++++++++++++--------
scripts/cppcheck.sh | 5 +++--
7 files changed, 60 insertions(+), 22 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 35eb08c..9d5aa28 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -360,7 +360,7 @@ $(efi_solib): $(efi_objects)
nm -D -u $@ | grep ' U ' && exit 1 || :

$(efi_loadername): $(efi_solib)
- $(AM_V_GEN) $(OBJCOPY) $(efi_sections) -j .wdfuncs \
+ $(AM_V_GEN) $(OBJCOPY) $(efi_sections) -j .init_array \
$(objcopy_format) $< $@

$(kernel_stub_solib): $(kernel_stub_objects)
diff --git a/configure.ac b/configure.ac
index 59f9e03..038930b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -235,6 +235,7 @@ fi
# GNU_EFI_VERSION resolves to gnu-efi's version without dots, e.g., GNU_EFI_VERSION=3016
# gnu-efi versions < 3.0.16 resolve to GNU_EFI_VERSION=0
AC_SUBST([GNU_EFI_VERSION], [$(($PKG_CONFIG --modversion "gnu-efi" 2>/dev/null || echo 0) | $TR -d '.' )])
+AC_DEFINE_UNQUOTED([GNU_EFI_VERSION], [${GNU_EFI_VERSION}], [gnu-efi version])

AC_SUBST([OBJCOPY_HAS_EFI_APP_TARGET], [$($OBJCOPY --info | $GREP -q pei- 2>/dev/null && echo "true")])

diff --git a/drivers/watchdog/wdfuncs_end.c b/drivers/watchdog/wdfuncs_end.c
index 792cbdf..d19a0ab 100644
--- a/drivers/watchdog/wdfuncs_end.c
+++ b/drivers/watchdog/wdfuncs_end.c
@@ -1,7 +1,7 @@
/*
* EFI Boot Guard
*
- * Copyright (c) Siemens AG, 2024
+ * Copyright (c) Siemens AG, 2024-2025
*
* Authors:
* Christian Storm <christi...@siemens.com>
@@ -12,10 +12,14 @@
* SPDX-License-Identifier: GPL-2.0-only
*/

+#if GNU_EFI_VERSION < 3016
+
#include <efi.h>
#include "utils.h"

-/* Section .wdfunc's end address for watchdog probing function pointers
+/* Section .init_array's end address for watchdog probing function pointers
* preceding this marker, if any. */
-WATCHDOG_PROBE wdfuncs_end __attribute__((used, section(".wdfuncs"))) =
+WATCHDOG_PROBE wdfuncs_end __attribute__((used, section(".init_array"))) =
(WATCHDOG_PROBE)0x4353;
+
+#endif
diff --git a/drivers/watchdog/wdfuncs_start.c b/drivers/watchdog/wdfuncs_start.c
index e327ac9..bdf47f5 100644
--- a/drivers/watchdog/wdfuncs_start.c
+++ b/drivers/watchdog/wdfuncs_start.c
@@ -1,7 +1,7 @@
/*
* EFI Boot Guard
*
- * Copyright (c) Siemens AG, 2024
+ * Copyright (c) Siemens AG, 2024-2025
*
* Authors:
* Christian Storm <christi...@siemens.com>
@@ -12,10 +12,14 @@
* SPDX-License-Identifier: GPL-2.0-only
*/

+#if GNU_EFI_VERSION < 3016
+
#include <efi.h>
#include "utils.h"

-/* Section .wdfunc's sentinel value and start address for watchdog probing
+/* Section .init_array's sentinel value and start address for watchdog probing
* function pointers following this marker, if any. */
-WATCHDOG_PROBE wdfuncs_start __attribute__((used, section(".wdfuncs"))) =
+WATCHDOG_PROBE wdfuncs_start __attribute__((used, section(".init_array"))) =
(WATCHDOG_PROBE)0x5343;
+
+#endif
diff --git a/include/utils.h b/include/utils.h
index 101e30b..54b9a42 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -1,7 +1,7 @@
/*
* EFI Boot Guard
*
- * Copyright (c) Siemens AG, 2017-2021
+ * Copyright (c) Siemens AG, 2017-2025
*
* Authors:
* Jan Kiszka <jan.k...@siemens.com>
@@ -42,11 +42,20 @@ EFI_DEVICE_PATH *FileDevicePathFromConfig(EFI_HANDLE device,
CHAR16 *GetBootMediumPath(const CHAR16 *input);

typedef EFI_STATUS (*WATCHDOG_PROBE)(EFI_PCI_IO *, UINT16, UINT16, UINTN);
-#define _CONCAT(prefix, func) prefix ## func
-#define CONCAT(prefix, func) _CONCAT(prefix, func)
+
+typedef struct _WATCHDOG_DRIVER {
+ WATCHDOG_PROBE probe;
+ struct _WATCHDOG_DRIVER *next;
+} WATCHDOG_DRIVER;
+
+VOID register_watchdog(WATCHDOG_DRIVER *driver);
+
#define WATCHDOG_REGISTER(_func) \
- __attribute__((used, section(".wdfuncs"))) static WATCHDOG_PROBE \
- CONCAT(wdfuncs##_, _func) = (WATCHDOG_PROBE)_func;
+ static WATCHDOG_DRIVER this_driver = {.probe = _func}; \
+ static void __attribute__((constructor)) register_driver(void) \
+ { \
+ register_watchdog(&this_driver); \
+ }

VOID PrintC(const UINT8 color, const CHAR16 *fmt, ...);
#define ERROR(fmt, ...) \
diff --git a/main.c b/main.c
index fdd5e93..5d61385 100644
--- a/main.c
+++ b/main.c
@@ -1,7 +1,7 @@
/*
* EFI Boot Guard
*
- * Copyright (c) Siemens AG, 2017
+ * Copyright (c) Siemens AG, 2017-2025
*
* Authors:
* Jan Kiszka <jan.k...@siemens.com>
@@ -31,10 +31,27 @@ extern CHAR16 *boot_medium_path;
#define PCI_GET_VENDOR_ID(id) (UINT16)(id)
#define PCI_GET_PRODUCT_ID(id) (UINT16)((id) >> 16)

+static WATCHDOG_DRIVER *watchdog_drivers;
+static WATCHDOG_DRIVER *last_watchdog_driver;
+
+VOID register_watchdog(WATCHDOG_DRIVER *driver)
+{
+ if (last_watchdog_driver != NULL)
+ last_watchdog_driver->next = driver;
+ else
+ watchdog_drivers = driver;
+ last_watchdog_driver = driver;
+}

static EFI_STATUS probe_watchdogs(UINTN timeout)
{
- if (wdfuncs_end - wdfuncs_start - 1 == 0) {
+#if GNU_EFI_VERSION < 3016
+ const unsigned long *entry = wdfuncs_start;
+ for (entry++; entry < wdfuncs_end; entry++) {
+ ((void (*)(void))*entry)();
+ }
+#endif
+ if (watchdog_drivers == NULL) {
if (timeout > 0) {
ERROR(L"No watchdog drivers registered, but timeout is non-zero.\n");
return EFI_UNSUPPORTED;
@@ -83,14 +100,16 @@ static EFI_STATUS probe_watchdogs(UINTN timeout)
continue;
}

- const unsigned long *entry = wdfuncs_start;
- for (entry++; entry < wdfuncs_end; entry++) {
- WATCHDOG_PROBE probe = (WATCHDOG_PROBE) *entry;
- if ((status = probe(pci_io, PCI_GET_VENDOR_ID(value),
- PCI_GET_PRODUCT_ID(value),
- timeout)) == EFI_SUCCESS) {
+ WATCHDOG_DRIVER *driver = watchdog_drivers;
+ while (driver) {
+ status = driver->probe(pci_io,
+ PCI_GET_VENDOR_ID(value),
+ PCI_GET_PRODUCT_ID(value),
+ timeout);
+ if (status == EFI_SUCCESS) {
break;
}
+ driver = driver->next;
}

(VOID) BS->CloseProtocol(handle_buffer[index], &PciIoProtocol,
diff --git a/scripts/cppcheck.sh b/scripts/cppcheck.sh
index 0bd1310..d6e4aba 100755
--- a/scripts/cppcheck.sh
+++ b/scripts/cppcheck.sh
@@ -45,8 +45,9 @@ suppress+=" --suppress=nullPointerRedundantCheck:kernel-stub/main.c"
suppress+=" --suppress=unusedStructMember:kernel-stub/main.c"
suppress+=" --suppress=unusedStructMember:kernel-stub/fdt.c"
# Not applicable because of API requirements
-suppress+=" --suppress=constParameterCallback:drivers/watchdog/ipc4x7e_wdt.c"
-suppress+=" --suppress=constParameterCallback:drivers/watchdog/w83627hf_wdt.c"
+suppress+=" --suppress=constParameterPointer:drivers/watchdog/ipc4x7e_wdt.c"
+suppress+=" --suppress=constParameterPointer:drivers/watchdog/w83627hf_wdt.c"
+# Not applicable because of API requirements
suppress+=" --suppress=constParameterCallback:kernel-stub/initrd.c"

enable="--enable=warning \
--
2.43.0

[Storm, Christian]

Hi Jan,

> Unfortunately, it turned out that we are creating holes in the final EFI
> binaries by using sections that are not know

s/know/known/

> to the gnu-efi linker
> script. These wholes

s/wholes/holes/g

Kind regards,
Christian

--
Dr. Christian Storm
Siemens AG, FT RPD CED
Friedrich-Ludwig-Bauer-Str. 3, 85748 Garching, Germany

[Jan Kiszka]

On 05.09.25 18:48, 'Storm, Christian' via EFI Boot Guard wrote:
> Hi Jan,
>
>> Unfortunately, it turned out that we are creating holes in the final EFI
>> binaries by using sections that are not know
>
> s/know/known/
>
>> to the gnu-efi linker
>> script. These wholes
>
> s/wholes/holes/g
>

I only found this one case - will correct both findings in next.

Thanks!
Jan

Siemens AG, Foundational Technologies
Linux Expert Center