Re: [PATCH v3] xen: Strip xen.efi by default

[Demi Marie Obenour <demiobenour@xxxxxxxxx>]

On 11/5/25 10:38, Frediano Ziglio wrote:
> From: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> 
> For xen.gz file we strip all symbols and have an additional
> xen-syms file version with all symbols.
> Make xen.efi more coherent stripping all symbols too.
> xen-syms.efi can be used for debugging.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx>
> ---
> Changes since v1:
> - avoid leaving target if some command fails.
> 
> Changes since v2:
> - do not convert type but retain PE format;
> = use xen-syms.efi for new file name, more consistent with ELF.
> ---
>  docs/misc/efi.pandoc  |  8 +-------
>  xen/Kconfig.debug     |  9 ++-------
>  xen/Makefile          | 19 -------------------
>  xen/arch/x86/Makefile |  9 ++++++---
>  4 files changed, 9 insertions(+), 36 deletions(-)
> 
> diff --git a/docs/misc/efi.pandoc b/docs/misc/efi.pandoc
> index 11c1ac3346..c66b18a66b 100644
> --- a/docs/misc/efi.pandoc
> +++ b/docs/misc/efi.pandoc
> @@ -20,13 +20,7 @@ Xen to load the configuration file even if multiboot 
> modules are found.
>  Once built, `make install-xen` will place the resulting binary directly into
>  the EFI boot partition, provided `EFI_VENDOR` is set in the environment (and
>  `EFI_MOUNTPOINT` is overridden as needed, should the default of `/boot/efi` 
> not
> -match your system). When built with debug info, the binary can be quite 
> large.
> -Setting `INSTALL_EFI_STRIP=1` in the environment will cause it to be stripped
> -of debug info in the process of installing. `INSTALL_EFI_STRIP` can also be 
> set
> -to any combination of options suitable to pass to `strip`, in case the 
> default
> -ones don't do. The xen.efi binary will also be installed in 
> `/usr/lib64/efi/`,
> -unless `EFI_DIR` is set in the environment to override this default. This
> -binary will not be stripped in the process.
> +match your system).
>  
>  The binary itself will require a configuration file (names with the `.efi`
>  extension of the binary's name replaced by `.cfg`, and - until an existing
> diff --git a/xen/Kconfig.debug b/xen/Kconfig.debug
> index d900d926c5..58ee10ee3e 100644
> --- a/xen/Kconfig.debug
> +++ b/xen/Kconfig.debug
> @@ -147,12 +147,7 @@ config DEBUG_INFO
>         Say Y here if you want to build Xen with debug information. This
>         information is needed e.g. for doing crash dump analysis of the
>         hypervisor via the "crash" tool.
> -       Saying Y will increase the size of the xen-syms and xen.efi
> -       binaries. In case the space on the EFI boot partition is rather
> -       limited, you may want to install a stripped variant of xen.efi in
> -       the EFI boot partition (look for "INSTALL_EFI_STRIP" in
> -       docs/misc/efi.pandoc for more information - when not using
> -       "make install-xen" for installing xen.efi, stripping needs to be
> -       done outside the Xen build environment).
> +       Saying Y will increase the size of the xen-syms and xen.efi.elf
> +       binaries.
>  
>  endmenu
> diff --git a/xen/Makefile b/xen/Makefile
> index ddcee8835c..605a26c181 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -493,22 +493,6 @@ endif
>  .PHONY: _build
>  _build: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
>  
> -# Strip
> -#
> -# INSTALL_EFI_STRIP, if defined, will cause xen.efi to be stripped before it
> -# is installed. If INSTALL_EFI_STRIP is '1', then the default option(s) below
> -# will be used. Otherwise, INSTALL_EFI_STRIP value will be used as the
> -# option(s) to the strip command.
> -ifdef INSTALL_EFI_STRIP
> -
> -ifeq ($(INSTALL_EFI_STRIP),1)
> -efi-strip-opt := --strip-debug --keep-file-symbols
> -else
> -efi-strip-opt := $(INSTALL_EFI_STRIP)
> -endif
> -
> -endif
> -
>  .PHONY: _install
>  _install: D=$(DESTDIR)
>  _install: T=$(notdir $(TARGET))
> @@ -535,9 +519,6 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
>               ln -sf $(T)-$(XEN_FULLVERSION).efi 
> $(D)$(EFI_DIR)/$(T)-$(XEN_VERSION).efi; \
>               ln -sf $(T)-$(XEN_FULLVERSION).efi $(D)$(EFI_DIR)/$(T).efi; \
>               if [ -n '$(EFI_MOUNTPOINT)' -a -n '$(EFI_VENDOR)' ]; then \
> -                     $(if $(efi-strip-opt), \
> -                          $(STRIP) $(efi-strip-opt) -p -o 
> $(TARGET).efi.stripped $(TARGET).efi && \
> -                          $(INSTALL_DATA) $(TARGET).efi.stripped 
> $(D)$(EFI_MOUNTPOINT)/efi/$(EFI_VENDOR)/$(T)-$(XEN_FULLVERSION).efi ||) \
>                       $(INSTALL_DATA) $(TARGET).efi 
> $(D)$(EFI_MOUNTPOINT)/efi/$(EFI_VENDOR)/$(T)-$(XEN_FULLVERSION).efi; \
>               elif [ "$(D)" = "$(patsubst $(shell cd $(XEN_ROOT) && 
> pwd)/%,%,$(D))" ]; then \
>                       echo 'EFI installation only partially done (EFI_VENDOR 
> not set)' >&2; \
> diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
> index 407571c510..c118ab7b7d 100644
> --- a/xen/arch/x86/Makefile
> +++ b/xen/arch/x86/Makefile
> @@ -228,14 +228,17 @@ endif
>       $(MAKE) $(build)=$(@D) .$(@F).1r.o .$(@F).1s.o
>       $(LD) $(call EFI_LDFLAGS,$(VIRT_BASE)) -T $(obj)/efi.lds $< \
>             $(dot-target).1r.o $(dot-target).1s.o $(orphan-handling-y) \
> -           $(note_file_option) -o $@
> -     $(NM) -pa --format=sysv $@ \
> +           $(note_file_option) -o $@.tmp
> +     $(NM) -pa --format=sysv $@.tmp \
>               | $(objtree)/tools/symbols --all-symbols --xensyms --sysv 
> --sort \
>               > $@.map
>  ifeq ($(CONFIG_DEBUG_INFO),y)
> -     $(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(OBJCOPY) -O 
> elf64-x86-64 $@ $@.elf
> +     $(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))cp -f \
> +        $@.tmp $(TARGET)-syms.efi
> +     $(if $(filter --strip-debug,$(EFI_LDFLAGS)),:$(space))$(STRIP) $@.tmp
>  endif
>       rm -f $(dot-target).[0-9]* $(@D)/..$(@F).[0-9]*
> +     mv -f $@.tmp $@
>  ifeq ($(CONFIG_XEN_IBT),y)
>       $(SHELL) $(srctree)/tools/check-endbr.sh $@
>  endif

Does this also strip the string table from xen.efi?  I'm concerned that
signing xen.efi for secure boot won't work if there is a string table.
In particular, it appears that EDK2 will miscalculate the file hash if
the string table is before the signature.  Moving the string table after
the signature invalidates the pointer to it.  The only exception is if
the string table is itself in a section, but I don't know if that is the
case.
-- 
Sincerely,
Demi Marie Obenour (she/her/hers)

Attachment: OpenPGP_0xB288B55FFF9C22C1.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature