[efi] Enable stack protection where possible

Enable -fstack-protector for EFI builds, where binary size is less
critical than for BIOS builds.

The stack cookie must be constructed immediately on entry, which
prohibits the use of any viable entropy source.  Construct a cookie by
XORing together various mildly random quantities to produce a value
that will at least not be identical on each run.

On detecting a stack corruption, attempt to call Exit() with an
appropriate error.  If that fails, then lock up the machine since
there is no other safe action that can be taken.

The old conditional check for support of -fno-stack-protector is
omitted since this flag dates back to GCC 4.1.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/Makefile.efi b/src/Makefile.efi
index 151b33186092dc07a5e5bcf626a27e53a40a1cad..10f3fe74be5b70fe724d881debcda1a1146e258d 100644 (file)
--- a/src/Makefile.efi
+++ b/src/Makefile.efi
@@ -1,5 +1,13 @@
 # -*- makefile -*- : Force emacs to use Makefile mode
 
+# Enable stack protection if available
+#
+SPG_TEST = $(CC) -fstack-protector-strong -mstack-protector-guard=global \
+                -x c -c /dev/null -o /dev/null >/dev/null 2>&1
+SPG_FLAGS := $(shell $(SPG_TEST) && $(ECHO) '-fstack-protector-strong ' \
+                                           '-mstack-protector-guard=global')
+CFLAGS += $(SPG_FLAGS)
+
 # The EFI linker script
 #
 LDSCRIPT       = scripts/efi.lds

diff --git a/src/Makefile.housekeeping b/src/Makefile.housekeeping
index 1dd147949e90bf670506dbd8ee0045d747d7b21a..66d6dd449d03d0f8c3095653ffc306dfb9e69078 100644 (file)
--- a/src/Makefile.housekeeping
+++ b/src/Makefile.housekeeping
@@ -146,17 +146,6 @@ define NEWLINE
 
 endef
 
-# Some widespread patched versions of gcc include -fstack-protector by
-# default, even when -ffreestanding is specified.  We therefore need
-# to disable -fstack-protector if the compiler supports it.
-#
-ifeq ($(CCTYPE),gcc)
-SP_TEST = $(CC) -fno-stack-protector -x c -c /dev/null \
-               -o /dev/null >/dev/null 2>&1
-SP_FLAGS := $(shell $(SP_TEST) && $(ECHO) '-fno-stack-protector')
-WORKAROUND_CFLAGS += $(SP_FLAGS)
-endif
-
 # gcc 4.4 generates .eh_frame sections by default, which distort the
 # output of "size".  Inhibit this.
 #
@@ -415,6 +404,13 @@ ifdef BIN
 incdirs :
        @$(ECHO) $(INCDIRS)
 
+# Inhibit -fstack-protector (which is implicitly enabled in some
+# patched gcc versions) unless explicitly mentioned in CFLAGS.
+#
+ifeq ($(findstring -fstack-protector,$(CFLAGS)),)
+CFLAGS         += -fno-stack-protector
+endif
+
 # Common flags
 #
 CFLAGS         += $(foreach INC,$(INCDIRS),-I$(INC))

diff --git a/src/include/ipxe/efi/efi.h b/src/include/ipxe/efi/efi.h
index 669e5364a1fd395df2ebf32cd5f0069f15bb1cbb..b8777ef47d4e00c804e2edd007a074c18434fe9e 100644 (file)
--- a/src/include/ipxe/efi/efi.h
+++ b/src/include/ipxe/efi/efi.h
@@ -65,6 +65,8 @@ typedef struct {} *EFI_HANDLE;
 
 #include <ipxe/tables.h>
 #include <ipxe/uuid.h>
+#include <ipxe/version.h>
+#include <ipxe/profile.h>
 
 /** An EFI protocol used by iPXE */
 struct efi_protocol {
@@ -272,6 +274,36 @@ extern void dbg_efi_protocols ( EFI_HANDLE handle );
 #define DBGCP_EFI_PROTOCOLS( ... )                             \
        DBGC_EFI_PROTOCOLS_IF ( PROFILE, ##__VA_ARGS__ )
 
+extern unsigned long __stack_chk_guard;
+extern unsigned long efi_stack_cookie ( EFI_HANDLE handle );
+extern void __stack_chk_fail ( void );
+
+/**
+ * Initialise stack cookie
+ *
+ * @v handle           Image handle
+ */
+static inline __attribute__ (( always_inline )) void
+efi_init_stack_guard ( EFI_HANDLE handle ) {
+
+       /* The calling function must not itself use stack protection,
+        * since the change in the stack guard value would trigger a
+        * false positive.
+        *
+        * There is unfortunately no way to annotate a function to
+        * exclude the use of stack protection.  We must therefore
+        * rely on correctly anticipating the compiler's decision on
+        * the use of stack protection.
+        *
+        * The calculation of the stack cookie value deliberately
+        * takes the address of a stack variable (to provide an
+        * additional source of entropy).  This operation would
+        * trigger the application of stack protection to the calling
+        * function, and so must be externalised.
+        */
+       __stack_chk_guard = efi_stack_cookie ( handle );
+}
+
 extern EFI_STATUS efi_init ( EFI_HANDLE image_handle,
                             EFI_SYSTEM_TABLE *systab );
 

diff --git a/src/interface/efi/efi_init.c b/src/interface/efi/efi_init.c
index ed9707f2c91f50354400f459825b78253bef708c..df46bb17bf37f4c67e454d89c96692a2abd7dec0 100644 (file)
--- a/src/interface/efi/efi_init.c
+++ b/src/interface/efi/efi_init.c
@@ -47,6 +47,16 @@ int efi_shutdown_in_progress;
 /** Event used to signal shutdown */
 static EFI_EVENT efi_shutdown_event;
 
+/** Stack cookie */
+unsigned long __stack_chk_guard;
+
+/** Exit function
+ *
+ * Cached to minimise external dependencies when a stack check
+ * failure is triggered.
+ */
+static EFI_EXIT efi_exit;
+
 /* Forward declarations */
 static EFI_STATUS EFIAPI efi_unload ( EFI_HANDLE image_handle );
 
@@ -87,6 +97,29 @@ static void * efi_find_table ( EFI_GUID *guid ) {
        return NULL;
 }
 
+/**
+ * Construct a stack cookie value
+ *
+ * @v handle           Image handle
+ * @ret cookie         Stack cookie
+ */
+__attribute__ (( noinline )) unsigned long
+efi_stack_cookie ( EFI_HANDLE handle ) {
+
+       /* There is no viable source of entropy available at this
+        * point.  Construct a value that is at least likely to vary
+        * between platforms and invocations.
+        *
+        * Ensure that the value contains a NUL byte, to act as a
+        * runaway string terminator.  Construct the NUL using a shift
+        * rather than a mask, to avoid losing valuable entropy in the
+        * low-order bits.
+        */
+       return ( ( ( ( unsigned long ) handle ) ^
+                  ( ( unsigned long ) &handle ) ^
+                  profile_timestamp() ^ build_id ) << 8 );
+}
+
 /**
  * Initialise EFI environment
  *
@@ -130,6 +163,9 @@ EFI_STATUS efi_init ( EFI_HANDLE image_handle,
        DBGC ( systab, "EFI handle %p systab %p\n", image_handle, systab );
        bs = systab->BootServices;
 
+       /* Store abort function pointer */
+       efi_exit = bs->Exit;
+
        /* Look up used protocols */
        for_each_table_entry ( prot, EFI_PROTOCOLS ) {
                if ( ( efirc = bs->LocateProtocol ( &prot->guid, NULL,
@@ -240,3 +276,29 @@ static EFI_STATUS EFIAPI efi_unload ( EFI_HANDLE image_handle __unused ) {
 
        return 0;
 }
+
+/**
+ * Abort on stack check failure
+ *
+ */
+__attribute__ (( noreturn )) void __stack_chk_fail ( void ) {
+       EFI_STATUS efirc;
+       int rc;
+
+       /* Report failure (when debugging) */
+       DBGC ( efi_systab, "EFI stack check failed (cookie %#lx); aborting\n",
+              __stack_chk_guard );
+
+       /* Attempt to exit cleanly with an error status */
+       if ( efi_exit ) {
+               efirc = efi_exit ( efi_image_handle, EFI_COMPROMISED_DATA,
+                                  0, NULL );
+               rc = -EEFI ( efirc );
+               DBGC ( efi_systab, "EFI stack check exit failed: %s\n",
+                      strerror ( rc ) );
+       }
+
+       /* If the exit fails for any reason, lock the system */
+       while ( 1 ) {}
+
+}

diff --git a/src/interface/efi/efidrvprefix.c b/src/interface/efi/efidrvprefix.c
index 4fbb19ff750cdeaad45dfb68c4637b594e109f20..f9bfa4c1d289a11d97b0db9e1f46cc3903a1a519 100644 (file)
--- a/src/interface/efi/efidrvprefix.c
+++ b/src/interface/efi/efidrvprefix.c
@@ -36,6 +36,9 @@ EFI_STATUS EFIAPI _efidrv_start ( EFI_HANDLE image_handle,
                                  EFI_SYSTEM_TABLE *systab ) {
        EFI_STATUS efirc;
 
+       /* Initialise stack cookie */
+       efi_init_stack_guard ( image_handle );
+
        /* Initialise EFI environment */
        if ( ( efirc = efi_init ( image_handle, systab ) ) != 0 )
                return efirc;

diff --git a/src/interface/efi/efiprefix.c b/src/interface/efi/efiprefix.c
index de3572c756a09d070c3d7ad60bf40a460ce5835d..2c5a5b31d78afe406af9b8a4a9bccdf160505e9e 100644 (file)
--- a/src/interface/efi/efiprefix.c
+++ b/src/interface/efi/efiprefix.c
@@ -41,6 +41,9 @@ EFI_STATUS EFIAPI _efi_start ( EFI_HANDLE image_handle,
        EFI_STATUS efirc;
        int rc;
 
+       /* Initialise stack cookie */
+       efi_init_stack_guard ( image_handle );
+
        /* Initialise EFI environment */
        if ( ( efirc = efi_init ( image_handle, systab ) ) != 0 )
                goto err_init;