uefi-capsule: Ensure SBAT metadata is added correctly

The current approach of adding SBAT metadata after linking is creating
an image that is badly formed in 2 ways:

 * The SBAT section's file offset and size are not a multiple of the
   file alignment.

 * The SBAT section has a virtual address of zero. EDK2 loads the header
   here, and so it gets rejected.

This changes the approach to match shim, where an object file is
created with a .sbat section and then the linker takes care of placing
the section at a more appropriate virtual address.

See https://github.com/vathpela/gnu-efi/pull/14 for the section addition.

plugins/uefi-capsule/efi/generate_binary.py

@@ -10,106 +10,41 @@
 import subprocess
 import sys
 import argparse
-import tempfile
-
-
-def _run_objcopy_sbat(args, tfd):
-    """ append SBAT metadata """
-    FWUPD_SUMMARY = "Firmware update daemon"
-    FWUPD_URL = "https://github.com/fwupd/fwupd"
-
-    with open(args.infile, "rb") as ifd:
-        tfd.write(ifd.read())
-
-    # not specified
-    if not args.sbat_distro_id:
-        return
-
-    with tempfile.NamedTemporaryFile() as sfd:
-
-        # spec
-        sfd.write(
-            "{0},{1},{2},{0},{1},{3}\n".format(
-                "sbat",
-                args.sbat_version,
-                "UEFI shim",
-                "https://github.com/rhboot/shim/blob/main/SBAT.md",
-            ).encode()
-        )
-
-        # fwupd
-        sfd.write(
-            "{0},{1},{2},{0},{3},{4}\n".format(
-                args.project_name,
-                args.sbat_generation,
-                "Firmware update daemon",
-                args.project_version,
-                FWUPD_URL,
-            ).encode()
-        )
-
-        # distro specifics, falling back to the project defaults
-        sfd.write(
-            "{0}-{1},{2},{3},{4},{5},{6}\n".format(
-                args.project_name,
-                args.sbat_distro_id,
-                args.sbat_distro_generation or args.sbat_generation,
-                args.sbat_distro_summary or FWUPD_SUMMARY,
-                args.sbat_distro_pkgname,
-                args.sbat_distro_version or args.project_version,
-                args.sbat_distro_url or FWUPD_URL,
-            ).encode()
-        )
-
-        # all written
-        sfd.seek(0)
-
-        # add a section to the object; use `objdump -s -j .sbat` to verify
-        argv = [
-            args.objcopy,
-            "--add-section",
-            ".sbat={}".format(sfd.name),
-            tfd.name,
-        ]
-        subprocess.run(argv, check=True)
 
 
 def _run_objcopy(args):
 
-    with tempfile.NamedTemporaryFile() as tfd:
+    argv = [
-        _run_objcopy_sbat(args, tfd)
+        args.objcopy,
+        "-j",
+        ".text",
+        "-j",
+        ".sbat",
+        "-j",
+        ".sdata",
+        "-j",
+        ".data",
+        "-j",
+        ".dynamic",
+        "-j",
+        ".dynsym",
+        "-j",
+        ".rel*",
+        args.infile,
+        args.outfile,
+    ]
 
-        argv = [
+    # aarch64 and arm32 don't have an EFI capable objcopy
-            args.objcopy,
+    # Use 'binary' instead, and add required symbols manually
-            "-j",
+    if args.arch in ["aarch64", "arm"]:
-            ".text",
+        argv.extend(["-O", "binary"])
-            "-j",
+    else:
-            ".sbat",
+        argv.extend(["--target", "efi-app-{}".format(args.arch)])
-            "-j",
+    try:
-            ".sdata",
+        subprocess.run(argv, check=True)
-            "-j",
+    except FileNotFoundError as e:
-            ".data",
+        print(str(e))
-            "-j",
+        sys.exit(1)
-            ".dynamic",
-            "-j",
-            ".dynsym",
-            "-j",
-            ".rel*",
-            tfd.name,
-            args.outfile,
-        ]
-
-        # aarch64 and arm32 don't have an EFI capable objcopy
-        # Use 'binary' instead, and add required symbols manually
-        if args.arch in ["aarch64", "arm"]:
-            argv.extend(["-O", "binary"])
-        else:
-            argv.extend(["--target", "efi-app-{}".format(args.arch)])
-        try:
-            subprocess.run(argv, check=True)
-        except FileNotFoundError as e:
-            print(str(e))
-            sys.exit(1)
 
 
 def _run_genpeimg(args):
@@ -143,57 +78,6 @@ if __name__ == "__main__":
         default="x86_64",
         help="EFI architecture",
     )
-    parser.add_argument(
-        "--project-name",
-        help="SBAT project name",
-    )
-    parser.add_argument(
-        "--project-version",
-        help="SBAT project version",
-    )
-    parser.add_argument(
-        "--sbat-version",
-        default=1,
-        type=int,
-        help="SBAT version",
-    )
-    parser.add_argument(
-        "--sbat-generation",
-        default=1,
-        type=int,
-        help="SBAT generation",
-    )
-    parser.add_argument(
-        "--sbat-distro-id",
-        default=None,
-        help="SBAT distribution ID"
-    )
-    parser.add_argument(
-        "--sbat-distro-generation",
-        default=None,
-        type=int,
-        help="SBAT distribution generation",
-    )
-    parser.add_argument(
-        "--sbat-distro-summary",
-        default=None,
-        help="SBAT distribution summary",
-    )
-    parser.add_argument(
-        "--sbat-distro-pkgname",
-        default=None,
-        help="SBAT distribution package name",
-    )
-    parser.add_argument(
-        "--sbat-distro-version",
-        default=None,
-        help="SBAT distribution version",
-    )
-    parser.add_argument(
-        "--sbat-distro-url",
-        default=None,
-        help="SBAT distribution URL",
-    )
     parser.add_argument(
         "infile",
         help="Input file",

plugins/uefi-capsule/efi/generate_sbat.py

@@ -0,0 +1,149 @@
+#!/usr/bin/python3
+#
+# Copyright (C) 2021 Javier Martinez Canillas <javierm@redhat.com>
+# Copyright (C) 2021 Richard Hughes <richard@hughsie.com>
+#
+# SPDX-License-Identifier: LGPL-2.1+
+#
+# pylint: disable=missing-docstring, invalid-name
+
+import subprocess
+import sys
+import argparse
+import tempfile
+
+
+def _generate_sbat(args):
+    """ append SBAT metadata """
+    FWUPD_SUMMARY = "Firmware update daemon"
+    FWUPD_URL = "https://github.com/fwupd/fwupd"
+
+    subprocess.run(
+        [args.cc, "-x", "c", "-c", "-o", args.outfile, "/dev/null"], check=True
+    )
+
+    # not specified
+    if not args.sbat_distro_id:
+        return
+
+    with tempfile.NamedTemporaryFile() as sfd:
+
+        # spec
+        sfd.write(
+            "{0},{1},{2},{0},{1},{3}\n".format(
+                "sbat",
+                args.sbat_version,
+                "UEFI shim",
+                "https://github.com/rhboot/shim/blob/main/SBAT.md",
+            ).encode()
+        )
+
+        # fwupd
+        sfd.write(
+            "{0},{1},{2},{0},{3},{4}\n".format(
+                args.project_name,
+                args.sbat_generation,
+                "Firmware update daemon",
+                args.project_version,
+                FWUPD_URL,
+            ).encode()
+        )
+
+        # distro specifics, falling back to the project defaults
+        sfd.write(
+            "{0}-{1},{2},{3},{4},{5},{6}\n".format(
+                args.project_name,
+                args.sbat_distro_id,
+                args.sbat_distro_generation or args.sbat_generation,
+                args.sbat_distro_summary or FWUPD_SUMMARY,
+                args.sbat_distro_pkgname,
+                args.sbat_distro_version or args.project_version,
+                args.sbat_distro_url or FWUPD_URL,
+            ).encode()
+        )
+
+        # all written
+        sfd.seek(0)
+
+        # add a section to the object; use `objdump -s -j .sbat` to verify
+        argv = [
+            args.objcopy,
+            "--add-section",
+            ".sbat={}".format(sfd.name),
+            args.outfile,
+        ]
+        subprocess.run(argv, check=True)
+
+
+if __name__ == "__main__":
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument(
+        "--cc",
+        default="gcc",
+        help="Compiler to use for generating sbat object",
+    )
+    parser.add_argument(
+        "--objcopy",
+        default="objcopy",
+        help="Binary file to use for objcopy",
+    )
+    parser.add_argument(
+        "--project-name",
+        help="SBAT project name",
+    )
+    parser.add_argument(
+        "--project-version",
+        help="SBAT project version",
+    )
+    parser.add_argument(
+        "--sbat-version",
+        default=1,
+        type=int,
+        help="SBAT version",
+    )
+    parser.add_argument(
+        "--sbat-generation",
+        default=1,
+        type=int,
+        help="SBAT generation",
+    )
+    parser.add_argument(
+        "--sbat-distro-id",
+        default=None,
+        help="SBAT distribution ID"
+    )
+    parser.add_argument(
+        "--sbat-distro-generation",
+        default=None,
+        type=int,
+        help="SBAT distribution generation",
+    )
+    parser.add_argument(
+        "--sbat-distro-summary",
+        default=None,
+        help="SBAT distribution summary",
+    )
+    parser.add_argument(
+        "--sbat-distro-pkgname",
+        default=None,
+        help="SBAT distribution package name",
+    )
+    parser.add_argument(
+        "--sbat-distro-version",
+        default=None,
+        help="SBAT distribution version",
+    )
+    parser.add_argument(
+        "--sbat-distro-url",
+        default=None,
+        help="SBAT distribution URL",
+    )
+    parser.add_argument(
+        "outfile",
+        help="Output file",
+    )
+    _args = parser.parse_args()
+    _generate_sbat(_args)
+
+    sys.exit(0)

plugins/uefi-capsule/efi/lds/elf_aarch64_efi.lds

@@ -0,0 +1,87 @@
+OUTPUT_FORMAT("elf64-littleaarch64", "elf64-littleaarch64", "elf64-littleaarch64")
+OUTPUT_ARCH(aarch64)
+ENTRY(_start)
+SECTIONS
+{
+  .text 0x0 : {
+    _text = .;
+    *(.text.head)
+    *(.text)
+    *(.text.*)
+    *(.gnu.linkonce.t.*)
+    *(.srodata)
+    *(.rodata*)
+    . = ALIGN(16);
+    _etext = .;
+  }
+
+  . = ALIGN(4096);
+  .dynamic  : { *(.dynamic) }
+
+  . = ALIGN(4096);
+  .note.gnu.build-id : {
+    *(.note.gnu.build-id)
+  }
+
+  . = ALIGN(4096);
+  .data.ident : {
+    *(.data.ident)
+  }
+
+  . = ALIGN(4096);
+  .data :
+  {
+   _data = .;
+   *(.sdata)
+   *(.data)
+   *(.data1)
+   *(.data.*)
+   *(.got.plt)
+   *(.got)
+
+   /* the EFI loader doesn't seem to like a .bss section, so we stick
+      it all into .data: */
+   . = ALIGN(16);
+   _bss = .;
+   *(.sbss)
+   *(.scommon)
+   *(.dynbss)
+   *(.bss)
+   *(COMMON)
+   . = ALIGN(16);
+   _bss_end = .;
+  }
+
+  . = ALIGN(4096);
+  .sbat :
+  {
+    _sbat = .;
+    *(.sbat)
+    *(.sbat.*)
+    _esbat = .;
+  }
+  . = ALIGN(4096);
+  .rela :
+  {
+    *(.rela.dyn)
+    *(.rela.plt)
+    *(.rela.got)
+    *(.rela.data)
+    *(.rela.data*)
+  }
+  _edata = .;
+  _data_size = . - _data;
+
+  . = ALIGN(4096);
+  .dynsym   : { *(.dynsym) }
+  . = ALIGN(4096);
+  .dynstr   : { *(.dynstr) }
+  . = ALIGN(4096);
+  /DISCARD/ :
+  {
+    *(.rel.reloc)
+    *(.eh_frame)
+    *(.note.GNU-stack)
+  }
+  .comment 0 : { *(.comment) }
+}

plugins/uefi-capsule/efi/lds/elf_ia32_efi.lds

@@ -0,0 +1,87 @@
+OUTPUT_FORMAT("elf32-i386", "elf32-i386", "elf32-i386")
+OUTPUT_ARCH(i386)
+ENTRY(_start)
+SECTIONS
+{
+  . = 0;
+  ImageBase = .;
+  .hash : { *(.hash) }	/* this MUST come first! */
+  . = ALIGN(4096);
+  .text :
+  {
+   _text = .;
+   *(.text)
+   *(.text.*)
+   *(.gnu.linkonce.t.*)
+   _etext = .;
+  }
+  .reloc :
+  {
+   *(.reloc)
+  }
+  . = ALIGN(4096);
+  .note.gnu.build-id : {
+    *(.note.gnu.build-id)
+  }
+  . = ALIGN(4096);
+  .data.ident : {
+    *(.data.ident)
+  }
+
+  . = ALIGN(4096);
+  .data :
+  {
+   _data = .;
+   *(.rodata*)
+   *(.data)
+   *(.data1)
+   *(.data.*)
+   *(.sdata)
+   *(.got.plt)
+   *(.got)
+   /* the EFI loader doesn't seem to like a .bss section, so we stick
+      it all into .data: */
+   *(.sbss)
+   *(.scommon)
+   *(.dynbss)
+   *(.bss)
+   *(COMMON)
+  }
+
+  . = ALIGN(4096);
+  .sbat :
+  {
+    _sbat = .;
+    *(.sbat)
+    *(.sbat.*)
+    _esbat = .;
+  }
+  . = ALIGN(4096);
+  .dynamic  : { *(.dynamic) }
+  . = ALIGN(4096);
+  .rel :
+  {
+    *(.rel.data)
+    *(.rel.data.*)
+    *(.rel.got)
+    *(.rel.stab)
+    *(.data.rel.ro.local)
+    *(.data.rel.local)
+    *(.data.rel.ro)
+    *(.data.rel*)
+  }
+  _edata = .;
+  _data_size = . - _data;
+  . = ALIGN(4096);
+  .dynsym   : { *(.dynsym) }
+  . = ALIGN(4096);
+  .dynstr   : { *(.dynstr) }
+  . = ALIGN(4096);
+  /DISCARD/ :
+  {
+    *(.rel.reloc)
+    *(.eh_frame)
+    *(.note.GNU-stack)
+  }
+  .comment 0 : { *(.comment) }
+}

plugins/uefi-capsule/efi/lds/elf_x86_64_efi.lds

@@ -0,0 +1,89 @@
+/* Same as elf_x86_64_fbsd_efi.lds, except for OUTPUT_FORMAT below - KEEP IN SYNC */
+OUTPUT_FORMAT("elf64-x86-64", "elf64-x86-64", "elf64-x86-64")
+OUTPUT_ARCH(i386:x86-64)
+ENTRY(_start)
+SECTIONS
+{
+  . = 0;
+  ImageBase = .;
+  .hash : { *(.hash) }	/* this MUST come first! */
+  . = ALIGN(4096);
+  .eh_frame :
+  {
+    *(.eh_frame)
+  }
+  . = ALIGN(4096);
+  .text :
+  {
+   _text = .;
+   *(.text)
+   _etext = .;
+  }
+  . = ALIGN(4096);
+  .reloc :
+  {
+   *(.reloc)
+  }
+  . = ALIGN(4096);
+  .note.gnu.build-id : {
+    *(.note.gnu.build-id)
+  }
+
+  . = ALIGN(4096);
+  .data.ident : {
+    *(.data.ident)
+  }
+
+  . = ALIGN(4096);
+  .data :
+  {
+   _data = .;
+   *(.rodata*)
+   *(.got.plt)
+   *(.got)
+   *(.data*)
+   *(.sdata)
+   /* the EFI loader doesn't seem to like a .bss section, so we stick
+      it all into .data: */
+   *(.sbss)
+   *(.scommon)
+   *(.dynbss)
+   *(.bss)
+   *(COMMON)
+   *(.rel.local)
+  }
+
+  . = ALIGN(4096);
+  .sbat :
+  {
+    _sbat = .;
+    *(.sbat)
+    *(.sbat.*)
+    _esbat = .;
+  }
+  . = ALIGN(4096);
+  .dynamic  : { *(.dynamic) }
+  . = ALIGN(4096);
+  .rela :
+  {
+    *(.rela.data*)
+    *(.rela.got*)
+    *(.rela.stab*)
+  }
+  _edata = .;
+  _data_size = . - _data;
+
+  . = ALIGN(4096);
+  .dynsym   : { *(.dynsym) }
+  . = ALIGN(4096);
+  .dynstr   : { *(.dynstr) }
+  . = ALIGN(4096);
+  .ignored.reloc :
+  {
+    *(.rela.reloc)
+    *(.eh_frame)
+    *(.note.GNU-stack)
+  }
+  .comment 0 : { *(.comment) }
+  .note.gnu.build-id : { *(.note.gnu.build-id) }
+}

plugins/uefi-capsule/efi/meson.build

@@ -61,8 +61,20 @@ else
   endif
 endif
 
+# is the system linker script new enough to know about SBAT?
+# i.e. gnu-efi with https://github.com/vathpela/gnu-efi/pull/14 has been installed
+efi_crtdir = efi_ldsdir
+if get_option('efi_sbat_distro_id') != ''
+  cmd = run_command('grep', '-q', 'sbat', join_paths(efi_ldsdir, arch_lds))
+  if cmd.returncode() != 0
+    warning('Cannot find SBAT section in @0@, using local copy'.format(join_paths(efi_ldsdir, arch_lds)))
+    efi_ldsdir = join_paths(meson.current_source_dir(), 'lds')
+  endif
+endif
+
 message('efi-libdir: "@0@"'.format(efi_libdir))
 message('efi-ldsdir: "@0@"'.format(efi_ldsdir))
+message('efi-crtdir: "@0@"'.format(efi_crtdir))
 message('efi-includedir: "@0@"'.format(efi_incdir))
 
 debugdir = join_paths (libdir, 'debug')
@@ -111,9 +123,9 @@ efi_ldflags = ['-T',
                '-Bsymbolic',
                '-nostdlib',
                '-znocombreloc',
-               '-L', efi_ldsdir,
+               '-L', efi_crtdir,
                '-L', efi_libdir,
-               join_paths(efi_ldsdir, arch_crt)]
+               join_paths(efi_crtdir, arch_crt)]
 if host_cpu == 'aarch64' or host_cpu == 'arm'
   # Aarch64 and ARM32 don't have an EFI capable objcopy. Use 'binary'
   # instead, and add required symbols manually.
@@ -147,8 +159,27 @@ o_file4 = custom_target('fwup-common.o',
                         command : [efi_cc, '-c', '@INPUT@', '-o', '@OUTPUT@']
                                   + compile_args)
 
+o_file5 = custom_target('fwup-sbat.o',
+                        output : 'fwup-sbat.o',
+                        command : [
+                          join_paths(meson.current_source_dir(), 'generate_sbat.py'),
+                          '@OUTPUT@',
+                          '--cc', efi_cc,
+                          '--objcopy', efi_objcopy,
+                          '--project-name', meson.project_name(),
+                          '--project-version', meson.project_version(),
+                          '--sbat-version', '1',
+                          '--sbat-generation', '@0@'.format(get_option('efi_sbat_fwupd_generation')),
+                          '--sbat-distro-id', get_option('efi_sbat_distro_id'),
+                          '--sbat-distro-generation', '0',
+                          '--sbat-distro-summary', get_option('efi_sbat_distro_summary'),
+                          '--sbat-distro-pkgname', get_option('efi_sbat_distro_pkgname'),
+                          '--sbat-distro-version', get_option('efi_sbat_distro_version'),
+                          '--sbat-distro-url', get_option('efi_sbat_distro_url'),
+                        ])
+
 so = custom_target('fwup.so',
-                   input : [o_file1, o_file2, o_file3, o_file4],
+                   input : [o_file1, o_file2, o_file3, o_file4, o_file5],
                    output : 'fwup.so',
                    command : [efi_ld, '-o', '@OUTPUT@'] +
                              efi_ldflags + ['@INPUT@'] +
@@ -167,16 +198,6 @@ app = custom_target(efi_name,
                       '@INPUT@', '@OUTPUT@',
                       '--arch', gnu_efi_arch,
                       '--objcopy', efi_objcopy,
-                      '--project-name', meson.project_name(),
-                      '--project-version', meson.project_version(),
-                      '--sbat-version', '1',
-                      '--sbat-generation', '@0@'.format(get_option('efi_sbat_fwupd_generation')),
-                      '--sbat-distro-id', get_option('efi_sbat_distro_id'),
-                      '--sbat-distro-generation', '0',
-                      '--sbat-distro-summary', get_option('efi_sbat_distro_summary'),
-                      '--sbat-distro-pkgname', get_option('efi_sbat_distro_pkgname'),
-                      '--sbat-distro-version', get_option('efi_sbat_distro_version'),
-                      '--sbat-distro-url', get_option('efi_sbat_distro_url'),
                     ],
                     install : true,
                     install_dir : efi_app_location)