From: Christian Storm <christian.st...@siemens.com>

gnu-efi 3.0.16 commit 4f8b339 dated 2023-03-28
    "Make ELF constructors and destructors work: .init_array"
introduces proper support for ELF constructors and destructors
transcribing it to EFI binaries.

With the watchdog drivers' __attribute__((constructor)) decorated
probe functions, pointers to them are put in the ELF's .init_array
section by the compiler. This fact is exploited for easy iteration
over the probing functions in EFI Boot Guard.
gnu-efi 3.0.16, however, calls the .init_array recorded functions
in its _entry() function prior to calling efi_main(), just like the
mechanism for "regular" libc applications is.

The probing functions don't qualify for being called as constructors
as initialization has not been done at this point. Hence, assemble
the probing function pointer list proper in a dedicated .wdfuncs
section to work on gnu-efi >= 3.0.15.

Signed-off-by: Christian Storm <christian.st...@siemens.com>
---
 Makefile.am                         |  8 ++++----
 drivers/watchdog/amdfch_wdt.c       |  7 ++++---
 drivers/watchdog/atom-quark.c       |  7 ++++---
 drivers/watchdog/hpwdt.c            |  7 ++++---
 drivers/watchdog/i6300esb.c         |  7 ++++---
 drivers/watchdog/init_array_end.S   | 17 -----------------
 drivers/watchdog/init_array_start.S | 23 -----------------------
 drivers/watchdog/ipc4x7e_wdt.c      |  7 ++++---
 drivers/watchdog/ipmi_wdt.c         | 11 ++++++-----
 drivers/watchdog/itco.c             |  7 ++++---
 drivers/watchdog/w83627hf_wdt.c     |  7 ++++---
 drivers/watchdog/wdat.c             | 11 ++++++-----
 drivers/watchdog/wdfuncs_end.c      | 21 +++++++++++++++++++++
 drivers/watchdog/wdfuncs_start.c    | 21 +++++++++++++++++++++
 include/utils.h                     |  7 +++++++
 main.c                              | 11 +++++------
 scripts/cppcheck.sh                 |  2 +-
 17 files changed, 99 insertions(+), 82 deletions(-)
 delete mode 100644 drivers/watchdog/init_array_end.S
 delete mode 100644 drivers/watchdog/init_array_start.S
 create mode 100644 drivers/watchdog/wdfuncs_end.c
 create mode 100644 drivers/watchdog/wdfuncs_start.c

diff --git a/Makefile.am b/Makefile.am
index 6ad18e7..47dfbe5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -182,9 +182,9 @@ efi_sources_watchdogs =
 endif
 
 efi_sources = \
-       drivers/watchdog/init_array_start.S \
+       drivers/watchdog/wdfuncs_start.c \
        $(efi_sources_watchdogs) \
-       drivers/watchdog/init_array_end.S \
+       drivers/watchdog/wdfuncs_end.c \
        env/syspart.c \
        env/fatvars.c \
        utils.c \
@@ -299,8 +299,8 @@ $(efi_solib): $(efi_objects)
        nm -D -u $@ | grep ' U ' && exit 1 || :
 
 $(efi_loadername): $(efi_solib)
-       $(AM_V_GEN) $(OBJCOPY) -j .text -j .sdata -j .data -j .dynamic \
-         -j .dynsym -j .rel* -j .init_array $(objcopy_format) $< $@
+       $(AM_V_GEN) $(OBJCOPY) -j .text -j .wdfuncs -j .sdata -j .data -j 
.dynamic \
+         -j .dynsym -j .rel*  $(objcopy_format) $< $@
 
 $(kernel_stub_solib): $(kernel_stub_objects)
        $(AM_V_CCLD)$(LD) $(efi_ldflags) $(kernel_stub_objects) \
diff --git a/drivers/watchdog/amdfch_wdt.c b/drivers/watchdog/amdfch_wdt.c
index a68939d..a071d85 100644
--- a/drivers/watchdog/amdfch_wdt.c
+++ b/drivers/watchdog/amdfch_wdt.c
@@ -137,9 +137,8 @@ static void amdfch_wdt_ping(VOID)
        writel(val, AMDFCH_WDT_CONTROL(watchdog.base));
 }
 
-static EFI_STATUS __attribute__((constructor))
-init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id,
-     UINTN timeout)
+static EFI_STATUS init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,
+                      UINT16 pci_device_id, UINTN timeout)
 {
        EFI_STATUS status;
        UINT8 pci_revision_id = 0;
@@ -185,3 +184,5 @@ init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 
pci_device_id,
 
        return EFI_SUCCESS;
 }
+
+WATCHDOG_REGISTER(init);
diff --git a/drivers/watchdog/atom-quark.c b/drivers/watchdog/atom-quark.c
index c0896f2..66c2a06 100644
--- a/drivers/watchdog/atom-quark.c
+++ b/drivers/watchdog/atom-quark.c
@@ -53,9 +53,8 @@ static void write_timer_regs(UINT32 wdt_base, UINT32 timer, 
UINT32 value)
        }
 }
 
-static EFI_STATUS __attribute__((constructor))
-init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id,
-     UINTN timeout)
+static EFI_STATUS init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,
+                      UINT16 pci_device_id, UINTN timeout)
 {
        UINT32 wdt_base;
        EFI_STATUS status;
@@ -91,3 +90,5 @@ init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 
pci_device_id,
 
        return status;
 }
+
+WATCHDOG_REGISTER(init);
diff --git a/drivers/watchdog/hpwdt.c b/drivers/watchdog/hpwdt.c
index 8c72f6c..9b31c40 100644
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -31,9 +31,8 @@
 #define PCI_GET_SUBSYS_VENDOR_ID(id)   (UINT16)(id)
 #define PCI_GET_SUBSYS_PRODUCT_ID(id)  (UINT16)((id) >> 16)
 
-static EFI_STATUS __attribute__((constructor))
-init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id,
-     UINTN timeout)
+static EFI_STATUS init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,
+                      UINT16 pci_device_id, UINTN timeout)
 {
        EFI_STATUS status;
        UINT16 reload;
@@ -86,3 +85,5 @@ init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 
pci_device_id,
 
        return EFI_SUCCESS;
 }
+
+WATCHDOG_REGISTER(init);
diff --git a/drivers/watchdog/i6300esb.c b/drivers/watchdog/i6300esb.c
index 4f50e69..69a64e0 100644
--- a/drivers/watchdog/i6300esb.c
+++ b/drivers/watchdog/i6300esb.c
@@ -44,9 +44,8 @@ static EFI_STATUS unlock_timer_regs(EFI_PCI_IO *pci_io)
            pci_io, EfiPciIoWidthUint32, 0, ESB_RELOAD_REG, 1, &value);
 }
 
-static EFI_STATUS __attribute__((constructor))
-init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id,
-     UINTN timeout)
+static EFI_STATUS init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,
+                      UINT16 pci_device_id, UINTN timeout)
 {
        EFI_STATUS status;
        UINT32 value;
@@ -88,3 +87,5 @@ init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 
pci_device_id,
 
        return status;
 }
+
+WATCHDOG_REGISTER(init);
diff --git a/drivers/watchdog/init_array_end.S 
b/drivers/watchdog/init_array_end.S
deleted file mode 100644
index d91969c..0000000
--- a/drivers/watchdog/init_array_end.S
+++ /dev/null
@@ -1,17 +0,0 @@
-/*
- * EFI Boot Guard
- *
- * Copyright (c) Siemens AG, 2017
- *
- * Authors:
- *  Jan Kiszka <jan.kis...@siemens.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2.  See
- * the COPYING file in the top-level directory.
- *
- * SPDX-License-Identifier:    GPL-2.0
- */
-
-.section .init_array
-.global init_array_end
-init_array_end:
diff --git a/drivers/watchdog/init_array_start.S 
b/drivers/watchdog/init_array_start.S
deleted file mode 100644
index 6d4b7cd..0000000
--- a/drivers/watchdog/init_array_start.S
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- * EFI Boot Guard
- *
- * Copyright (c) Siemens AG, 2017
- *
- * Authors:
- *  Jan Kiszka <jan.kis...@siemens.com>
- *
- * This work is licensed under the terms of the GNU GPL, version 2.  See
- * the COPYING file in the top-level directory.
- *
- * SPDX-License-Identifier:    GPL-2.0
- */
-
-.section .init_array
-/* Dummy value to avoid zero-size .init_array section linker error on
- * architectures having no watchdog drivers */
-.word 0x5343
-/* align to 4K so that signing tools will not stumble over this section */
-/* Note: Explicitly use .b(yte-)align as .align takes power of 2 on ARM */
-.balign 0x1000
-.global init_array_start
-init_array_start:
diff --git a/drivers/watchdog/ipc4x7e_wdt.c b/drivers/watchdog/ipc4x7e_wdt.c
index 046e3fe..13ef89f 100644
--- a/drivers/watchdog/ipc4x7e_wdt.c
+++ b/drivers/watchdog/ipc4x7e_wdt.c
@@ -74,9 +74,8 @@ static UINT64 get_sbreg_rba(VOID)
        return sbreg;
 }
 
-static EFI_STATUS __attribute__((constructor))
-init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id,
-     UINTN timeout)
+static EFI_STATUS init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,
+                      UINT16 pci_device_id, UINTN timeout)
 {
        UINTN pad_cfg;
        UINT8 val;
@@ -137,3 +136,5 @@ init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 
pci_device_id,
                return EFI_UNSUPPORTED;
        }
 }
+
+WATCHDOG_REGISTER(init);
diff --git a/drivers/watchdog/ipmi_wdt.c b/drivers/watchdog/ipmi_wdt.c
index 19e7813..abbd0f7 100644
--- a/drivers/watchdog/ipmi_wdt.c
+++ b/drivers/watchdog/ipmi_wdt.c
@@ -153,11 +153,10 @@ send_ipmi_cmd(UINT16 io_base, UINT8 cmd, UINT8 *data, 
UINTN datalen)
        return status;
 }
 
-static EFI_STATUS __attribute__((constructor))
-init(__attribute__((unused)) EFI_PCI_IO *pci_io,
-     __attribute__((unused)) UINT16 pci_vendor_id,
-     __attribute__((unused)) UINT16 pci_device_id,
-     UINTN timeout)
+static EFI_STATUS init(__attribute__((unused)) EFI_PCI_IO *pci_io,
+                      __attribute__((unused)) UINT16 pci_vendor_id,
+                      __attribute__((unused)) UINT16 pci_device_id,
+                      UINTN timeout)
 {
        SMBIOS_STRUCTURE_TABLE *smbios_table;
        SMBIOS_STRUCTURE_POINTER smbios_struct;
@@ -212,3 +211,5 @@ init(__attribute__((unused)) EFI_PCI_IO *pci_io,
        BS->CloseEvent(cmdtimer);
        return status;
 }
+
+WATCHDOG_REGISTER(init);
diff --git a/drivers/watchdog/itco.c b/drivers/watchdog/itco.c
index 037c203..9252014 100644
--- a/drivers/watchdog/itco.c
+++ b/drivers/watchdog/itco.c
@@ -273,9 +273,8 @@ static void update_no_reboot_flag_apl(const iTCO_info *itco)
        }
 }
 
-static EFI_STATUS __attribute__((constructor))
-init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id,
-     UINTN timeout)
+static EFI_STATUS init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,
+                      UINT16 pci_device_id, UINTN timeout)
 {
        UINT32 pm_base, tco_base, value;
        UINT8 itco_chip;
@@ -341,3 +340,5 @@ init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 
pci_device_id,
 
        return EFI_SUCCESS;
 }
+
+WATCHDOG_REGISTER(init);
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 8163099..df1da11 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -197,9 +197,8 @@ static int wdt_set_time(unsigned int timeout)
        return 0;
 }
 
-static EFI_STATUS __attribute__((constructor))
-init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id,
-     UINTN timeout)
+static EFI_STATUS init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id,
+                      UINT16 pci_device_id, UINTN timeout)
 {
        int chip, ret;
 
@@ -224,3 +223,5 @@ init(EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 
pci_device_id,
        }
        return EFI_UNSUPPORTED;
 }
+
+WATCHDOG_REGISTER(init);
diff --git a/drivers/watchdog/wdat.c b/drivers/watchdog/wdat.c
index 7ba0ab1..f5cb5e4 100644
--- a/drivers/watchdog/wdat.c
+++ b/drivers/watchdog/wdat.c
@@ -417,11 +417,10 @@ run_action(ACPI_TABLE_WDAT *wdat_table, UINT8 action, 
UINT32 param, UINT32 *retv
        return status;
 }
 
-static EFI_STATUS __attribute__((constructor))
-init(EFI_PCI_IO __attribute__((unused)) *pci_io,
-     UINT16 __attribute__((unused)) pci_vendor_id,
-     UINT16 __attribute__((unused)) pci_device_id,
-     UINTN timeout)
+static EFI_STATUS init(EFI_PCI_IO __attribute__((unused)) * pci_io,
+                      UINT16 __attribute__((unused)) pci_vendor_id,
+                      UINT16 __attribute__((unused)) pci_device_id,
+                      UINTN timeout)
 {
        ACPI_TABLE_WDAT *wdat_table;
        EFI_STATUS status;
@@ -485,3 +484,5 @@ init(EFI_PCI_IO __attribute__((unused)) *pci_io,
 
        return EFI_SUCCESS;
 }
+
+WATCHDOG_REGISTER(init);
diff --git a/drivers/watchdog/wdfuncs_end.c b/drivers/watchdog/wdfuncs_end.c
new file mode 100644
index 0000000..1a4317c
--- /dev/null
+++ b/drivers/watchdog/wdfuncs_end.c
@@ -0,0 +1,21 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2024
+ *
+ * Authors:
+ *  Christian Storm <christian.st...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier:    GPL-2.0
+ */
+
+#include <efi.h>
+#include "utils.h"
+
+/* Section .wdfunc's end address for watchdog probing function pointers
+ * preceding this marker, if any. */
+WATCHDOG_PROBE wdfuncs_end __attribute__((used, section(".wdfuncs"))) =
+       (WATCHDOG_PROBE)0x4353;
diff --git a/drivers/watchdog/wdfuncs_start.c b/drivers/watchdog/wdfuncs_start.c
new file mode 100644
index 0000000..6bbedf6
--- /dev/null
+++ b/drivers/watchdog/wdfuncs_start.c
@@ -0,0 +1,21 @@
+/*
+ * EFI Boot Guard
+ *
+ * Copyright (c) Siemens AG, 2024
+ *
+ * Authors:
+ *  Christian Storm <christian.st...@siemens.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * SPDX-License-Identifier:    GPL-2.0
+ */
+
+#include <efi.h>
+#include "utils.h"
+
+/* Section .wdfunc'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)0x5343;
diff --git a/include/utils.h b/include/utils.h
index bb9efda..6efd434 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -41,6 +41,13 @@ EFI_DEVICE_PATH *FileDevicePathFromConfig(EFI_HANDLE device,
 CHAR16 *GetBootMediumPath(CHAR16 *input);
 BOOLEAN IsOnBootMedium(EFI_DEVICE_PATH *dp);
 
+typedef EFI_STATUS (*WATCHDOG_PROBE)(EFI_PCI_IO *, UINT16, UINT16, UINTN);
+#define _CONCAT(prefix, func) prefix  ## func
+#define CONCAT(prefix, func) _CONCAT(prefix, func)
+#define WATCHDOG_REGISTER(_func)                                               
\
+       __attribute__((used, section(".wdfuncs"))) static WATCHDOG_PROBE       \
+               CONCAT(wdfuncs##_, _func) = (WATCHDOG_PROBE)_func;
+
 VOID PrintC(const UINT8 color, const CHAR16 *fmt, ...);
 #define ERROR(fmt, ...)                                                        
\
        do {                                                                   \
diff --git a/main.c b/main.c
index 44c0725..3fdc8fc 100644
--- a/main.c
+++ b/main.c
@@ -24,18 +24,17 @@
 #include "utils.h"
 #include "loader_interface.h"
 
-extern const unsigned long init_array_start[];
-extern const unsigned long init_array_end[];
+extern const unsigned long wdfuncs_start[];
+extern const unsigned long wdfuncs_end[];
 extern CHAR16 *boot_medium_path;
 
 #define PCI_GET_VENDOR_ID(id)  (UINT16)(id)
 #define PCI_GET_PRODUCT_ID(id) (UINT16)((id) >> 16)
 
-typedef EFI_STATUS (*WATCHDOG_PROBE)(EFI_PCI_IO *, UINT16, UINT16, UINTN);
 
 static EFI_STATUS probe_watchdogs(UINTN timeout)
 {
-       if (init_array_end - init_array_start == 0) {
+       if (wdfuncs_end - wdfuncs_start - 1 == 0) {
                if (timeout > 0) {
                        ERROR(L"No watchdog drivers registered, but timeout is 
non-zero.\n");
                        return EFI_UNSUPPORTED;
@@ -84,8 +83,8 @@ static EFI_STATUS probe_watchdogs(UINTN timeout)
                        continue;
                }
 
-               for (const unsigned long *entry = init_array_start;
-                    entry < init_array_end; entry++) {
+               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),
diff --git a/scripts/cppcheck.sh b/scripts/cppcheck.sh
index 2b5560f..08525a2 100755
--- a/scripts/cppcheck.sh
+++ b/scripts/cppcheck.sh
@@ -34,7 +34,7 @@ suppress+=" --suppress=unusedFunction:env/env_api_fat.c"
 # Some functions are used by linker wrapping
 suppress+=" --suppress=unusedFunction:tools/tests/test_probe_config_file.c"
 suppress+=" --suppress=unusedFunction:tools/tests/test_ebgenv_api.c"
-# False positive on init_array iteration
+# False positive on wdfuncs iteration
 suppress+=" --suppress=comparePointers:main.c"
 # False positive on constructors, first hit
 suppress+=" --suppress=unusedFunction:drivers/watchdog/amdfch_wdt.c"
-- 
2.43.2

-- 
You received this message because you are subscribed to the Google Groups "EFI 
Boot Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to efibootguard-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/efibootguard-dev/FF702BE3-1F50-4F38-A370-C90B6E9FC53C%40siemens.com.

Reply via email to