From 465663e5f6b350abdb18f0ab51ec8924e739bc78 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Thu, 2 Dec 2021 17:43:29 -0500 Subject: Add some missing PE image flag definitions This patch adds some missing definitions for PE header flags. We don't use all of them, but it's less confusing with the list matching the spec, except where the spec is obviously wrong. Signed-off-by: Peter Jones --- include/peimage.h | 54 +++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 7 deletions(-) (limited to 'include/peimage.h') diff --git a/include/peimage.h b/include/peimage.h index 3b3f01a7..4bcb940d 100644 --- a/include/peimage.h +++ b/include/peimage.h @@ -236,6 +236,24 @@ typedef struct { EFI_IMAGE_DATA_DIRECTORY DataDirectory[EFI_IMAGE_NUMBER_OF_DIRECTORY_ENTRIES]; } EFI_IMAGE_OPTIONAL_HEADER64; +#define EFI_IMAGE_DLLCHARACTERISTICS_RESERVED_0001 0x0001 +#define EFI_IMAGE_DLLCHARACTERISTICS_RESERVED_0002 0x0002 +#define EFI_IMAGE_DLLCHARACTERISTICS_RESERVED_0004 0x0004 +#define EFI_IMAGE_DLLCHARACTERISTICS_RESERVED_0008 0x0008 +#if 0 /* This is not in the PE spec. */ +#define EFI_IMAGE_DLLCHARACTERISTICS_RESERVED_0010 0x0010 +#endif +#define EFI_IMAGE_DLLCHARACTERISTICS_HIGH_ENTROPY_VA 0x0020 +#define EFI_IMAGE_DLLCHARACTERISTICS_DYNAMIC_BASE 0x0040 +#define EFI_IMAGE_DLLCHARACTERISTICS_FORCE_INTEGRITY 0x0080 +#define EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT 0x0100 +#define EFI_IMAGE_DLLCHARACTERISTICS_NO_ISOLATION 0x0200 +#define EFI_IMAGE_DLLCHARACTERISTICS_NO_SEH 0x0400 +#define EFI_IMAGE_DLLCHARACTERISTICS_NO_BIND 0x0800 +#define EFI_IMAGE_DLLCHARACTERISTICS_APPCONTAINER 0x1000 +#define EFI_IMAGE_DLLCHARACTERISTICS_WDM_DRIVER 0x2000 +#define EFI_IMAGE_DLLCHARACTERISTICS_GUARD_CF 0x4000 +#define EFI_IMAGE_DLLCHARACTERISTICS_TERMINAL_SERVER_AWARE 0x8000 /// /// @attention @@ -303,16 +321,31 @@ typedef struct { // // Section Flags Values // -#define EFI_IMAGE_SCN_TYPE_NO_PAD 0x00000008 ///< Reserved. +#define EFI_IMAGE_SCN_RESERVED_00000000 0x00000000 +#define EFI_IMAGE_SCN_RESERVED_00000001 0x00000001 +#define EFI_IMAGE_SCN_RESERVED_00000002 0x00000002 +#define EFI_IMAGE_SCN_RESERVED_00000004 0x00000004 +#define EFI_IMAGE_SCN_TYPE_NO_PAD 0x00000008 +#define EFI_IMAGE_SCN_RESERVED_00000010 0x00000010 #define EFI_IMAGE_SCN_CNT_CODE 0x00000020 #define EFI_IMAGE_SCN_CNT_INITIALIZED_DATA 0x00000040 #define EFI_IMAGE_SCN_CNT_UNINITIALIZED_DATA 0x00000080 - -#define EFI_IMAGE_SCN_LNK_OTHER 0x00000100 ///< Reserved. -#define EFI_IMAGE_SCN_LNK_INFO 0x00000200 ///< Section contains comments or some other type of information. -#define EFI_IMAGE_SCN_LNK_REMOVE 0x00000800 ///< Section contents will not become part of image. +#define EFI_IMAGE_SCN_LNK_OTHER 0x00000100 +#define EFI_IMAGE_SCN_LNK_INFO 0x00000200 +#define EFI_IMAGE_SCN_RESERVED_00000400 0x00000400 +#define EFI_IMAGE_SCN_LNK_REMOVE 0x00000800 #define EFI_IMAGE_SCN_LNK_COMDAT 0x00001000 - +#define EFI_IMAGE_SCN_RESERVED_00002000 0x00002000 +#define EFI_IMAGE_SCN_RESERVED_00004000 0x00004000 +#define EFI_IMAGE_SCN_GPREL 0x00008000 +/* + * PE 9.3 says both IMAGE_SCN_MEM_PURGEABLE and IMAGE_SCN_MEM_16BIT are + * 0x00020000, but I think it's wrong. --pjones + */ +#define EFI_IMAGE_SCN_MEM_PURGEABLE 0x00010000 // "Reserved for future use." +#define EFI_IMAGE_SCN_MEM_16BIT 0x00020000 // "Reserved for future use." +#define EFI_IMAGE_SCN_MEM_LOCKED 0x00040000 // "Reserved for future use." +#define EFI_IMAGE_SCN_MEM_PRELOAD 0x00080000 // "Reserved for future use." #define EFI_IMAGE_SCN_ALIGN_1BYTES 0x00100000 #define EFI_IMAGE_SCN_ALIGN_2BYTES 0x00200000 #define EFI_IMAGE_SCN_ALIGN_4BYTES 0x00300000 @@ -320,7 +353,14 @@ typedef struct { #define EFI_IMAGE_SCN_ALIGN_16BYTES 0x00500000 #define EFI_IMAGE_SCN_ALIGN_32BYTES 0x00600000 #define EFI_IMAGE_SCN_ALIGN_64BYTES 0x00700000 - +#define EFI_IMAGE_SCN_ALIGN_128BYTES 0x00800000 +#define EFI_IMAGE_SCN_ALIGN_256BYTES 0x00900000 +#define EFI_IMAGE_SCN_ALIGN_512BYTES 0x00a00000 +#define EFI_IMAGE_SCN_ALIGN_1024BYTES 0x00b00000 +#define EFI_IMAGE_SCN_ALIGN_2048BYTES 0x00c00000 +#define EFI_IMAGE_SCN_ALIGN_4096BYTES 0x00d00000 +#define EFI_IMAGE_SCN_ALIGN_8192BYTES 0x00e00000 +#define EFI_IMAGE_SCN_LNK_NRELOC_OVFL 0x01000000 #define EFI_IMAGE_SCN_MEM_DISCARDABLE 0x02000000 #define EFI_IMAGE_SCN_MEM_NOT_CACHED 0x04000000 #define EFI_IMAGE_SCN_MEM_NOT_PAGED 0x08000000 -- cgit v1.2.3 From f28833f7cbb3f536081b19c8a2cc6f709e772128 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 18 May 2022 14:54:06 -0400 Subject: peimage.h: make our signature macros force the type scan-build invoked clang in a way that complains about our SIGNATURE_XX() macro's sizes being used to assign to things that are that size in post-process-pe.c. This patch makes them cast the results to the appropriately sized type. Signed-off-by: Peter Jones --- include/peimage.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'include/peimage.h') diff --git a/include/peimage.h b/include/peimage.h index 4bcb940d..e97b29c4 100644 --- a/include/peimage.h +++ b/include/peimage.h @@ -17,10 +17,14 @@ #include "wincert.h" -#define SIGNATURE_16(A, B) ((A) | (B << 8)) -#define SIGNATURE_32(A, B, C, D) (SIGNATURE_16 (A, B) | (SIGNATURE_16 (C, D) << 16)) -#define SIGNATURE_64(A, B, C, D, E, F, G, H) \ - (SIGNATURE_32 (A, B, C, D) | ((UINT64) (SIGNATURE_32 (E, F, G, H)) << 32)) +#define SIGNATURE_16(A, B) \ + ((UINT16)(((UINT16)(A)) | (((UINT16)(B)) << ((UINT16)8)))) +#define SIGNATURE_32(A, B, C, D) \ + ((UINT32)(((UINT32)SIGNATURE_16(A, B)) | \ + (((UINT32)SIGNATURE_16(C, D)) << (UINT32)16))) +#define SIGNATURE_64(A, B, C, D, E, F, G, H) \ + ((UINT64)((UINT64)SIGNATURE_32(A, B, C, D) | \ + ((UINT64)(SIGNATURE_32(E, F, G, H)) << (UINT64)32))) #define ALIGN_VALUE(Value, Alignment) ((Value) + (((Alignment) - (Value)) & ((Alignment) - 1))) #define ALIGN_POINTER(Pointer, Alignment) ((VOID *) (ALIGN_VALUE ((UINTN)(Pointer), (Alignment)))) -- cgit v1.2.3 From e4f40ae862b5389c8cc7d4f938a34238421456a1 Mon Sep 17 00:00:00 2001 From: Nicholas Bishop Date: Tue, 20 Dec 2022 14:40:39 -0500 Subject: pe: Add IS_PAGE_ALIGNED macro This makes some checks in `get_mem_attrs` and `update_mem_attrs` clearer. Also add `test-pe-util.c` with a test for the new macro. The file is named that way instead of `test-pe.c` to avoid having to get `pe.c` building in the unit test environment. Signed-off-by: Nicholas Bishop --- include/peimage.h | 3 +++ pe.c | 4 ++-- test-pe-util.c | 30 ++++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 test-pe-util.c (limited to 'include/peimage.h') diff --git a/include/peimage.h b/include/peimage.h index e97b29c4..6eef1051 100644 --- a/include/peimage.h +++ b/include/peimage.h @@ -29,6 +29,9 @@ #define ALIGN_VALUE(Value, Alignment) ((Value) + (((Alignment) - (Value)) & ((Alignment) - 1))) #define ALIGN_POINTER(Pointer, Alignment) ((VOID *) (ALIGN_VALUE ((UINTN)(Pointer), (Alignment)))) +// Check if `val` is evenly aligned to the page size. +#define IS_PAGE_ALIGNED(val) (!((val) & EFI_PAGE_MASK)) + // // PE32+ Subsystem type for EFI images // diff --git a/pe.c b/pe.c index 5ad0914b..85b64c09 100644 --- a/pe.c +++ b/pe.c @@ -937,7 +937,7 @@ get_mem_attrs (uintptr_t addr, size_t size, uint64_t *attrs) if (EFI_ERROR(efi_status) || !proto) return efi_status; - if (physaddr & 0xfff || size & 0xfff || size == 0 || attrs == NULL) { + if (!IS_PAGE_ALIGNED(physaddr) || !IS_PAGE_ALIGNED(size) || size == 0 || attrs == NULL) { dprint(L"%a called on 0x%llx-0x%llx and attrs 0x%llx\n", __func__, (unsigned long long)physaddr, (unsigned long long)(physaddr+size-1), @@ -971,7 +971,7 @@ update_mem_attrs(uintptr_t addr, uint64_t size, (unsigned long long)addr, (unsigned long long)size, &before, efi_status); - if (physaddr & 0xfff || size & 0xfff || size == 0) { + if (!IS_PAGE_ALIGNED(physaddr) || !IS_PAGE_ALIGNED(size) || size == 0) { dprint(L"%a called on 0x%llx-0x%llx (size 0x%llx) +%a%a%a -%a%a%a\n", __func__, (unsigned long long)physaddr, (unsigned long long)(physaddr + size - 1), diff --git a/test-pe-util.c b/test-pe-util.c new file mode 100644 index 00000000..d5765488 --- /dev/null +++ b/test-pe-util.c @@ -0,0 +1,30 @@ +// SPDX-License-Identifier: BSD-2-Clause-Patent +/* + * test-pe-util.c - test PE utilities + */ + +#ifndef SHIM_UNIT_TEST +#define SHIM_UNIT_TEST +#endif +#include "shim.h" + +static int +test_is_page_aligned(void) +{ + assert_true_return(IS_PAGE_ALIGNED(0), -1, "\n"); + assert_false_return(IS_PAGE_ALIGNED(1), -1, "\n"); + assert_false_return(IS_PAGE_ALIGNED(4095), -1, "\n"); + assert_true_return(IS_PAGE_ALIGNED(4096), -1, "\n"); + assert_false_return(IS_PAGE_ALIGNED(4097), -1, "\n"); + + return 0; +} + +int +main(void) +{ + int status = 0; + test(test_is_page_aligned); + + return status; +} -- cgit v1.2.3 From 7cde2cc52f19f733de7855419d1c43a13a8d6c5f Mon Sep 17 00:00:00 2001 From: Dennis Tseng Date: Fri, 28 Apr 2023 10:47:33 +0800 Subject: post-process-pe: add tests to validate NX compliance This changes post-process-pe to give warnings, and optionally errors, if a shim binary is built with Section Alignment or characteristics are not compatible with NX, or if the EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT flag is not set and require_nx_compat is true. Co-authored-by: Peter Jones Co-authored-by: Kamil Aronowski Signed-off-by: Dennis Tseng --- include/peimage.h | 1 + post-process-pe.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 60 insertions(+), 1 deletion(-) (limited to 'include/peimage.h') diff --git a/include/peimage.h b/include/peimage.h index 6eef1051..4182a17b 100644 --- a/include/peimage.h +++ b/include/peimage.h @@ -824,6 +824,7 @@ typedef struct { EFI_IMAGE_DATA_DIRECTORY *RelocDir; EFI_IMAGE_DATA_DIRECTORY *SecDir; UINT64 NumberOfRvaAndSizes; + UINT16 DllCharacteristics; EFI_IMAGE_OPTIONAL_HEADER_UNION *PEHdr; } PE_COFF_LOADER_IMAGE_CONTEXT; diff --git a/post-process-pe.c b/post-process-pe.c index de8f4a38..008da93b 100644 --- a/post-process-pe.c +++ b/post-process-pe.c @@ -43,6 +43,7 @@ static int verbosity; }) static bool set_nx_compat = false; +static bool require_nx_compat = false; typedef uint8_t UINT8; typedef uint16_t UINT16; @@ -162,6 +163,7 @@ load_pe(const char *const file, void *const data, const size_t datasize, ctx->ImageSize = PEHdr->Pe32Plus.OptionalHeader.SizeOfImage; ctx->SectionAlignment = PEHdr->Pe32Plus.OptionalHeader.SectionAlignment; + ctx->DllCharacteristics = PEHdr->Pe32Plus.OptionalHeader.DllCharacteristics; FileAlignment = PEHdr->Pe32Plus.OptionalHeader.FileAlignment; OptHeaderSize = sizeof(EFI_IMAGE_OPTIONAL_HEADER64); } else { @@ -172,6 +174,7 @@ load_pe(const char *const file, void *const data, const size_t datasize, ctx->ImageSize = (UINT64)PEHdr->Pe32.OptionalHeader.SizeOfImage; ctx->SectionAlignment = PEHdr->Pe32.OptionalHeader.SectionAlignment; + ctx->DllCharacteristics = PEHdr->Pe32.OptionalHeader.DllCharacteristics; FileAlignment = PEHdr->Pe32.OptionalHeader.FileAlignment; OptHeaderSize = sizeof(EFI_IMAGE_OPTIONAL_HEADER32); } @@ -358,6 +361,50 @@ set_dll_characteristics(PE_COFF_LOADER_IMAGE_CONTEXT *ctx) } else { ctx->PEHdr->Pe32.OptionalHeader.DllCharacteristics = newflags; } + ctx->DllCharacteristics = newflags; +} + +static int +validate_nx_compat(PE_COFF_LOADER_IMAGE_CONTEXT *ctx) +{ + EFI_IMAGE_SECTION_HEADER *Section; + int i; + bool nx_compat_flag; + const int level = require_nx_compat ? ERROR : WARNING; + int ret = 0; + + nx_compat_flag = EFI_IMAGE_DLLCHARACTERISTICS_NX_COMPAT + & ctx->DllCharacteristics; + debug(NOISE, "NX-Compat-Flag: %s\n", nx_compat_flag ? "set" : "unset"); + if (!nx_compat_flag) { + debug(level, "NX Compatibility flag is not set\n"); + if (require_nx_compat) + ret = -1; + } + + debug(NOISE, "Section alignment is 0x%x, page size is 0x%x\n", + ctx->SectionAlignment, PAGE_SIZE); + if (ctx->SectionAlignment != PAGE_SIZE) { + debug(level, "Section alignment is not page aligned\n"); + if (require_nx_compat) + ret = -1; + } + + Section = ctx->FirstSection; + for (i=0, Section = ctx->FirstSection; i < ctx->NumberOfSections; i++, Section++) { + debug(NOISE, "Section %d has WRITE=%d and EXECUTE=%d\n", i, + (Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) ? 1 : 0, + (Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) ? 1 : 0); + + if ((Section->Characteristics & EFI_IMAGE_SCN_MEM_WRITE) && + (Section->Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE)) { + debug(level, "Section %d is writable and executable\n", i); + if (require_nx_compat) + ret = -1; + } + } + + return ret; } static void @@ -449,6 +496,10 @@ handle_one(char *f) set_dll_characteristics(&ctx); + rc = validate_nx_compat(&ctx); + if (rc < 0) + err(2, "NX compatibility check failed\n"); + fix_timestamp(&ctx); fix_checksum(&ctx, map, sz); @@ -483,6 +534,7 @@ static void __attribute__((__noreturn__)) usage(int status) fprintf(out, " -v Be more verbose\n"); fprintf(out, " -N Disable the NX compatibility flag\n"); fprintf(out, " -n Enable the NX compatibility flag\n"); + fprintf(out, " -x Error on NX incompatibility\n"); fprintf(out, " -h Print this help text and exit\n"); exit(status); @@ -510,11 +562,14 @@ int main(int argc, char **argv) {.name = "verbose", .val = 'v', }, + {.name = "error-nx-compat", + .val = 'x', + }, {.name = ""} }; int longindex = -1; - while ((i = getopt_long(argc, argv, "hNnqv", options, &longindex)) != -1) { + while ((i = getopt_long(argc, argv, "hNnqvx", options, &longindex)) != -1) { switch (i) { case 'h': case '?': @@ -532,6 +587,9 @@ int main(int argc, char **argv) case 'v': verbosity = MIN(verbosity + 1, MAX_VERBOSITY); break; + case 'x': + require_nx_compat = true; + break; } } -- cgit v1.2.3 From c5c5287b8a0da20b711601921dc34a9cfa2e39db Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Tue, 25 Feb 2025 09:00:20 -0500 Subject: peimage.h: minor whitespace fixes Signed-off-by: Peter Jones --- include/peimage.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'include/peimage.h') diff --git a/include/peimage.h b/include/peimage.h index 4182a17b..5d049686 100644 --- a/include/peimage.h +++ b/include/peimage.h @@ -144,12 +144,12 @@ typedef struct { /// /// @attention -/// EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC means PE32 and +/// EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC means PE32 and /// EFI_IMAGE_OPTIONAL_HEADER32 must be used. The data structures only vary /// after NT additional fields. /// #define EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC 0x10b - + /// /// Optional Header Standard Fields for PE32. /// @@ -195,7 +195,7 @@ typedef struct { /// /// @attention -/// EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC means PE32+ and +/// EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC means PE32+ and /// EFI_IMAGE_OPTIONAL_HEADER64 must be used. The data structures only vary /// after NT additional fields. /// @@ -465,7 +465,7 @@ typedef struct { #define EFI_IMAGE_COMDAT_SELECT_SAME_SIZE 3 #define EFI_IMAGE_COMDAT_SELECT_EXACT_MATCH 4 #define EFI_IMAGE_COMDAT_SELECT_ASSOCIATIVE 5 - + // // the following values only be referred in PeCoff, not defined in PECOFF. // @@ -500,9 +500,9 @@ typedef struct { #define EFI_IMAGE_REL_I386_SECREL 0x000B #define EFI_IMAGE_REL_I386_REL32 0x0014 ///< PC-relative 32-bit reference to the symbols virtual address. -// +// // x64 processor relocation types. -// +// #define IMAGE_REL_AMD64_ABSOLUTE 0x0000 #define IMAGE_REL_AMD64_ADDR64 0x0001 #define IMAGE_REL_AMD64_ADDR32 0x0002 -- cgit v1.2.3