Skip to content

Commit

Permalink
Post-process our PE to be sure.
Browse files Browse the repository at this point in the history
On some versions of binutils[0], including binutils-2.23.52.0.1-55.el7,
do not correctly initialize the data when computing the PE optional
header checksum.  Unfortunately, this means that any time you get a
build that reproduces correctly using the version of objcopy from those
versions, it's just a matter of luck.

This patch introduces a new utility program, post-process-pe, which does
some basic validation of the resulting binaries, and if necessary,
performs some minor repairs:

- sets the timestamp to 0
  - this was previously done with dd using constant offsets that aren't
    really safe.
- re-computes the checksum.

[0] I suspect, but have not yet fully verified, that this is
    accidentally fixed by the following upstream binutils commit:

    commit cf7a3c01d82abdf110ef85ab770e5997d8ac28ac
    Author: Alan Modra <amodra@gmail.com>
    Date:   Tue Dec 15 22:09:30 2020 +1030

      Lose some COFF/PE static vars, and peicode.h constify

      This patch tidies some COFF and PE code that unnecessarily used static
      variables to communicate between functions.

v2 - MAP_PRIVATE was totally wrong...

Signed-off-by: Peter Jones <pjones@redhat.com>
  • Loading branch information
vathpela committed May 25, 2021
1 parent 493bd94 commit 05875f3
Show file tree
Hide file tree
Showing 4 changed files with 510 additions and 9 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Make.local
/build*/
/certdb/
/cov-int/
/post-process-pe
/random.bin
/sbat.*.csv
/scan-results/
Expand Down
4 changes: 0 additions & 4 deletions Make.defaults
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ ifeq ($(ARCH),x86_64)
ARCH_SUFFIX ?= x64
ARCH_SUFFIX_UPPER ?= X64
ARCH_LDFLAGS ?=
TIMESTAMP_LOCATION := 136
endif
ifeq ($(ARCH),ia32)
ARCH_CFLAGS ?= -mno-mmx -mno-sse -mno-red-zone -nostdinc \
Expand All @@ -75,7 +74,6 @@ ifeq ($(ARCH),ia32)
ARCH_SUFFIX_UPPER ?= IA32
ARCH_LDFLAGS ?=
ARCH_CFLAGS ?= -m32
TIMESTAMP_LOCATION := 136
endif
ifeq ($(ARCH),aarch64)
ARCH_CFLAGS ?= -DMDE_CPU_AARCH64 -DPAGE_SIZE=4096 -mstrict-align
Expand All @@ -86,7 +84,6 @@ ifeq ($(ARCH),aarch64)
SUBSYSTEM := 0xa
ARCH_LDFLAGS += --defsym=EFI_SUBSYSTEM=$(SUBSYSTEM)
ARCH_CFLAGS ?=
TIMESTAMP_LOCATION := 72
endif
ifeq ($(ARCH),arm)
ARCH_CFLAGS ?= -DMDE_CPU_ARM -DPAGE_SIZE=4096 -mno-unaligned-access
Expand All @@ -96,7 +93,6 @@ ifeq ($(ARCH),arm)
FORMAT := -O binary
SUBSYSTEM := 0xa
ARCH_LDFLAGS += --defsym=EFI_SUBSYSTEM=$(SUBSYSTEM)
TIMESTAMP_LOCATION := 72
endif
DEFINES = -DDEFAULT_LOADER='L"$(DEFAULT_LOADER)"' \
Expand Down
13 changes: 8 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,10 @@ sbat_data.o : /dev/null
$@
$(foreach vs,$(VENDOR_SBATS),$(call add-vendor-sbat,$(vs),$@))

$(SHIMNAME) : $(SHIMSONAME)
$(MMNAME) : $(MMSONAME)
$(FBNAME) : $(FBSONAME)
$(SHIMNAME) : $(SHIMSONAME) post-process-pe
$(MMNAME) : $(MMSONAME) post-process-pe
$(FBNAME) : $(FBSONAME) post-process-pe
$(SHIMNAME) $(MMNAME) $(FBNAME) : | post-process-pe

LIBS = Cryptlib/libcryptlib.a \
Cryptlib/OpenSSL/libopenssl.a \
Expand Down Expand Up @@ -164,6 +165,9 @@ lib/lib.a: | $(TOPDIR)/lib/Makefile $(wildcard $(TOPDIR)/include/*.[ch])
mkdir -p lib
$(MAKE) VPATH=$(TOPDIR)/lib TOPDIR=$(TOPDIR) -C lib -f $(TOPDIR)/lib/Makefile

post-process-pe : $(TOPDIR)/post-process-pe.c
$(HOSTCC) -std=gnu11 -Og -g3 -Wall -Wextra -Wno-missing-field-initializers -Werror -o $@ $<

buildid : $(TOPDIR)/buildid.c
$(HOSTCC) -I/usr/include -Og -g3 -Wall -Werror -Wextra -o $@ $< -lelf

Expand Down Expand Up @@ -246,8 +250,7 @@ endif
-j .rela* -j .reloc -j .eh_frame \
-j .vendor_cert -j .sbat \
$(FORMAT) $< $@
# I am tired of wasting my time fighting binutils timestamp code.
dd conv=notrunc bs=1 count=4 seek=$(TIMESTAMP_LOCATION) if=/dev/zero of=$@
./post-process-pe -vv $@

ifneq ($(origin ENABLE_SHIM_HASH),undefined)
%.hash : %.efi
Expand Down
Loading

0 comments on commit 05875f3

Please sign in to comment.