[dhcp] Unregister ProxyDHCP and PXEBS settings on a successful DHCPACK

When a DHCP transaction does not result in the registration of a new
"proxydhcp" or "pxebs" settings block, any existing settings blocks
are currently left unaltered.

This can cause surprising behaviour.  For example: when chainloading
iPXE, the "proxydhcp" and "pxebs" settings blocks may be prepopulated
using cached values from the previous PXE bootloader.  If iPXE
performs a subsequent DHCP request, then the DHCP or ProxyDHCP servers
may choose to respond differently to iPXE.  The response may choose to
omit the ProxyDHCP or PXEBS stages, in which case no new "proxydhcp"
or "pxebs" settings blocks may be registered.  This will result in
iPXE using a combination of both old and new DHCP responses.

Fix by assuming that a successful DHCPACK effectively acquires
ownership of the "proxydhcp" and "pxebs" settings blocks, and that any
existing settings blocks should therefore be unregistered.

Reported-by: Henry Tung <htung@palantir.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

.github/workflows/build.yml

@@ -49,8 +49,7 @@ jobs:
           sudo apt update
           sudo apt install -y -o Acquire::Retries=50 \
                            mtools syslinux isolinux \
-                           libc6-dev-i386 valgrind \
-                           libgcc-s1:i386 libc6-dbg:i386
+                           libc6-dev-i386 libc6-dbg:i386 valgrind
       - name: Build (BIOS)
         run: |
           make -j 4 -C src

src/arch/x86/image/bzimage.c

@@ -355,10 +355,6 @@ static size_t bzimage_load_initrd ( struct image *image,
 	size_t offset;
 	size_t pad_len;
 
-	/* Skip hidden images */
-	if ( initrd->flags & IMAGE_HIDDEN )
-		return 0;
-
 	/* Create cpio header for non-prebuilt images */
 	offset = cpio_header ( initrd, &cpio );
 

src/arch/x86/image/multiboot.c

@@ -204,10 +204,6 @@ static int multiboot_add_modules ( struct image *image, physaddr_t start,
 			break;
 		}
 
-		/* Skip hidden images */
-		if ( module_image->flags & IMAGE_HIDDEN )
-			continue;
-
 		/* Page-align the module */
 		start = ( ( start + 0xfff ) & ~0xfff );
 

src/config/config.c

@@ -290,9 +290,6 @@ REQUIRE_OBJECT ( cert_cmd );
 #ifdef IMAGE_MEM_CMD
 REQUIRE_OBJECT ( image_mem_cmd );
 #endif
-#ifdef SHIM_CMD
-REQUIRE_OBJECT ( shim_cmd );
-#endif
 
 /*
  * Drag in miscellaneous objects

src/config/console.h

@@ -39,7 +39,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 //#define	CONSOLE_SYSLOG		/* Syslog console */
 //#define	CONSOLE_SYSLOGS		/* Encrypted syslog console */
 //#define	CONSOLE_VMWARE		/* VMware logfile console */
-#define	CONSOLE_DEBUGCON	/* Bochs/QEMU/KVM debug port console */
+//#define	CONSOLE_DEBUGCON	/* Bochs/QEMU/KVM debug port console */
 //#define	CONSOLE_INT13		/* INT13 disk log console */
 
 /*

src/config/defaults/efi.h

@@ -47,7 +47,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #define USB_BLOCK		/* USB block devices */
 
 #define	REBOOT_CMD		/* Reboot command */
-#define SHIM_CMD		/* EFI shim command */
 
 #if defined ( __i386__ ) || defined ( __x86_64__ )
 #define IOAPI_X86

src/config/general.h

@@ -160,7 +160,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 //#define CERT_CMD		/* Certificate management commands */
 //#define IMAGE_MEM_CMD		/* Read memory command */
 #define IMAGE_ARCHIVE_CMD	/* Archive image management commands */
-//#define SHIM_CMD		/* EFI shim command */
 
 /*
  * ROM-specific options

src/core/image.c

@@ -56,15 +56,8 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 /** List of registered images */
 struct list_head images = LIST_HEAD_INIT ( images );
 
-/** Image selected for execution */
-struct image_tag selected_image __image_tag = {
-	.name = "SELECTED",
-};
-
 /** Currently-executing image */
-struct image_tag current_image __image_tag = {
-	.name = "CURRENT",
-};
+struct image *current_image;
 
 /** Current image trust requirement */
 static int require_trusted_images = 0;
@@ -79,13 +72,8 @@ static int require_trusted_images_permanent = 0;
  */
 static void free_image ( struct refcnt *refcnt ) {
 	struct image *image = container_of ( refcnt, struct image, refcnt );
-	struct image_tag *tag;
 
 	DBGC ( image, "IMAGE %s freed\n", image->name );
-	for_each_table_entry ( tag, IMAGE_TAGS ) {
-		if ( tag->image == image )
-			tag->image = NULL;
-	}
 	free ( image->name );
 	free ( image->cmdline );
 	uri_put ( image->uri );
@@ -273,6 +261,12 @@ int register_image ( struct image *image ) {
 			return rc;
 	}
 
+	/* Avoid ending up with multiple "selected" images on
+	 * re-registration
+	 */
+	if ( image_find_selected() )
+		image->flags &= ~IMAGE_SELECTED;
+
 	/* Add to image list */
 	image_get ( image );
 	image->flags |= IMAGE_REGISTERED;
@@ -326,23 +320,6 @@ struct image * find_image ( const char *name ) {
 	return NULL;
 }
 
-/**
- * Find image by tag
- *
- * @v tag		Image tag
- * @ret image		Executable image, or NULL
- */
-struct image * find_image_tag ( struct image_tag *tag ) {
-	struct image *image;
-
-	for_each_image ( image ) {
-		if ( tag->image == image )
-			return image;
-	}
-
-	return NULL;
-}
-
 /**
  * Execute image
  *
@@ -369,13 +346,13 @@ int image_exec ( struct image *image ) {
 	if ( image->uri )
 		churi ( image->uri );
 
-	/* Set as currently running image */
-	saved_current_image = image_tag ( image, &current_image );
+	/* Preserve record of any currently-running image */
+	saved_current_image = current_image;
 
 	/* Take out a temporary reference to the image, so that it
 	 * does not get freed when temporarily unregistered.
 	 */
-	image_get ( image );
+	current_image = image_get ( image );
 
 	/* Check that this image can be executed */
 	if ( ! ( image->type && image->type->exec ) ) {
@@ -442,7 +419,7 @@ int image_exec ( struct image *image ) {
 	image_put ( image );
 
 	/* Restore previous currently-running image */
-	image_tag ( saved_current_image, &current_image );
+	current_image = saved_current_image;
 
 	/* Reset current working directory */
 	churi ( old_cwuri );
@@ -465,7 +442,7 @@ int image_exec ( struct image *image ) {
  * registered until the currently-executing image returns.
  */
 int image_replace ( struct image *replacement ) {
-	struct image *image = current_image.image;
+	struct image *image = current_image;
 	int rc;
 
 	/* Sanity check */
@@ -501,17 +478,37 @@ int image_replace ( struct image *replacement ) {
  * @ret rc		Return status code
  */
 int image_select ( struct image *image ) {
+	struct image *tmp;
+
+	/* Unselect all other images */
+	for_each_image ( tmp )
+		tmp->flags &= ~IMAGE_SELECTED;
 
 	/* Check that this image can be executed */
 	if ( ! ( image->type && image->type->exec ) )
 		return -ENOEXEC;
 
 	/* Mark image as selected */
-	image_tag ( image, &selected_image );
+	image->flags |= IMAGE_SELECTED;
 
 	return 0;
 }
 
+/**
+ * Find selected image
+ *
+ * @ret image		Executable image, or NULL
+ */
+struct image * image_find_selected ( void ) {
+	struct image *image;
+
+	for_each_image ( image ) {
+		if ( image->flags & IMAGE_SELECTED )
+			return image;
+	}
+	return NULL;
+}
+
 /**
  * Change image trust requirement
  *

src/drivers/net/intel.c

@@ -290,18 +290,6 @@ static int intel_reset ( struct intel_nic *intel ) {
 		       pba, readl ( intel->regs + INTEL_PBA ) );
 	}
 
-	/* The Intel I210's packet buffer size registers reset only on
-	 * power up.  If an operating system changes these but then
-	 * the computer recieves a reset signal without losing power,
-	 * the registers will stay the same (but be incompatible with
-	 * other register defaults), thus making the device unable to
-	 * pass traffic.
-	 */
-	if ( intel->flags & INTEL_PBSIZE_RST ) {
-		writel ( INTEL_RXPBS_I210, intel->regs + INTEL_RXPBS );
-		writel ( INTEL_TXPBS_I210, intel->regs + INTEL_TXPBS );
-	}
-
 	/* Always reset MAC.  Required to reset the TX and RX rings. */
 	writel ( ( ctrl | INTEL_CTRL_RST ), intel->regs + INTEL_CTRL );
 	mdelay ( INTEL_RESET_DELAY_MS );
@@ -1151,7 +1139,7 @@ static struct pci_device_id intel_nics[] = {
 	PCI_ROM ( 0x8086, 0x1525, "82567v-4", "82567V-4", 0 ),
 	PCI_ROM ( 0x8086, 0x1526, "82576-5", "82576", 0 ),
 	PCI_ROM ( 0x8086, 0x1527, "82580-f2", "82580 Fiber", 0 ),
-	PCI_ROM ( 0x8086, 0x1533, "i210", "I210", INTEL_PBSIZE_RST ),
+	PCI_ROM ( 0x8086, 0x1533, "i210", "I210", 0 ),
 	PCI_ROM ( 0x8086, 0x1539, "i211", "I211", 0 ),
 	PCI_ROM ( 0x8086, 0x153a, "i217lm", "I217-LM", INTEL_NO_PHY_RST ),
 	PCI_ROM ( 0x8086, 0x153b, "i217v", "I217-V", 0 ),
@@ -1159,7 +1147,7 @@ static struct pci_device_id intel_nics[] = {
 	PCI_ROM ( 0x8086, 0x155a, "i218lm", "I218-LM", INTEL_NO_PHY_RST ),
 	PCI_ROM ( 0x8086, 0x156f, "i219lm", "I219-LM", INTEL_I219 ),
 	PCI_ROM ( 0x8086, 0x1570, "i219v", "I219-V", INTEL_I219 ),
-	PCI_ROM ( 0x8086, 0x157b, "i210-2", "I210", INTEL_PBSIZE_RST ),
+	PCI_ROM ( 0x8086, 0x157b, "i210-2", "I210", 0 ),
 	PCI_ROM ( 0x8086, 0x15a0, "i218lm-2", "I218-LM", INTEL_NO_PHY_RST ),
 	PCI_ROM ( 0x8086, 0x15a1, "i218v-2", "I218-V", 0 ),
 	PCI_ROM ( 0x8086, 0x15a2, "i218lm-3", "I218-LM", INTEL_NO_PHY_RST ),

src/drivers/net/intel.h

@@ -138,10 +138,6 @@ struct intel_descriptor {
 /** Packet Buffer Size */
 #define INTEL_PBS 0x01008UL
 
-/** Receive packet buffer size */
-#define INTEL_RXPBS 0x02404UL
-#define INTEL_RXPBS_I210	0x000000a2UL	/**< I210 power-up default */
-
 /** Receive Descriptor register block */
 #define INTEL_RD 0x02800UL
 
@@ -158,10 +154,6 @@ struct intel_descriptor {
 /** Receive buffer length */
 #define INTEL_RX_MAX_LEN 2048
 
-/** Transmit packet buffer size */
-#define INTEL_TXPBS 0x03404UL
-#define INTEL_TXPBS_I210	0x04000014UL	/**< I210 power-up default */
-
 /** Transmit Descriptor register block */
 #define INTEL_TD 0x03800UL
 
@@ -327,8 +319,6 @@ enum intel_flags {
 	INTEL_NO_ASDE = 0x0008,
 	/** Reset may cause a complete device hang */
 	INTEL_RST_HANG = 0x0010,
-	/** PBSIZE registers must be explicitly reset */
-	INTEL_PBSIZE_RST = 0x0020,
 };
 
 /** The i219 has a seriously broken reset mechanism */

src/hci/commands/image_cmd.c

@@ -129,7 +129,7 @@ static int imgsingle_exec ( int argc, char **argv,
 					    &image ) ) != 0 )
 			goto err_acquire;
 	} else {
-		image = find_image_tag ( &selected_image );
+		image = image_find_selected();
 		if ( ! image ) {
 			printf ( "No image selected\n" );
 			goto err_acquire;

src/hci/commands/shim_cmd.c

@@ -1,117 +0,0 @@
-/*
- * Copyright (C) 2023 Michael Brown <mbrown@fensystems.co.uk>.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of the
- * License, or any later version.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
- *
- * You can also choose to distribute this program under the terms of
- * the Unmodified Binary Distribution Licence (as given in the file
- * COPYING.UBDL), provided that you have satisfied its requirements.
- */
-
-FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
-
-#include <getopt.h>
-#include <ipxe/command.h>
-#include <ipxe/parseopt.h>
-#include <ipxe/efi/efi_image.h>
-#include <usr/imgmgmt.h>
-#include <usr/shimmgmt.h>
-
-/** @file
- *
- * EFI shim command
- *
- */
-
-/** "shim" options */
-struct shim_options {
-	/** Download timeout */
-	unsigned long timeout;
-	/** Require third party loader */
-	int require_loader;
-	/** Allow PXE base code protocol */
-	int allow_pxe;
-	/** Allow SBAT variable access */
-	int allow_sbat;
-};
-
-/** "shim" option list */
-static struct option_descriptor shim_opts[] = {
-	OPTION_DESC ( "timeout", 't', required_argument,
-		      struct shim_options, timeout, parse_timeout ),
-	OPTION_DESC ( "require-loader", 'l', no_argument,
-		      struct shim_options, require_loader, parse_flag ),
-	OPTION_DESC ( "allow-pxe", 'p', no_argument,
-		      struct shim_options, allow_pxe, parse_flag ),
-	OPTION_DESC ( "allow-sbat", 's', no_argument,
-		      struct shim_options, allow_sbat, parse_flag ),
-};
-
-/** "shim" command descriptor */
-static struct command_descriptor shim_cmd =
-	COMMAND_DESC ( struct shim_options, shim_opts, 0, 1, NULL );
-
-/**
- * The "shim" command
- *
- * @v argc		Argument count
- * @v argv		Argument list
- * @ret rc		Return status code
- */
-static int shim_exec ( int argc, char **argv ) {
-	struct shim_options opts;
-	struct image *image = NULL;
-	struct image *kernel;
-	char *name_uri;
-	int download;
-	int rc;
-
-	/* Parse options */
-	if ( ( rc = parse_options ( argc, argv, &shim_cmd, &opts ) ) != 0 )
-		goto err_parse;
-
-	/* Decide whether or not to download images */
-	kernel = find_image_tag ( &selected_image );
-	download = ( ! ( kernel && efi_can_load ( kernel ) ) );
-
-	/* Parse name/URI string */
-	name_uri = argv[optind];
-
-	/* Acquire image, if applicable */
-	if ( download && name_uri &&
-	     ( ( rc = imgacquire ( name_uri, opts.timeout,
-				   &image ) ) != 0 ) ) {
-		goto err_image;
-	}
-
-	/* (Un)register as shim */
-	if ( ( rc = shim ( image, opts.require_loader, opts.allow_pxe,
-			   opts.allow_sbat ) ) != 0 )
-		goto err_shim;
-
- err_shim:
- err_image:
- err_parse:
-	return rc;
-}
-
-/** Shim commands */
-struct command shim_commands[] __command = {
-	{
-		.name = "shim",
-		.exec = shim_exec,
-	},
-};

src/image/efi_image.c

@@ -31,8 +31,6 @@ FILE_LICENCE ( GPL2_OR_LATER );
 #include <ipxe/efi/efi_wrap.h>
 #include <ipxe/efi/efi_pxe.h>
 #include <ipxe/efi/efi_driver.h>
-#include <ipxe/efi/efi_image.h>
-#include <ipxe/efi/efi_shim.h>
 #include <ipxe/image.h>
 #include <ipxe/init.h>
 #include <ipxe/features.h>
@@ -111,14 +109,18 @@ efi_image_path ( struct image *image, EFI_DEVICE_PATH_PROTOCOL *parent ) {
  */
 static wchar_t * efi_image_cmdline ( struct image *image ) {
 	wchar_t *cmdline;
+	size_t len;
 
-	/* Allocate and construct command line */
-	if ( efi_asprintf ( &cmdline, "%s%s%s", image->name,
-			    ( image->cmdline ? " " : "" ),
-			    ( image->cmdline ? image->cmdline : "" ) ) < 0 ) {
+	len = ( strlen ( image->name ) +
+		( image->cmdline ?
+		  ( 1 /* " " */ + strlen ( image->cmdline ) ) : 0 ) );
+	cmdline = zalloc ( ( len + 1 /* NUL */ ) * sizeof ( wchar_t ) );
+	if ( ! cmdline )
 		return NULL;
-	}
-
+	efi_snprintf ( cmdline, ( len + 1 /* NUL */ ), "%s%s%s",
+		       image->name,
+		       ( image->cmdline ? " " : "" ),
+		       ( image->cmdline ? image->cmdline : "" ) );
 	return cmdline;
 }
 
@@ -136,65 +138,47 @@ static int efi_image_exec ( struct image *image ) {
 		EFI_LOADED_IMAGE_PROTOCOL *image;
 		void *interface;
 	} loaded;
-	struct image *shim;
-	struct image *exec;
 	EFI_HANDLE handle;
 	EFI_MEMORY_TYPE type;
 	wchar_t *cmdline;
-	unsigned int toggle;
 	EFI_STATUS efirc;
 	int rc;
 
 	/* Find an appropriate device handle to use */
 	snpdev = last_opened_snpdev();
 	if ( ! snpdev ) {
-		DBGC ( image, "EFIIMAGE %s could not identify SNP device\n",
-		       image->name );
+		DBGC ( image, "EFIIMAGE %p could not identify SNP device\n",
+		       image );
 		rc = -ENODEV;
 		goto err_no_snpdev;
 	}
 
-	/* Use shim instead of directly executing image if applicable */
-	shim = ( efi_can_load ( image ) ?
-		 NULL : find_image_tag ( &efi_shim ) );
-	exec = ( shim ? shim : image );
-	if ( shim ) {
-		DBGC ( image, "EFIIMAGE %s executing via %s\n",
-		       image->name, shim->name );
-	}
-
-	/* Re-register as a hidden image to allow for access via file I/O */
-	toggle = ( ~image->flags & IMAGE_HIDDEN );
-	image->flags |= IMAGE_HIDDEN;
-	if ( ( rc = register_image ( image ) ) != 0 )
-		goto err_register_image;
-
 	/* Install file I/O protocols */
 	if ( ( rc = efi_file_install ( snpdev->handle ) ) != 0 ) {
-		DBGC ( image, "EFIIMAGE %s could not install file protocol: "
-		       "%s\n", image->name, strerror ( rc ) );
+		DBGC ( image, "EFIIMAGE %p could not install file protocol: "
+		       "%s\n", image, strerror ( rc ) );
 		goto err_file_install;
 	}
 
 	/* Install PXE base code protocol */
 	if ( ( rc = efi_pxe_install ( snpdev->handle, snpdev->netdev ) ) != 0 ){
-		DBGC ( image, "EFIIMAGE %s could not install PXE protocol: "
-		       "%s\n", image->name, strerror ( rc ) );
+		DBGC ( image, "EFIIMAGE %p could not install PXE protocol: "
+		       "%s\n", image, strerror ( rc ) );
 		goto err_pxe_install;
 	}
 
 	/* Install iPXE download protocol */
 	if ( ( rc = efi_download_install ( snpdev->handle ) ) != 0 ) {
-		DBGC ( image, "EFIIMAGE %s could not install iPXE download "
-		       "protocol: %s\n", image->name, strerror ( rc ) );
+		DBGC ( image, "EFIIMAGE %p could not install iPXE download "
+		       "protocol: %s\n", image, strerror ( rc ) );
 		goto err_download_install;
 	}
 
 	/* Create device path for image */
-	path = efi_image_path ( exec, snpdev->path );
+	path = efi_image_path ( image, snpdev->path );
 	if ( ! path ) {
-		DBGC ( image, "EFIIMAGE %s could not create device path\n",
-		       image->name );
+		DBGC ( image, "EFIIMAGE %p could not create device path\n",
+		       image );
 		rc = -ENOMEM;
 		goto err_image_path;
 	}
@@ -202,30 +186,21 @@ static int efi_image_exec ( struct image *image ) {
 	/* Create command line for image */
 	cmdline = efi_image_cmdline ( image );
 	if ( ! cmdline ) {
-		DBGC ( image, "EFIIMAGE %s could not create command line\n",
-		       image->name );
+		DBGC ( image, "EFIIMAGE %p could not create command line\n",
+		       image );
 		rc = -ENOMEM;
 		goto err_cmdline;
 	}
 
-	/* Install shim special handling if applicable */
-	if ( shim &&
-	     ( ( rc = efi_shim_install ( shim, snpdev->handle,
-					 &cmdline ) ) != 0 ) ){
-		DBGC ( image, "EFIIMAGE %s could not install shim handling: "
-		       "%s\n", image->name, strerror ( rc ) );
-		goto err_shim_install;
-	}
-
 	/* Attempt loading image */
 	handle = NULL;
 	if ( ( efirc = bs->LoadImage ( FALSE, efi_image_handle, path,
-				       user_to_virt ( exec->data, 0 ),
-				       exec->len, &handle ) ) != 0 ) {
+				       user_to_virt ( image->data, 0 ),
+				       image->len, &handle ) ) != 0 ) {
 		/* Not an EFI image */
 		rc = -EEFI_LOAD ( efirc );
-		DBGC ( image, "EFIIMAGE %s could not load: %s\n",
-		       image->name, strerror ( rc ) );
+		DBGC ( image, "EFIIMAGE %p could not load: %s\n",
+		       image, strerror ( rc ) );
 		if ( efirc == EFI_SECURITY_VIOLATION ) {
 			goto err_load_image_security_violation;
 		} else {
@@ -245,8 +220,8 @@ static int efi_image_exec ( struct image *image ) {
 
 	/* Some EFI 1.10 implementations seem not to fill in DeviceHandle */
 	if ( loaded.image->DeviceHandle == NULL ) {
-		DBGC ( image, "EFIIMAGE %s filling in missing DeviceHandle\n",
-		       image->name );
+		DBGC ( image, "EFIIMAGE %p filling in missing DeviceHandle\n",
+		       image );
 		loaded.image->DeviceHandle = snpdev->handle;
 	}
 
@@ -276,14 +251,14 @@ static int efi_image_exec ( struct image *image ) {
 	/* Start the image */
 	if ( ( efirc = bs->StartImage ( handle, NULL, NULL ) ) != 0 ) {
 		rc = -EEFI_START ( efirc );
-		DBGC ( image, "EFIIMAGE %s could not start (or returned with "
-		       "error): %s\n", image->name, strerror ( rc ) );
+		DBGC ( image, "EFIIMAGE %p could not start (or returned with "
+		       "error): %s\n", image, strerror ( rc ) );
 		goto err_start_image;
 	}
 
 	/* If image was a driver, connect it up to anything available */
 	if ( type == EfiBootServicesCode ) {
-		DBGC ( image, "EFIIMAGE %s connecting drivers\n", image->name );
+		DBGC ( image, "EFIIMAGE %p connecting drivers\n", image );
 		efi_driver_reconnect_all();
 	}
 
@@ -311,9 +286,6 @@ static int efi_image_exec ( struct image *image ) {
 	if ( rc != 0 )
 		bs->UnloadImage ( handle );
  err_load_image:
-	if ( shim )
-		efi_shim_uninstall();
- err_shim_install:
 	free ( cmdline );
  err_cmdline:
 	free ( path );
@@ -324,9 +296,6 @@ static int efi_image_exec ( struct image *image ) {
  err_pxe_install:
 	efi_file_uninstall ( snpdev->handle );
  err_file_install:
-	unregister_image ( image );
- err_register_image:
-	image->flags ^= toggle;
  err_no_snpdev:
 	return rc;
 }
@@ -355,8 +324,8 @@ static int efi_image_probe ( struct image *image ) {
 				       image->len, &handle ) ) != 0 ) {
 		/* Not an EFI image */
 		rc = -EEFI_LOAD ( efirc );
-		DBGC ( image, "EFIIMAGE %s could not load: %s\n",
-		       image->name, strerror ( rc ) );
+		DBGC ( image, "EFIIMAGE %p could not load: %s\n",
+		       image, strerror ( rc ) );
 		if ( efirc == EFI_SECURITY_VIOLATION ) {
 			goto err_load_image_security_violation;
 		} else {
@@ -377,75 +346,9 @@ static int efi_image_probe ( struct image *image ) {
 	return rc;
 }
 
-/**
- * Probe EFI PE image
- *
- * @v image		EFI file
- * @ret rc		Return status code
- *
- * The extremely broken UEFI Secure Boot model provides no way for us
- * to unambiguously determine that a valid EFI executable image was
- * rejected by LoadImage() because it failed signature verification.
- * We must therefore use heuristics to guess whether not an image that
- * was rejected by LoadImage() could still be loaded via a separate PE
- * loader such as the UEFI shim.
- */
-static int efi_pe_image_probe ( struct image *image ) {
-	const UINT16 magic = ( ( sizeof ( UINTN ) == sizeof ( uint32_t ) ) ?
-			       EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC :
-			       EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC );
-	union {
-		EFI_IMAGE_DOS_HEADER dos;
-		EFI_IMAGE_OPTIONAL_HEADER_UNION pe;
-	} u;
-
-	/* Check for existence of DOS header */
-	if ( image->len < sizeof ( u.dos ) ) {
-		DBGC ( image, "EFIIMAGE %s too short for DOS header\n",
-		       image->name );
-		return -ENOEXEC;
-	}
-	copy_from_user ( &u.dos, image->data, 0, sizeof ( u.dos ) );
-	if ( u.dos.e_magic != EFI_IMAGE_DOS_SIGNATURE ) {
-		DBGC ( image, "EFIIMAGE %s missing MZ signature\n",
-		       image->name );
-		return -ENOEXEC;
-	}
-
-	/* Check for existence of PE header */
-	if ( ( image->len < u.dos.e_lfanew ) ||
-	     ( ( image->len - u.dos.e_lfanew ) < sizeof ( u.pe ) ) ) {
-		DBGC ( image, "EFIIMAGE %s too short for PE header\n",
-		       image->name );
-		return -ENOEXEC;
-	}
-	copy_from_user ( &u.pe, image->data, u.dos.e_lfanew, sizeof ( u.pe ) );
-	if ( u.pe.Pe32.Signature != EFI_IMAGE_NT_SIGNATURE ) {
-		DBGC ( image, "EFIIMAGE %s missing PE signature\n",
-		       image->name );
-		return -ENOEXEC;
-	}
-
-	/* Check PE header magic */
-	if ( u.pe.Pe32.OptionalHeader.Magic != magic ) {
-		DBGC ( image, "EFIIMAGE %s incorrect magic %04x\n",
-		       image->name, u.pe.Pe32.OptionalHeader.Magic );
-		return -ENOEXEC;
-	}
-
-	return 0;
-}
-
-/** EFI image types */
-struct image_type efi_image_type[] __image_type ( PROBE_NORMAL ) = {
-	{
-		.name = "EFI",
-		.probe = efi_image_probe,
-		.exec = efi_image_exec,
-	},
-	{
-		.name = "EFIPE",
-		.probe = efi_pe_image_probe,
-		.exec = efi_image_exec,
-	},
+/** EFI image type */
+struct image_type efi_image_type __image_type ( PROBE_NORMAL ) = {
+	.name = "EFI",
+	.probe = efi_image_probe,
+	.exec = efi_image_exec,
 };

src/image/script.c

@@ -311,7 +311,6 @@ static int terminate_on_label_found ( int rc ) {
  * @ret rc		Return status code
  */
 static int goto_exec ( int argc, char **argv ) {
-	struct image *image = current_image.image;
 	struct goto_options opts;
 	size_t saved_offset;
 	int rc;
@@ -321,7 +320,7 @@ static int goto_exec ( int argc, char **argv ) {
 		return rc;
 
 	/* Sanity check */
-	if ( ! image ) {
+	if ( ! current_image ) {
 		rc = -ENOTTY;
 		printf ( "Not in a script: %s\n", strerror ( rc ) );
 		return rc;
@@ -332,10 +331,10 @@ static int goto_exec ( int argc, char **argv ) {
 
 	/* Find label */
 	saved_offset = script_offset;
-	if ( ( rc = process_script ( image, goto_find_label,
+	if ( ( rc = process_script ( current_image, goto_find_label,
 				     terminate_on_label_found ) ) != 0 ) {
 		script_offset = saved_offset;
-		DBGC ( image, "[%04zx] No such label :%s\n",
+		DBGC ( current_image, "[%04zx] No such label :%s\n",
 		       script_offset, goto_label );
 		return rc;
 	}

src/include/ipxe/efi/Protocol/ShimLock.h

@@ -1,31 +0,0 @@
-#ifndef _IPXE_EFI_SHIM_LOCK_PROTOCOL_H
-#define _IPXE_EFI_SHIM_LOCK_PROTOCOL_H
-
-/** @file
- *
- * EFI "shim lock" protocol
- *
- */
-
-FILE_LICENCE ( BSD3 );
-
-#define EFI_SHIM_LOCK_PROTOCOL_GUID					\
-	{ 0x605dab50, 0xe046, 0x4300,					\
-	{ 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23 } }
-
-#define SHIMAPI __asmcall
-
-typedef
-EFI_STATUS SHIMAPI
-(*EFI_SHIM_LOCK_VERIFY) (
-  IN VOID *buffer,
-  IN UINT32 size
-  );
-
-typedef struct _EFI_SHIM_LOCK_PROTOCOL {
-  EFI_SHIM_LOCK_VERIFY Verify;
-  VOID *Reserved1;
-  VOID *Reserved2;
-} EFI_SHIM_LOCK_PROTOCOL;
-
-#endif /*_IPXE_EFI_SHIM_LOCK_PROTOCOL_H */

src/include/ipxe/efi/efi.h

@@ -197,7 +197,6 @@ extern EFI_GUID efi_pci_io_protocol_guid;
 extern EFI_GUID efi_pci_root_bridge_io_protocol_guid;
 extern EFI_GUID efi_pxe_base_code_protocol_guid;
 extern EFI_GUID efi_serial_io_protocol_guid;
-extern EFI_GUID efi_shim_lock_protocol_guid;
 extern EFI_GUID efi_simple_file_system_protocol_guid;
 extern EFI_GUID efi_simple_network_protocol_guid;
 extern EFI_GUID efi_simple_pointer_protocol_guid;

src/include/ipxe/efi/efi_image.h

@@ -1,27 +0,0 @@
-#ifndef _IPXE_EFI_IMAGE_H
-#define _IPXE_EFI_IMAGE_H
-
-/** @file
- *
- * EFI images
- *
- */
-
-FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
-
-#include <ipxe/image.h>
-
-extern struct image_type efi_image_type[] __image_type ( PROBE_NORMAL );
-
-/**
- * Check if EFI image can be loaded directly
- *
- * @v image		EFI image
- * @ret can_load	EFI image can be loaded directly
- */
-static inline int efi_can_load ( struct image *image ) {
-
-	return ( image->type == efi_image_type );
-}
-
-#endif /* _IPXE_EFI_IMAGE_H */

src/include/ipxe/efi/efi_shim.h

@@ -1,24 +0,0 @@
-#ifndef _IPXE_EFI_SHIM_H
-#define _IPXE_EFI_SHIM_H
-
-/** @file
- *
- * UEFI shim special handling
- *
- */
-
-FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
-
-#include <ipxe/image.h>
-#include <ipxe/efi/efi.h>
-
-extern int efi_shim_require_loader;
-extern int efi_shim_allow_pxe;
-extern int efi_shim_allow_sbat;
-extern struct image_tag efi_shim __image_tag;
-
-extern int efi_shim_install ( struct image *shim, EFI_HANDLE handle,
-			      wchar_t **cmdline );
-extern void efi_shim_uninstall ( void );
-
-#endif /* _IPXE_EFI_SHIM_H */

src/include/ipxe/efi/efi_strings.h

@@ -19,8 +19,6 @@ extern int efi_vssnprintf ( wchar_t *wbuf, ssize_t swsize, const char *fmt,
 			    va_list args );
 extern int efi_ssnprintf ( wchar_t *wbuf, ssize_t swsize,
 			   const char *fmt, ... );
-extern int efi_vasprintf ( wchar_t **strp, const char *fmt, va_list args );
-extern int efi_asprintf ( wchar_t **strp, const char *fmt, ... );
 
 /**
  * Write a formatted string to a wide-character buffer

src/include/ipxe/errfile.h

@@ -78,7 +78,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #define ERRFILE_dma		       ( ERRFILE_CORE | 0x00260000 )
 #define ERRFILE_cachedhcp	       ( ERRFILE_CORE | 0x00270000 )
 #define ERRFILE_acpimac		       ( ERRFILE_CORE | 0x00280000 )
-#define ERRFILE_efi_strings	       ( ERRFILE_CORE | 0x00290000 )
 
 #define ERRFILE_eisa		     ( ERRFILE_DRIVER | 0x00000000 )
 #define ERRFILE_isa		     ( ERRFILE_DRIVER | 0x00010000 )
@@ -405,7 +404,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #define ERRFILE_dhe		      ( ERRFILE_OTHER | 0x005a0000 )
 #define ERRFILE_efi_cmdline	      ( ERRFILE_OTHER | 0x005b0000 )
 #define ERRFILE_efi_rng		      ( ERRFILE_OTHER | 0x005c0000 )
-#define ERRFILE_efi_shim	      ( ERRFILE_OTHER | 0x005d0000 )
 
 /** @} */
 

src/include/ipxe/image.h

@@ -61,16 +61,16 @@ struct image {
 };
 
 /** Image is registered */
-#define IMAGE_REGISTERED 0x0001
+#define IMAGE_REGISTERED 0x00001
+
+/** Image is selected for execution */
+#define IMAGE_SELECTED 0x0002
 
 /** Image is trusted */
-#define IMAGE_TRUSTED 0x0002
+#define IMAGE_TRUSTED 0x0004
 
 /** Image will be automatically unregistered after execution */
-#define IMAGE_AUTO_UNREGISTER 0x0004
-
-/** Image will be hidden from enumeration */
-#define IMAGE_HIDDEN 0x0008
+#define IMAGE_AUTO_UNREGISTER 0x0008
 
 /** An executable image type */
 struct image_type {
@@ -150,23 +150,8 @@ struct image_type {
 /** An executable image type */
 #define __image_type( probe_order ) __table_entry ( IMAGE_TYPES, probe_order )
 
-/** An image tag */
-struct image_tag {
-	/** Name */
-	const char *name;
-	/** Image (weak reference, nullified when image is freed) */
-	struct image *image;
-};
-
-/** Image tag table */
-#define IMAGE_TAGS __table ( struct image_tag, "image_tags" )
-
-/** An image tag */
-#define __image_tag __table_entry ( IMAGE_TAGS, 01 )
-
 extern struct list_head images;
-extern struct image_tag current_image;
-extern struct image_tag selected_image;
+extern struct image *current_image;
 
 /** Iterate over all registered images */
 #define for_each_image( image ) \
@@ -176,6 +161,15 @@ extern struct image_tag selected_image;
 #define for_each_image_safe( image, tmp ) \
 	list_for_each_entry_safe ( (image), (tmp), &images, list )
 
+/**
+ * Test for existence of images
+ *
+ * @ret existence	Some images exist
+ */
+static inline int have_images ( void ) {
+	return ( ! list_empty ( &images ) );
+}
+
 /**
  * Retrieve first image
  *
@@ -193,11 +187,11 @@ extern int image_set_len ( struct image *image, size_t len );
 extern int image_set_data ( struct image *image, userptr_t data, size_t len );
 extern int register_image ( struct image *image );
 extern void unregister_image ( struct image *image );
-extern struct image * find_image ( const char *name );
-extern struct image * find_image_tag ( struct image_tag *tag );
+struct image * find_image ( const char *name );
 extern int image_exec ( struct image *image );
 extern int image_replace ( struct image *replacement );
 extern int image_select ( struct image *image );
+extern struct image * image_find_selected ( void );
 extern int image_set_trust ( int require_trusted, int permanent );
 extern struct image * image_memory ( const char *name, userptr_t data,
 				     size_t len );
@@ -256,28 +250,4 @@ static inline void image_untrust ( struct image *image ) {
 	image->flags &= ~IMAGE_TRUSTED;
 }
 
-/**
- * Mark image as hidden
- *
- * @v image		Image
- */
-static inline void image_hide ( struct image *image ) {
-	image->flags |= IMAGE_HIDDEN;
-}
-
-/**
- * Tag image
- *
- * @v image		Image
- * @v tag		Image tag
- * @ret prev		Previous tagged image (if any)
- */
-static inline struct image * image_tag ( struct image *image,
-					 struct image_tag *tag ) {
-	struct image *prev = tag->image;
-
-	tag->image = image;
-	return prev;
-}
-
 #endif /* _IPXE_IMAGE_H */

src/include/ipxe/tls.h

@@ -52,9 +52,6 @@ struct tls_header {
 /** Change cipher content type */
 #define TLS_TYPE_CHANGE_CIPHER 20
 
-/** Change cipher spec magic byte */
-#define TLS_CHANGE_CIPHER_SPEC 1
-
 /** Alert content type */
 #define TLS_TYPE_ALERT 21
 
@@ -398,8 +395,6 @@ struct tls_connection {
 	struct io_buffer rx_header_iobuf;
 	/** List of received data buffers */
 	struct list_head rx_data;
-	/** Received handshake fragment */
-	struct io_buffer *rx_handshake;
 };
 
 /** RX I/O buffer size

src/include/usr/shimmgmt.h

@@ -1,17 +0,0 @@
-#ifndef _USR_SHIMMGMT_H
-#define _USR_SHIMMGMT_H
-
-/** @file
- *
- * EFI shim management
- *
- */
-
-FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
-
-#include <ipxe/image.h>
-
-extern int shim ( struct image *image, int require_loader, int allow_pxe,
-		  int allow_sbat );
-
-#endif /* _USR_SHIMMGMT_H */

src/interface/efi/efi_debug.c

@@ -143,8 +143,6 @@ static struct efi_well_known_guid efi_well_known_guids[] = {
 	  "PxeBaseCode" },
 	{ &efi_serial_io_protocol_guid,
 	  "SerialIo" },
-	{ &efi_shim_lock_protocol_guid,
-	  "ShimLock" },
 	{ &efi_simple_file_system_protocol_guid,
 	  "SimpleFileSystem" },
 	{ &efi_simple_network_protocol_guid,

src/interface/efi/efi_file.c

@@ -93,18 +93,8 @@ struct efi_file {
 	size_t ( * read ) ( struct efi_file_reader *reader );
 };
 
-/** An EFI fixed device path file */
-struct efi_file_path {
-	/** EFI file */
-	struct efi_file file;
-	/** Device path */
-	EFI_DEVICE_PATH_PROTOCOL *path;
-	/** EFI handle */
-	EFI_HANDLE handle;
-};
-
 static struct efi_file efi_file_root;
-static struct efi_file_path efi_file_initrd;
+static struct efi_file efi_file_initrd;
 
 /**
  * Free EFI file
@@ -250,10 +240,6 @@ static size_t efi_file_read_initrd ( struct efi_file_reader *reader ) {
 	len = 0;
 	for_each_image ( image ) {
 
-		/* Skip hidden images */
-		if ( image->flags & IMAGE_HIDDEN )
-			continue;
-
 		/* Pad to alignment boundary */
 		pad_len = ( ( -reader->pos ) & ( INITRD_ALIGN - 1 ) );
 		if ( pad_len ) {
@@ -295,12 +281,10 @@ static size_t efi_file_read_initrd ( struct efi_file_reader *reader ) {
  * Open fixed file
  *
  * @v file		EFI file
- * @v wname		Filename
  * @v new		New EFI file
  * @ret efirc		EFI status code
  */
 static EFI_STATUS efi_file_open_fixed ( struct efi_file *file,
-					const wchar_t *wname,
 					EFI_FILE_PROTOCOL **new ) {
 
 	/* Increment reference count */
@@ -309,8 +293,7 @@ static EFI_STATUS efi_file_open_fixed ( struct efi_file *file,
 	/* Return opened file */
 	*new = &file->file;
 
-	DBGC ( file, "EFIFILE %s opened via %ls\n",
-	       efi_file_name ( file ), wname );
+	DBGC ( file, "EFIFILE %s opened\n", efi_file_name ( file ) );
 	return 0;
 }
 
@@ -327,36 +310,6 @@ static void efi_file_image ( struct efi_file *file, struct image *image ) {
 	file->read = efi_file_read_image;
 }
 
-/**
- * Open image-backed file
- *
- * @v image		Image
- * @v wname		Filename
- * @v new		New EFI file
- * @ret efirc		EFI status code
- */
-static EFI_STATUS efi_file_open_image ( struct image *image,
-					const wchar_t *wname,
-					EFI_FILE_PROTOCOL **new ) {
-	struct efi_file *file;
-
-	/* Allocate and initialise file */
-	file = zalloc ( sizeof ( *file ) );
-	if ( ! file )
-		return EFI_OUT_OF_RESOURCES;
-	ref_init ( &file->refcnt, efi_file_free );
-	memcpy ( &file->file, &efi_file_root.file, sizeof ( file->file ) );
-	memcpy ( &file->load, &efi_file_root.load, sizeof ( file->load ) );
-	efi_file_image ( file, image_get ( image ) );
-
-	/* Return opened file */
-	*new = &file->file;
-
-	DBGC ( file, "EFIFILE %s opened via %ls\n",
-	       efi_file_name ( file ), wname );
-	return 0;
-}
-
 /**
  * Open file
  *
@@ -372,9 +325,9 @@ efi_file_open ( EFI_FILE_PROTOCOL *this, EFI_FILE_PROTOCOL **new,
 		CHAR16 *wname, UINT64 mode, UINT64 attributes __unused ) {
 	struct efi_file *file = container_of ( this, struct efi_file, file );
 	char buf[ wcslen ( wname ) + 1 /* NUL */ ];
+	struct efi_file *new_file;
 	struct image *image;
 	char *name;
-	char *sep;
 
 	/* Convert name to ASCII */
 	snprintf ( buf, sizeof ( buf ), "%ls", wname );
@@ -388,7 +341,7 @@ efi_file_open ( EFI_FILE_PROTOCOL *this, EFI_FILE_PROTOCOL **new,
 
 	/* Allow root directory itself to be opened */
 	if ( ( name[0] == '\0' ) || ( name[0] == '.' ) )
-		return efi_file_open_fixed ( &efi_file_root, wname, new );
+		return efi_file_open_fixed ( &efi_file_root, new );
 
 	/* Fail unless opening from the root */
 	if ( file != &efi_file_root ) {
@@ -404,28 +357,31 @@ efi_file_open ( EFI_FILE_PROTOCOL *this, EFI_FILE_PROTOCOL **new,
 		return EFI_WRITE_PROTECTED;
 	}
 
-	/* Allow registered images to be opened */
-	if ( ( image = efi_file_find ( name ) ) != NULL )
-		return efi_file_open_image ( image, wname, new );
-
 	/* Allow magic initrd to be opened */
-	if ( strcasecmp ( name, efi_file_initrd.file.name ) == 0 ) {
-		return efi_file_open_fixed ( &efi_file_initrd.file, wname,
-					     new );
+	if ( strcasecmp ( name, efi_file_initrd.name ) == 0 )
+		return efi_file_open_fixed ( &efi_file_initrd, new );
+
+	/* Identify image */
+	image = efi_file_find ( name );
+	if ( ! image ) {
+		DBGC ( file, "EFIFILE %s does not exist\n", name );
+		return EFI_NOT_FOUND;
 	}
 
-	/* Allow currently selected image to be opened as "grub*.efi",
-	 * to work around buggy versions of the UEFI shim.
-	 */
-	if ( ( strncasecmp ( name, "grub", 4 ) == 0 ) &&
-	     ( ( sep = strrchr ( name, '.' ) ) != NULL ) &&
-	     ( strcasecmp ( sep, ".efi" ) == 0 ) &&
-	     ( ( image = find_image_tag ( &selected_image ) ) != NULL ) ) {
-		return efi_file_open_image ( image, wname, new );
-	}
+	/* Allocate and initialise file */
+	new_file = zalloc ( sizeof ( *new_file ) );
+	if ( ! new_file )
+		return EFI_OUT_OF_RESOURCES;
+	ref_init ( &file->refcnt, efi_file_free );
+	memcpy ( &new_file->file, &efi_file_root.file,
+		 sizeof ( new_file->file ) );
+	memcpy ( &new_file->load, &efi_file_root.load,
+		 sizeof ( new_file->load ) );
+	efi_file_image ( new_file, image_get ( image ) );
+	*new = &new_file->file;
+	DBGC ( new_file, "EFIFILE %s opened\n", efi_file_name ( new_file ) );
 
-	DBGC ( file, "EFIFILE %ls does not exist\n", wname );
-	return EFI_NOT_FOUND;
+	return 0;
 }
 
 /**
@@ -539,21 +495,13 @@ static EFI_STATUS efi_file_read_dir ( struct efi_file *file, UINTN *len,
 	/* Construct directory entries for image-backed files */
 	index = file->pos;
 	for_each_image ( image ) {
-
-		/* Skip hidden images */
-		if ( image->flags & IMAGE_HIDDEN )
-			continue;
-
-		/* Skip preceding images */
-		if ( index-- )
-			continue;
-
-		/* Construct directory entry */
-		efi_file_image ( &entry, image );
-		efirc = efi_file_info ( &entry, len, data );
-		if ( efirc == 0 )
-			file->pos++;
-		return efirc;
+		if ( index-- == 0 ) {
+			efi_file_image ( &entry, image );
+			efirc = efi_file_info ( &entry, len, data );
+			if ( efirc == 0 )
+				file->pos++;
+			return efirc;
+		}
 	}
 
 	/* No more entries */
@@ -753,10 +701,10 @@ static EFI_STATUS EFIAPI efi_file_flush ( EFI_FILE_PROTOCOL *this ) {
  * @v data		Buffer, or NULL
  * @ret efirc		EFI status code
  */
-static EFI_STATUS EFIAPI
-efi_file_load ( EFI_LOAD_FILE2_PROTOCOL *this,
-		EFI_DEVICE_PATH_PROTOCOL *path __unused,
-		BOOLEAN boot __unused, UINTN *len, VOID *data ) {
+static EFI_STATUS EFIAPI efi_file_load ( EFI_LOAD_FILE2_PROTOCOL *this,
+					 EFI_DEVICE_PATH_PROTOCOL *path __unused,
+					 BOOLEAN boot __unused, UINTN *len,
+					 VOID *data ) {
 	struct efi_file *file = container_of ( this, struct efi_file, load );
 	size_t max_len;
 	size_t file_len;
@@ -804,6 +752,30 @@ static struct efi_file efi_file_root = {
 	.name = "",
 };
 
+/** Magic initrd file */
+static struct efi_file efi_file_initrd = {
+	.refcnt = REF_INIT ( ref_no_free ),
+	.file = {
+		.Revision = EFI_FILE_PROTOCOL_REVISION,
+		.Open = efi_file_open,
+		.Close = efi_file_close,
+		.Delete = efi_file_delete,
+		.Read = efi_file_read,
+		.Write = efi_file_write,
+		.GetPosition = efi_file_get_position,
+		.SetPosition = efi_file_set_position,
+		.GetInfo = efi_file_get_info,
+		.SetInfo = efi_file_set_info,
+		.Flush = efi_file_flush,
+	},
+	.load = {
+		.LoadFile = efi_file_load,
+	},
+	.image = NULL,
+	.name = "initrd.magic",
+	.read = efi_file_read_initrd,
+};
+
 /** Linux initrd fixed device path */
 static struct {
 	VENDOR_DEVICE_PATH vendor;
@@ -824,33 +796,6 @@ static struct {
 	},
 };
 
-/** Magic initrd file */
-static struct efi_file_path efi_file_initrd = {
-	.file = {
-		.refcnt = REF_INIT ( ref_no_free ),
-		.file = {
-			.Revision = EFI_FILE_PROTOCOL_REVISION,
-			.Open = efi_file_open,
-			.Close = efi_file_close,
-			.Delete = efi_file_delete,
-			.Read = efi_file_read,
-			.Write = efi_file_write,
-			.GetPosition = efi_file_get_position,
-			.SetPosition = efi_file_set_position,
-			.GetInfo = efi_file_get_info,
-			.SetInfo = efi_file_set_info,
-			.Flush = efi_file_flush,
-		},
-		.load = {
-			.LoadFile = efi_file_load,
-		},
-		.image = NULL,
-		.name = "initrd.magic",
-		.read = efi_file_read_initrd,
-	},
-	.path = &efi_file_initrd_path.vendor.Header,
-};
-
 /**
  * Open root directory
  *
@@ -863,7 +808,7 @@ efi_file_open_volume ( EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *filesystem __unused,
 		       EFI_FILE_PROTOCOL **file ) {
 
 	DBGC ( &efi_file_root, "EFIFILE open volume\n" );
-	return efi_file_open_fixed ( &efi_file_root, L"<volume>", file );
+	return efi_file_open_fixed ( &efi_file_root, file );
 }
 
 /** EFI simple file system protocol */
@@ -960,148 +905,107 @@ static EFI_DISK_IO_PROTOCOL efi_disk_io_protocol = {
 };
 
 /**
- * Claim use of fixed device path
+ * (Re)install fixed device path file
  *
- * @v file		Fixed device path file
- * @ret rc		Return status code
- *
- * The design choice in Linux of using a single fixed device path is
- * unfortunately messy to support, since device paths must be unique
- * within a system.  When multiple bootloaders are used (e.g. GRUB
- * loading iPXE loading Linux) then only one bootloader can ever
- * install the device path onto a handle.  Bootloaders must therefore
- * be prepared to locate an existing handle and uninstall its device
- * path protocol instance before installing a new handle with the
- * required device path.
- */
-static int efi_file_path_claim ( struct efi_file_path *file ) {
-	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-	EFI_DEVICE_PATH_PROTOCOL *end;
-	EFI_HANDLE handle;
-	VOID *old;
-	EFI_STATUS efirc;
-	int rc;
-
-	/* Sanity check */
-	assert ( file->handle == NULL );
-
-	/* Locate handle with this device path, if any */
-	end = file->path;
-	if ( ( ( efirc = bs->LocateDevicePath ( &efi_device_path_protocol_guid,
-						&end, &handle ) ) != 0 ) ||
-	     ( end->Type != END_DEVICE_PATH_TYPE ) ) {
-		return 0;
-	}
-
-	/* Locate device path protocol on this handle */
-	if ( ( ( efirc = bs->HandleProtocol ( handle,
-					      &efi_device_path_protocol_guid,
-					      &old ) ) != 0 ) ) {
-		rc = -EEFI ( efirc );
-		DBGC ( file, "EFIFILE %s could not locate %s: %s\n",
-		       efi_file_name ( &file->file ),
-		       efi_devpath_text ( file->path ), strerror ( rc ) );
-		return rc;
-	}
-
-	/* Uninstall device path protocol, leaving other protocols untouched */
-	if ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
-				handle,
-				&efi_device_path_protocol_guid, old,
-				NULL ) ) != 0 ) {
-		rc = -EEFI ( efirc );
-		DBGC ( file, "EFIFILE %s could not claim %s: %s\n",
-		       efi_file_name ( &file->file ),
-		       efi_devpath_text ( file->path ), strerror ( rc ) );
-		return rc;
-	}
-
-	DBGC ( file, "EFIFILE %s claimed %s",
-	       efi_file_name ( &file->file ), efi_devpath_text ( file->path ) );
-	DBGC ( file, " from %s\n", efi_handle_name ( handle ) );
-	return 0;
-}
-
-/**
- * Install fixed device path file
- *
- * @v file		Fixed device path file
+ * @v path		Device path
+ * @v load		Load file protocol, or NULL to uninstall protocol
  * @ret rc		Return status code
  *
  * Linux 5.7 added the ability to autodetect an initrd by searching
  * for a handle via a fixed vendor-specific "Linux initrd device path"
  * and then locating and using the EFI_LOAD_FILE2_PROTOCOL instance on
  * that handle.
+ *
+ * The design choice in Linux of using a single fixed device path
+ * makes this unfortunately messy to support, since device paths must
+ * be unique within a system.  When multiple bootloaders are used
+ * (e.g. GRUB loading iPXE loading Linux) then only one bootloader can
+ * ever install the device path onto a handle.  Subsequent bootloaders
+ * must locate the existing handle and replace the load file protocol
+ * instance with their own.
  */
-static int efi_file_path_install ( struct efi_file_path *file ) {
+static int efi_file_path_install ( EFI_DEVICE_PATH_PROTOCOL *path,
+				   EFI_LOAD_FILE2_PROTOCOL *load ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	EFI_DEVICE_PATH_PROTOCOL *end;
+	EFI_HANDLE handle;
+	VOID *path_copy;
+	VOID *old;
+	size_t path_len;
 	EFI_STATUS efirc;
 	int rc;
 
-	/* Sanity check */
-	assert ( file->handle == NULL );
+	/* Locate or install the handle with this device path */
+	end = path;
+	if ( ( ( efirc = bs->LocateDevicePath ( &efi_device_path_protocol_guid,
+						&end, &handle ) ) == 0 ) &&
+	     ( end->Type == END_DEVICE_PATH_TYPE ) ) {
 
-	/* Create a new handle with this device path */
-	if ( ( efirc = bs->InstallMultipleProtocolInterfaces (
-				&file->handle,
-				&efi_device_path_protocol_guid, file->path,
-				&efi_load_file2_protocol_guid, &file->file.load,
+		/* Exact match: reuse (or uninstall from) this handle */
+		if ( load ) {
+			DBGC ( path, "EFIFILE %s reusing existing handle\n",
+			       efi_devpath_text ( path ) );
+		}
+
+	} else {
+
+		/* Allocate a permanent copy of the device path, since
+		 * this handle will survive after this binary is
+		 * unloaded.
+		 */
+		path_len = ( efi_path_len ( path ) + sizeof ( *end ) );
+		if ( ( efirc = bs->AllocatePool ( EfiBootServicesData, path_len,
+						  &path_copy ) ) != 0 ) {
+			rc = -EEFI ( efirc );
+			DBGC ( path, "EFIFILE %s could not allocate device path: "
+			       "%s\n", efi_devpath_text ( path ), strerror ( rc ) );
+			return rc;
+		}
+		memcpy ( path_copy, path, path_len );
+
+		/* Create a new handle with this device path */
+		handle = NULL;
+		if ( ( efirc = bs->InstallMultipleProtocolInterfaces (
+				&handle,
+				&efi_device_path_protocol_guid, path_copy,
 				NULL ) ) != 0 ) {
+			rc = -EEFI ( efirc );
+			DBGC ( path, "EFIFILE %s could not create handle: %s\n",
+			       efi_devpath_text ( path ), strerror ( rc ) );
+			return rc;
+		}
+	}
+
+	/* Uninstall existing load file protocol instance, if any */
+	if ( ( ( efirc = bs->HandleProtocol ( handle, &efi_load_file2_protocol_guid,
+					      &old ) ) == 0 ) &&
+	     ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
+				handle,
+				&efi_load_file2_protocol_guid, old,
+				NULL ) ) != 0 ) ) {
 		rc = -EEFI ( efirc );
-		DBGC ( file, "EFIFILE %s could not install %s: %s\n",
-		       efi_file_name ( &file->file ),
-		       efi_devpath_text ( file->path ), strerror ( rc ) );
+		DBGC ( path, "EFIFILE %s could not uninstall %s: %s\n",
+		       efi_devpath_text ( path ),
+		       efi_guid_ntoa ( &efi_load_file2_protocol_guid ),
+		       strerror ( rc ) );
 		return rc;
 	}
 
-	DBGC ( file, "EFIFILE %s installed as %s\n",
-	       efi_file_name ( &file->file ), efi_devpath_text ( file->path ) );
-	return 0;
-}
-
-/**
- * Uninstall fixed device path file
- *
- * @v file		Fixed device path file
- * @ret rc		Return status code
- */
-static void efi_file_path_uninstall ( struct efi_file_path *file ) {
-	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-	EFI_STATUS efirc;
-	int rc;
-
-	/* Do nothing if file is already uninstalled */
-	if ( ! file->handle )
-		return;
-
-	/* Uninstall protocols.  Do this via two separate calls, in
-	 * case another executable has already uninstalled the device
-	 * path protocol from our handle.
-	 */
-	if ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
-				file->handle,
-				&efi_device_path_protocol_guid, file->path,
-				NULL ) ) != 0 ) {
+	/* Install new load file protocol instance, if applicable */
+	if ( ( load != NULL ) &&
+	     ( ( efirc = bs->InstallMultipleProtocolInterfaces (
+				&handle,
+				&efi_load_file2_protocol_guid, load,
+				NULL ) ) != 0 ) ) {
 		rc = -EEFI ( efirc );
-		DBGC ( file, "EFIFILE %s could not uninstall %s: %s\n",
-		       efi_file_name ( &file->file ),
-		       efi_devpath_text ( file->path ), strerror ( rc ) );
-		/* Continue uninstalling */
-	}
-	if ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
-				file->handle,
-				&efi_load_file2_protocol_guid, &file->file.load,
-				NULL ) ) != 0 ) {
-		rc = -EEFI ( efirc );
-		DBGC ( file, "EFIFILE %s could not uninstall %s: %s\n",
-		       efi_file_name ( &file->file ),
+		DBGC ( path, "EFIFILE %s could not install %s: %s\n",
+		       efi_devpath_text ( path ),
 		       efi_guid_ntoa ( &efi_load_file2_protocol_guid ),
 		       strerror ( rc ) );
-		/* Continue uninstalling */
+		return rc;
 	}
 
-	/* Mark handle as uninstalled */
-	file->handle = NULL;
+	return 0;
 }
 
 /**
@@ -1112,11 +1016,11 @@ static void efi_file_path_uninstall ( struct efi_file_path *file ) {
  */
 int efi_file_install ( EFI_HANDLE handle ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
+	EFI_LOAD_FILE2_PROTOCOL *load;
 	union {
 		EFI_DISK_IO_PROTOCOL *diskio;
 		void *interface;
 	} diskio;
-	struct image *image;
 	EFI_STATUS efirc;
 	int rc;
 
@@ -1175,24 +1079,24 @@ int efi_file_install ( EFI_HANDLE handle ) {
 	}
 	assert ( diskio.diskio == &efi_disk_io_protocol );
 
-	/* Claim Linux initrd fixed device path */
-	if ( ( rc = efi_file_path_claim ( &efi_file_initrd ) ) != 0 )
-		goto err_initrd_claim;
-
-	/* Install Linux initrd fixed device path file if non-empty */
-	for_each_image ( image ) {
-		if ( image->flags & IMAGE_HIDDEN )
-			continue;
-		if ( ( rc = efi_file_path_install ( &efi_file_initrd ) ) != 0 )
-			goto err_initrd_install;
-		break;
+	/* Install Linux initrd fixed device path file
+	 *
+	 * Install the device path handle unconditionally, since we
+	 * are definitively the bootloader providing the initrd, if
+	 * any, to the booted image.  Install the load file protocol
+	 * instance only if the initrd is non-empty, since Linux does
+	 * not gracefully handle a zero-length initrd.
+	 */
+	load = ( have_images() ? &efi_file_initrd.load : NULL );
+	if ( ( rc = efi_file_path_install ( &efi_file_initrd_path.vendor.Header,
+					    load ) ) != 0 ) {
+		goto err_initrd;
 	}
 
 	return 0;
 
-	efi_file_path_uninstall ( &efi_file_initrd );
- err_initrd_install:
- err_initrd_claim:
+	efi_file_path_install ( &efi_file_initrd_path.vendor.Header, NULL );
+ err_initrd:
 	bs->CloseProtocol ( handle, &efi_disk_io_protocol_guid,
 			    efi_image_handle, handle );
  err_open:
@@ -1219,7 +1123,7 @@ void efi_file_uninstall ( EFI_HANDLE handle ) {
 	int rc;
 
 	/* Uninstall Linux initrd fixed device path file */
-	efi_file_path_uninstall ( &efi_file_initrd );
+	efi_file_path_install ( &efi_file_initrd_path.vendor.Header, NULL );
 
 	/* Close our own disk I/O protocol */
 	bs->CloseProtocol ( handle, &efi_disk_io_protocol_guid,

src/interface/efi/efi_guid.c

@@ -54,7 +54,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 #include <ipxe/efi/Protocol/PciRootBridgeIo.h>
 #include <ipxe/efi/Protocol/PxeBaseCode.h>
 #include <ipxe/efi/Protocol/SerialIo.h>
-#include <ipxe/efi/Protocol/ShimLock.h>
 #include <ipxe/efi/Protocol/SimpleFileSystem.h>
 #include <ipxe/efi/Protocol/SimpleNetwork.h>
 #include <ipxe/efi/Protocol/SimplePointer.h>
@@ -228,10 +227,6 @@ EFI_GUID efi_pxe_base_code_protocol_guid
 EFI_GUID efi_serial_io_protocol_guid
 	= EFI_SERIAL_IO_PROTOCOL_GUID;
 
-/** Shim lock protocol GUID */
-EFI_GUID efi_shim_lock_protocol_guid
-	= EFI_SHIM_LOCK_PROTOCOL_GUID;
-
 /** Simple file system protocol GUID */
 EFI_GUID efi_simple_file_system_protocol_guid
 	= EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID;

src/interface/efi/efi_shim.c

@@ -1,406 +0,0 @@
-/*
- * Copyright (C) 2022 Michael Brown <mbrown@fensystems.co.uk>.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of the
- * License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
- *
- * You can also choose to distribute this program under the terms of
- * the Unmodified Binary Distribution Licence (as given in the file
- * COPYING.UBDL), provided that you have satisfied its requirements.
- */
-
-FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
-
-#include <string.h>
-#include <stdlib.h>
-#include <errno.h>
-#include <ipxe/image.h>
-#include <ipxe/efi/efi.h>
-#include <ipxe/efi/efi_strings.h>
-#include <ipxe/efi/efi_shim.h>
-#include <ipxe/efi/Protocol/PxeBaseCode.h>
-#include <ipxe/efi/Protocol/ShimLock.h>
-
-/** @file
- *
- * UEFI shim special handling
- *
- */
-
-FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
-
-/**
- * Require use of a third party loader binary
- *
- * The UEFI shim is gradually becoming less capable of directly
- * executing a Linux kernel image, due to an ever increasing list of
- * assumptions that it will only ever be used in conjunction with a
- * second stage loader binary such as GRUB.
- *
- * For example: shim will erroneously complain if the image that it
- * loads and executes does not in turn call in to the "shim lock
- * protocol" to verify a separate newly loaded binary before calling
- * ExitBootServices(), even if no such separate binary is used or
- * required.
- *
- * Experience shows that there is unfortunately no point in trying to
- * get a fix for this upstreamed into shim.  We therefore default to
- * reducing the Secure Boot attack surface by removing, where
- * possible, this spurious requirement for the use of an additional
- * second stage loader.
- *
- * This option may be used to require the use of an additional second
- * stage loader binary, in case this behaviour is ever desirable.
- */
-int efi_shim_require_loader = 0;
-
-/**
- * Allow use of PXE base code protocol
- *
- * We provide shim with access to all of the relevant downloaded files
- * via our EFI_SIMPLE_FILE_SYSTEM_PROTOCOL interface.  However, shim
- * will instead try to redownload the files via TFTP since it prefers
- * to use the EFI_PXE_BASE_CODE_PROTOCOL installed on the same handle.
- *
- * Experience shows that there is unfortunately no point in trying to
- * get a fix for this upstreamed into shim.  We therefore default to
- * working around this undesirable behaviour by stopping the PXE base
- * code protocol before invoking shim.
- *
- * This option may be used to allow shim to use the PXE base code
- * protocol, in case this behaviour is ever desirable.
- */
-int efi_shim_allow_pxe = 0;
-
-/**
- * Allow SBAT variable access
- *
- * The UEFI shim implements a fairly nicely designed revocation
- * mechanism designed around the concept of security generations.
- * Unfortunately nobody in the shim community has thus far added the
- * relevant metadata to the Linux kernel, with the result that current
- * versions of shim are incapable of booting current versions of the
- * Linux kernel.
- *
- * Experience shows that there is unfortunately no point in trying to
- * get a fix for this upstreamed into shim.  We therefore default to
- * working around this undesirable behaviour by patching accesses to
- * the "SbatLevel" variable used to hold SBAT configuration.
- *
- * This option may be used to allow shim unpatched access to the
- * "SbatLevel" variable, in case this behaviour is ever desirable.
- */
-int efi_shim_allow_sbat = 0;
-
-/** UEFI shim image */
-struct image_tag efi_shim __image_tag = {
-	.name = "SHIM",
-};
-
-/** Original GetMemoryMap() function */
-static EFI_GET_MEMORY_MAP efi_shim_orig_get_memory_map;
-
-/** Original ExitBootServices() function */
-static EFI_EXIT_BOOT_SERVICES efi_shim_orig_exit_boot_services;
-
-/** Original SetVariable() function */
-static EFI_SET_VARIABLE efi_shim_orig_set_variable;
-
-/** Original GetVariable() function */
-static EFI_GET_VARIABLE efi_shim_orig_get_variable;
-
-/** Verify read from SbatLevel variable */
-static int efi_shim_sbatlevel_verify;
-
-/**
- * Check if variable is SbatLevel
- *
- * @v name		Variable name
- * @v guid		Variable namespace GUID
- * @ret is_sbatlevel	Variable is SbatLevel
- */
-static int efi_shim_is_sbatlevel ( const CHAR16 *name, const EFI_GUID *guid ) {
-	static CHAR16 sbatlevel[] = L"SbatLevel";
-	EFI_GUID *shimlock = &efi_shim_lock_protocol_guid;
-
-	return ( ( memcmp ( name, sbatlevel, sizeof ( sbatlevel ) ) == 0 ) &&
-		 ( memcmp ( guid, shimlock, sizeof ( *shimlock ) ) == 0 ) );
-}
-
-/**
- * Unlock UEFI shim
- *
- */
-static void efi_shim_unlock ( void ) {
-	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-	uint8_t empty[0];
-	union {
-		EFI_SHIM_LOCK_PROTOCOL *lock;
-		void *interface;
-	} u;
-	EFI_STATUS efirc;
-
-	/* Locate shim lock protocol */
-	if ( ( efirc = bs->LocateProtocol ( &efi_shim_lock_protocol_guid,
-					    NULL, &u.interface ) ) == 0 ) {
-		u.lock->Verify ( empty, sizeof ( empty ) );
-		DBGC ( &efi_shim, "SHIM unlocked via %p\n", u.lock );
-	}
-}
-
-/**
- * Wrap GetMemoryMap()
- *
- * @v len		Memory map size
- * @v map		Memory map
- * @v key		Memory map key
- * @v desclen		Descriptor size
- * @v descver		Descriptor version
- * @ret efirc		EFI status code
- */
-static EFIAPI EFI_STATUS efi_shim_get_memory_map ( UINTN *len,
-						   EFI_MEMORY_DESCRIPTOR *map,
-						   UINTN *key, UINTN *desclen,
-						   UINT32 *descver ) {
-
-	/* Unlock shim */
-	if ( ! efi_shim_require_loader )
-		efi_shim_unlock();
-
-	/* Hand off to original GetMemoryMap() */
-	return efi_shim_orig_get_memory_map ( len, map, key, desclen,
-					      descver );
-}
-
-/**
- * Wrap ExitBootServices()
- *
- * @v handle		Image handle
- * @v key		Memory map key
- * @ret efirc		EFI status code
- */
-static EFIAPI EFI_STATUS efi_shim_exit_boot_services ( EFI_HANDLE handle,
-						       UINTN key ) {
-	EFI_RUNTIME_SERVICES *rs = efi_systab->RuntimeServices;
-
-	/* Restore original runtime services functions */
-	rs->GetVariable = efi_shim_orig_get_variable;
-	rs->SetVariable = efi_shim_orig_set_variable;
-
-	/* Hand off to original ExitBootServices() */
-	return efi_shim_orig_exit_boot_services ( handle, key );
-}
-
-/**
- * Wrap SetVariable()
- *
- * @v name		Variable name
- * @v guid		Variable namespace GUID
- * @v attrs		Attributes
- * @v len		Buffer size
- * @v data		Data buffer
- * @ret efirc		EFI status code
- */
-static EFI_STATUS EFIAPI
-efi_shim_set_variable ( CHAR16 *name, EFI_GUID *guid, UINT32 attrs,
-			UINTN len, VOID *data ) {
-	EFI_STATUS efirc;
-
-	/* Call original SetVariable() */
-	efirc = efi_shim_orig_set_variable ( name, guid, attrs, len, data );
-
-	/* Allow verification of SbatLevel variable content */
-	if ( efi_shim_is_sbatlevel ( name, guid ) && ( efirc == 0 ) ) {
-		DBGC ( &efi_shim, "SHIM detected write to %ls:\n", name );
-		DBGC_HDA ( &efi_shim, 0, data, len );
-		efi_shim_sbatlevel_verify = 1;
-	}
-
-	return efirc;
-}
-
-/**
- * Wrap GetVariable()
- *
- * @v name		Variable name
- * @v guid		Variable namespace GUID
- * @v attrs		Attributes to fill in
- * @v len		Buffer size
- * @v data		Data buffer
- * @ret efirc		EFI status code
- */
-static EFI_STATUS EFIAPI
-efi_shim_get_variable ( CHAR16 *name, EFI_GUID *guid, UINT32 *attrs,
-			UINTN *len, VOID *data ) {
-	char *value = data;
-	EFI_STATUS efirc;
-
-	/* Call original GetVariable() */
-	efirc = efi_shim_orig_get_variable ( name, guid, attrs, len, data );
-
-	/* Patch SbatLevel variable if applicable */
-	if ( efi_shim_is_sbatlevel ( name, guid ) && data && ( efirc == 0 ) ) {
-		if ( efi_shim_allow_sbat ) {
-			DBGC ( &efi_shim, "SHIM allowing read from %ls:\n",
-			       name );
-		} else if ( efi_shim_sbatlevel_verify ) {
-			DBGC ( &efi_shim, "SHIM allowing one read from %ls:\n",
-			       name );
-			efi_shim_sbatlevel_verify = 0;
-		} else {
-			DBGC ( &efi_shim, "SHIM patching read from %ls:\n",
-			       name );
-			value[0] = '\0';
-		}
-		DBGC_HDA ( &efi_shim, 0, data, *len );
-	}
-
-	return efirc;
-}
-
-/**
- * Inhibit use of PXE base code
- *
- * @v handle		EFI handle
- * @ret rc		Return status code
- */
-static int efi_shim_inhibit_pxe ( EFI_HANDLE handle ) {
-	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-	union {
-		EFI_PXE_BASE_CODE_PROTOCOL *pxe;
-		void *interface;
-	} u;
-	EFI_STATUS efirc;
-	int rc;
-
-	/* Locate PXE base code */
-	if ( ( efirc = bs->OpenProtocol ( handle,
-					  &efi_pxe_base_code_protocol_guid,
-					  &u.interface, efi_image_handle, NULL,
-					  EFI_OPEN_PROTOCOL_GET_PROTOCOL ))!=0){
-		rc = -EEFI ( efirc );
-		DBGC ( &efi_shim, "SHIM could not open PXE base code: %s\n",
-		       strerror ( rc ) );
-		goto err_no_base;
-	}
-
-	/* Stop PXE base code */
-	if ( ( efirc = u.pxe->Stop ( u.pxe ) ) != 0 ) {
-		rc = -EEFI ( efirc );
-		DBGC ( &efi_shim, "SHIM could not stop PXE base code: %s\n",
-		       strerror ( rc ) );
-		goto err_stop;
-	}
-
-	/* Success */
-	rc = 0;
-	DBGC ( &efi_shim, "SHIM stopped PXE base code\n" );
-
- err_stop:
-	bs->CloseProtocol ( handle, &efi_pxe_base_code_protocol_guid,
-			    efi_image_handle, NULL );
- err_no_base:
-	return rc;
-}
-
-/**
- * Update command line
- *
- * @v shim		Shim image
- * @v cmdline		Command line to update
- * @ret rc		Return status code
- */
-static int efi_shim_cmdline ( struct image *shim, wchar_t **cmdline ) {
-	wchar_t *shimcmdline;
-	int len;
-	int rc;
-
-	/* Construct new command line */
-	len = ( shim->cmdline ?
-		efi_asprintf ( &shimcmdline, "%s %s", shim->name,
-			       shim->cmdline ) :
-		efi_asprintf ( &shimcmdline, "%s %ls", shim->name,
-			       *cmdline ) );
-	if ( len < 0 ) {
-		rc = len;
-		DBGC ( &efi_shim, "SHIM could not construct command line: "
-		       "%s\n", strerror ( rc ) );
-		return rc;
-	}
-
-	/* Replace command line */
-	free ( *cmdline );
-	*cmdline = shimcmdline;
-
-	return 0;
-}
-
-/**
- * Install UEFI shim special handling
- *
- * @v shim		Shim image
- * @v handle		EFI device handle
- * @v cmdline		Command line to update
- * @ret rc		Return status code
- */
-int efi_shim_install ( struct image *shim, EFI_HANDLE handle,
-		       wchar_t **cmdline ) {
-	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-	EFI_RUNTIME_SERVICES *rs = efi_systab->RuntimeServices;
-	int rc;
-
-	/* Stop PXE base code */
-	if ( ( ! efi_shim_allow_pxe ) &&
-	     ( ( rc = efi_shim_inhibit_pxe ( handle ) ) != 0 ) ) {
-		goto err_inhibit_pxe;
-	}
-
-	/* Update command line */
-	if ( ( rc = efi_shim_cmdline ( shim, cmdline ) ) != 0 )
-		goto err_cmdline;
-
-	/* Record original boot and runtime services functions */
-	efi_shim_orig_get_memory_map = bs->GetMemoryMap;
-	efi_shim_orig_exit_boot_services = bs->ExitBootServices;
-	efi_shim_orig_set_variable = rs->SetVariable;
-	efi_shim_orig_get_variable = rs->GetVariable;
-
-	/* Wrap relevant boot and runtime services functions */
-	bs->GetMemoryMap = efi_shim_get_memory_map;
-	bs->ExitBootServices = efi_shim_exit_boot_services;
-	rs->SetVariable = efi_shim_set_variable;
-	rs->GetVariable = efi_shim_get_variable;
-
-	return 0;
-
- err_cmdline:
- err_inhibit_pxe:
-	return rc;
-}
-
-/**
- * Uninstall UEFI shim special handling
- *
- */
-void efi_shim_uninstall ( void ) {
-	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-	EFI_RUNTIME_SERVICES *rs = efi_systab->RuntimeServices;
-
-	/* Restore original boot and runtime services functions */
-	bs->GetMemoryMap = efi_shim_orig_get_memory_map;
-	bs->ExitBootServices = efi_shim_orig_exit_boot_services;
-	rs->SetVariable = efi_shim_orig_set_variable;
-	rs->GetVariable = efi_shim_orig_get_variable;
-}

src/interface/efi/efi_strings.c

@@ -25,8 +25,6 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 
 #include <stddef.h>
 #include <stdarg.h>
-#include <stdlib.h>
-#include <errno.h>
 #include <ipxe/vsprintf.h>
 #include <ipxe/efi/efi_strings.h>
 
@@ -152,45 +150,3 @@ int efi_ssnprintf ( wchar_t *wbuf, ssize_t swsize, const char *fmt, ... ) {
 	va_end ( args );
 	return len;
 }
-
-/**
- * Write a formatted string to newly allocated memory
- *
- * @v wstrp		Pointer to hold allocated string
- * @v fmt		Format string
- * @v args		Arguments corresponding to the format string
- * @ret len		Length of formatted string (in wide characters)
- */
-int efi_vasprintf ( wchar_t **wstrp, const char *fmt, va_list args ) {
-	size_t len;
-	va_list args_tmp;
-
-	/* Calculate length needed for string */
-	va_copy ( args_tmp, args );
-	len = ( efi_vsnprintf ( NULL, 0, fmt, args_tmp ) + 1 );
-	va_end ( args_tmp );
-
-	/* Allocate and fill string */
-	*wstrp = malloc ( len * sizeof ( **wstrp ) );
-	if ( ! *wstrp )
-		return -ENOMEM;
-	return efi_vsnprintf ( *wstrp, len, fmt, args );
-}
-
-/**
- * Write a formatted string to newly allocated memory
- *
- * @v wstrp		Pointer to hold allocated string
- * @v fmt		Format string
- * @v ...		Arguments corresponding to the format string
- * @ret len		Length of formatted string (in wide characters)
- */
-int efi_asprintf ( wchar_t **wstrp, const char *fmt, ... ) {
-	va_list args;
-	int len;
-
-	va_start ( args, fmt );
-	len = efi_vasprintf ( wstrp, fmt, args );
-	va_end ( args );
-	return len;
-}

src/net/tls.c

@@ -388,7 +388,6 @@ static void free_tls ( struct refcnt *refcnt ) {
 		list_del ( &iobuf->list );
 		free_iob ( iobuf );
 	}
-	free_iob ( tls->rx_handshake );
 	x509_chain_put ( tls->certs );
 	x509_chain_put ( tls->chain );
 	x509_root_put ( tls->root );
@@ -1683,14 +1682,9 @@ static int tls_send_certificate_verify ( struct tls_connection *tls ) {
  * @ret rc		Return status code
  */
 static int tls_send_change_cipher ( struct tls_connection *tls ) {
-	static const struct {
-		uint8_t spec;
-	} __attribute__ (( packed )) change_cipher = {
-		.spec = TLS_CHANGE_CIPHER_SPEC,
-	};
-
+	static const uint8_t change_cipher[1] = { 1 };
 	return tls_send_plaintext ( tls, TLS_TYPE_CHANGE_CIPHER,
-				    &change_cipher, sizeof ( change_cipher ) );
+				    change_cipher, sizeof ( change_cipher ) );
 }
 
 /**
@@ -1737,27 +1731,20 @@ static int tls_send_finished ( struct tls_connection *tls ) {
  * Receive new Change Cipher record
  *
  * @v tls		TLS connection
- * @v iobuf		I/O buffer
+ * @v data		Plaintext record
+ * @v len		Length of plaintext record
  * @ret rc		Return status code
  */
 static int tls_new_change_cipher ( struct tls_connection *tls,
-				   struct io_buffer *iobuf ) {
-	const struct {
-		uint8_t spec;
-	} __attribute__ (( packed )) *change_cipher = iobuf->data;
-	size_t len = iob_len ( iobuf );
+				   const void *data, size_t len ) {
 	int rc;
 
-	/* Sanity check */
-	if ( ( sizeof ( *change_cipher ) != len ) ||
-	     ( change_cipher->spec != TLS_CHANGE_CIPHER_SPEC ) ) {
+	if ( ( len != 1 ) || ( *( ( uint8_t * ) data ) != 1 ) ) {
 		DBGC ( tls, "TLS %p received invalid Change Cipher\n", tls );
-		DBGC_HD ( tls, change_cipher, len );
+		DBGC_HD ( tls, data, len );
 		return -EINVAL_CHANGE_CIPHER;
 	}
-	iob_pull ( iobuf, sizeof ( *change_cipher ) );
 
-	/* Change receive cipher spec */
 	if ( ( rc = tls_change_cipher ( tls, &tls->rx_cipherspec_pending,
 					&tls->rx_cipherspec ) ) != 0 ) {
 		DBGC ( tls, "TLS %p could not activate RX cipher: %s\n",
@@ -1773,27 +1760,25 @@ static int tls_new_change_cipher ( struct tls_connection *tls,
  * Receive new Alert record
  *
  * @v tls		TLS connection
- * @v iobuf		I/O buffer
+ * @v data		Plaintext record
+ * @v len		Length of plaintext record
  * @ret rc		Return status code
  */
-static int tls_new_alert ( struct tls_connection *tls,
-			   struct io_buffer *iobuf ) {
+static int tls_new_alert ( struct tls_connection *tls, const void *data,
+			   size_t len ) {
 	const struct {
 		uint8_t level;
 		uint8_t description;
 		char next[0];
-	} __attribute__ (( packed )) *alert = iobuf->data;
-	size_t len = iob_len ( iobuf );
+	} __attribute__ (( packed )) *alert = data;
 
 	/* Sanity check */
 	if ( sizeof ( *alert ) != len ) {
 		DBGC ( tls, "TLS %p received overlength Alert\n", tls );
-		DBGC_HD ( tls, alert, len );
+		DBGC_HD ( tls, data, len );
 		return -EINVAL_ALERT;
 	}
-	iob_pull ( iobuf, sizeof ( *alert ) );
 
-	/* Handle alert */
 	switch ( alert->level ) {
 	case TLS_ALERT_WARNING:
 		DBGC ( tls, "TLS %p received warning alert %d\n",
@@ -2407,33 +2392,38 @@ static int tls_new_finished ( struct tls_connection *tls,
  * Receive new Handshake record
  *
  * @v tls		TLS connection
- * @v iobuf		I/O buffer
+ * @v data		Plaintext record
+ * @v len		Length of plaintext record
  * @ret rc		Return status code
  */
 static int tls_new_handshake ( struct tls_connection *tls,
-			       struct io_buffer *iobuf ) {
-	size_t remaining;
+			       const void *data, size_t len ) {
+	size_t remaining = len;
 	int rc;
 
-	while ( ( remaining = iob_len ( iobuf ) ) ) {
+	while ( remaining ) {
 		const struct {
 			uint8_t type;
 			tls24_t length;
 			uint8_t payload[0];
-		} __attribute__ (( packed )) *handshake = iobuf->data;
+		} __attribute__ (( packed )) *handshake = data;
 		const void *payload;
 		size_t payload_len;
 		size_t record_len;
 
 		/* Parse header */
 		if ( sizeof ( *handshake ) > remaining ) {
-			/* Leave remaining fragment unconsumed */
-			break;
+			DBGC ( tls, "TLS %p received underlength Handshake\n",
+			       tls );
+			DBGC_HD ( tls, data, remaining );
+			return -EINVAL_HANDSHAKE;
 		}
 		payload_len = tls_uint24 ( &handshake->length );
 		if ( payload_len > ( remaining - sizeof ( *handshake ) ) ) {
-			/* Leave remaining fragment unconsumed */
-			break;
+			DBGC ( tls, "TLS %p received overlength Handshake\n",
+			       tls );
+			DBGC_HD ( tls, data, len );
+			return -EINVAL_HANDSHAKE;
 		}
 		payload = &handshake->payload;
 		record_len = ( sizeof ( *handshake ) + payload_len );
@@ -2480,60 +2470,15 @@ static int tls_new_handshake ( struct tls_connection *tls,
 		 * which are explicitly excluded).
 		 */
 		if ( handshake->type != TLS_HELLO_REQUEST )
-			tls_add_handshake ( tls, handshake, record_len );
+			tls_add_handshake ( tls, data, record_len );
 
 		/* Abort on failure */
 		if ( rc != 0 )
 			return rc;
 
 		/* Move to next handshake record */
-		iob_pull ( iobuf, record_len );
-	}
-
-	return 0;
-}
-
-/**
- * Receive new unknown record
- *
- * @v tls		TLS connection
- * @v iobuf		I/O buffer
- * @ret rc		Return status code
- */
-static int tls_new_unknown ( struct tls_connection *tls __unused,
-			     struct io_buffer *iobuf ) {
-
-	/* RFC4346 says that we should just ignore unknown record types */
-	iob_pull ( iobuf, iob_len ( iobuf ) );
-	return 0;
-}
-
-/**
- * Receive new data record
- *
- * @v tls		TLS connection
- * @v rx_data		List of received data buffers
- * @ret rc		Return status code
- */
-static int tls_new_data ( struct tls_connection *tls,
-			  struct list_head *rx_data ) {
-	struct io_buffer *iobuf;
-	int rc;
-
-	/* Fail unless we are ready to receive data */
-	if ( ! tls_ready ( tls ) )
-		return -ENOTCONN;
-
-	/* Deliver each I/O buffer in turn */
-	while ( ( iobuf = list_first_entry ( rx_data, struct io_buffer,
-					     list ) ) ) {
-		list_del ( &iobuf->list );
-		if ( ( rc = xfer_deliver_iob ( &tls->plainstream,
-					       iobuf ) ) != 0 ) {
-			DBGC ( tls, "TLS %p could not deliver data: "
-			       "%s\n", tls, strerror ( rc ) );
-			return rc;
-		}
+		data += record_len;
+		remaining -= record_len;
 	}
 
 	return 0;
@@ -2549,18 +2494,41 @@ static int tls_new_data ( struct tls_connection *tls,
  */
 static int tls_new_record ( struct tls_connection *tls, unsigned int type,
 			    struct list_head *rx_data ) {
-	int ( * handler ) ( struct tls_connection *tls,
-			    struct io_buffer *iobuf );
-	struct io_buffer *tmp = NULL;
-	struct io_buffer **iobuf;
+	struct io_buffer *iobuf;
+	int ( * handler ) ( struct tls_connection *tls, const void *data,
+			    size_t len );
 	int rc;
 
-	/* Deliver data records as-is to the plainstream interface */
-	if ( type == TLS_TYPE_DATA )
-		return tls_new_data ( tls, rx_data );
+	/* Deliver data records to the plainstream interface */
+	if ( type == TLS_TYPE_DATA ) {
 
-	/* Determine handler and fragment buffer */
-	iobuf = &tmp;
+		/* Fail unless we are ready to receive data */
+		if ( ! tls_ready ( tls ) )
+			return -ENOTCONN;
+
+		/* Deliver each I/O buffer in turn */
+		while ( ( iobuf = list_first_entry ( rx_data, struct io_buffer,
+						     list ) ) ) {
+			list_del ( &iobuf->list );
+			if ( ( rc = xfer_deliver_iob ( &tls->plainstream,
+						       iobuf ) ) != 0 ) {
+				DBGC ( tls, "TLS %p could not deliver data: "
+				       "%s\n", tls, strerror ( rc ) );
+				return rc;
+			}
+		}
+		return 0;
+	}
+
+	/* For all other records, merge into a single I/O buffer */
+	iobuf = iob_concatenate ( rx_data );
+	if ( ! iobuf ) {
+		DBGC ( tls, "TLS %p could not concatenate non-data record "
+		       "type %d\n", tls, type );
+		return -ENOMEM_RX_CONCAT;
+	}
+
+	/* Determine handler */
 	switch ( type ) {
 	case TLS_TYPE_CHANGE_CIPHER:
 		handler = tls_new_change_cipher;
@@ -2570,44 +2538,19 @@ static int tls_new_record ( struct tls_connection *tls, unsigned int type,
 		break;
 	case TLS_TYPE_HANDSHAKE:
 		handler = tls_new_handshake;
-		iobuf = &tls->rx_handshake;
 		break;
 	default:
-		DBGC ( tls, "TLS %p unknown record type %d\n", tls, type );
-		handler = tls_new_unknown;
+		/* RFC4346 says that we should just ignore unknown
+		 * record types.
+		 */
+		handler = NULL;
+		DBGC ( tls, "TLS %p ignoring record type %d\n", tls, type );
 		break;
 	}
 
-	/* Merge into a single I/O buffer */
-	if ( *iobuf )
-		list_add ( &(*iobuf)->list, rx_data );
-	*iobuf = iob_concatenate ( rx_data );
-	if ( ! *iobuf ) {
-		DBGC ( tls, "TLS %p could not concatenate non-data record "
-		       "type %d\n", tls, type );
-		rc = -ENOMEM_RX_CONCAT;
-		goto err_concatenate;
-	}
-
-	/* Handle record */
-	if ( ( rc = handler ( tls, *iobuf ) ) != 0 )
-		goto err_handle;
-
-	/* Discard I/O buffer if empty */
-	if ( ! iob_len ( *iobuf ) ) {
-		free_iob ( *iobuf );
-		*iobuf = NULL;
-	}
-
-	/* Sanity check */
-	assert ( tmp == NULL );
-
-	return 0;
-
- err_handle:
-	free_iob ( *iobuf );
-	*iobuf = NULL;
- err_concatenate:
+	/* Handle record and free I/O buffer */
+	rc = ( handler ? handler ( tls, iobuf->data, iob_len ( iobuf ) ) : 0 );
+	free_iob ( iobuf );
 	return rc;
 }
 

src/usr/imgmgmt.c

@@ -156,21 +156,15 @@ int imgacquire ( const char *name_uri, unsigned long timeout,
  * @v image		Executable/loadable image
  */
 void imgstat ( struct image *image ) {
-	struct image_tag *tag;
-
 	printf ( "%s : %zd bytes", image->name, image->len );
 	if ( image->type )
 		printf ( " [%s]", image->type->name );
-	for_each_table_entry ( tag, IMAGE_TAGS ) {
-		if ( tag->image == image )
-			printf ( " [%s]", tag->name );
-	}
 	if ( image->flags & IMAGE_TRUSTED )
 		printf ( " [TRUSTED]" );
+	if ( image->flags & IMAGE_SELECTED )
+		printf ( " [SELECTED]" );
 	if ( image->flags & IMAGE_AUTO_UNREGISTER )
 		printf ( " [AUTOFREE]" );
-	if ( image->flags & IMAGE_HIDDEN )
-		printf ( " [HIDDEN]" );
 	if ( image->cmdline )
 		printf ( " \"%s\"", image->cmdline );
 	printf ( "\n" );

src/usr/shimmgmt.c

@@ -1,61 +0,0 @@
-/*
- * Copyright (C) 2023 Michael Brown <mbrown@fensystems.co.uk>.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of the
- * License, or any later version.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
- *
- * You can also choose to distribute this program under the terms of
- * the Unmodified Binary Distribution Licence (as given in the file
- * COPYING.UBDL), provided that you have satisfied its requirements.
- */
-
-FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
-
-#include <ipxe/efi/efi.h>
-#include <ipxe/efi/efi_shim.h>
-#include <usr/shimmgmt.h>
-
-/** @file
- *
- * EFI shim management
- *
- */
-
-/**
- * Set shim image
- *
- * @v image		Shim image, or NULL to clear shim
- * @v require_loader	Require use of a third party loader
- * @v allow_pxe		Allow use of PXE base code
- * @v allow_sbat	Allow SBAT variable access
- * @ret rc		Return status code
- */
-int shim ( struct image *image, int require_loader, int allow_pxe,
-	   int allow_sbat ) {
-
-	/* Record (or clear) shim image */
-	image_tag ( image, &efi_shim );
-
-	/* Avoid including image in constructed initrd */
-	if ( image )
-		image_hide ( image );
-
-	/* Record configuration */
-	efi_shim_require_loader = require_loader;
-	efi_shim_allow_pxe = allow_pxe;
-	efi_shim_allow_sbat = allow_sbat;
-
-	return 0;
-}

src/util/elf2efi.c

@@ -168,9 +168,6 @@
  */
 #define EFI_IMAGE_ALIGN 0x1000
 
-/** Number of data directory entries */
-#define NUMBER_OF_DIRECTORY_ENTRIES 8
-
 struct elf_file {
 	void *data;
 	size_t len;
@@ -181,7 +178,6 @@ struct pe_section {
 	struct pe_section *next;
 	EFI_IMAGE_SECTION_HEADER hdr;
 	void ( * fixup ) ( struct pe_section *section );
-	int hidden;
 	uint8_t contents[0];
 };
 
@@ -195,6 +191,7 @@ struct pe_relocs {
 
 struct pe_header {
 	EFI_IMAGE_DOS_HEADER dos;
+	uint8_t padding[128];
 	EFI_IMAGE_NT_HEADERS nt;
 };
 
@@ -208,11 +205,7 @@ static struct pe_header efi_pe_header = {
 		.FileHeader = {
 			.TimeDateStamp = 0x10d1a884,
 			.SizeOfOptionalHeader =
-				( sizeof ( efi_pe_header.nt.OptionalHeader ) -
-				  ( ( EFI_IMAGE_NUMBER_OF_DIRECTORY_ENTRIES -
-				      NUMBER_OF_DIRECTORY_ENTRIES ) *
-				    sizeof ( efi_pe_header.nt.OptionalHeader.
-					     DataDirectory[0] ) ) ),
+				sizeof ( efi_pe_header.nt.OptionalHeader ),
 			.Characteristics = ( EFI_IMAGE_FILE_DLL |
 					     EFI_IMAGE_FILE_MACHINE |
 					     EFI_IMAGE_FILE_EXECUTABLE_IMAGE ),
@@ -225,17 +218,15 @@ static struct pe_header efi_pe_header = {
 			.FileAlignment = EFI_FILE_ALIGN,
 			.SizeOfImage = EFI_IMAGE_ALIGN,
 			.SizeOfHeaders = sizeof ( efi_pe_header ),
-			.NumberOfRvaAndSizes = NUMBER_OF_DIRECTORY_ENTRIES,
+			.NumberOfRvaAndSizes =
+				EFI_IMAGE_NUMBER_OF_DIRECTORY_ENTRIES,
 		},
 	},
 };
 
 /** Command-line options */
 struct options {
-	/** PE32+ subsystem type */
 	unsigned int subsystem;
-	/** Create hybrid BIOS/UEFI binary */
-	int hybrid;
 };
 
 /**
@@ -642,12 +633,10 @@ static struct pe_section * process_section ( struct elf_file *elf,
 	/* Update RVA limits */
 	start = new->hdr.VirtualAddress;
 	end = ( start + new->hdr.Misc.VirtualSize );
-	if ( ! new->hidden ) {
-		if ( ( ! *applicable_start ) || ( *applicable_start >= start ) )
-			*applicable_start = start;
-		if ( *applicable_end < end )
-			*applicable_end = end;
-	}
+	if ( ( ! *applicable_start ) || ( *applicable_start >= start ) )
+		*applicable_start = start;
+	if ( *applicable_end < end )
+		*applicable_end = end;
 	if ( data_start < code_end )
 		data_start = code_end;
 	if ( data_mid < data_start )
@@ -667,11 +656,8 @@ static struct pe_section * process_section ( struct elf_file *elf,
 		( data_end - data_mid );
 
 	/* Update remaining file header fields */
-	if ( ! new->hidden ) {
-		pe_header->nt.FileHeader.NumberOfSections++;
-		pe_header->nt.OptionalHeader.SizeOfHeaders +=
-			sizeof ( new->hdr );
-	}
+	pe_header->nt.FileHeader.NumberOfSections++;
+	pe_header->nt.OptionalHeader.SizeOfHeaders += sizeof ( new->hdr );
 	pe_header->nt.OptionalHeader.SizeOfImage =
 		efi_image_align ( data_end );
 
@@ -687,12 +673,10 @@ static struct pe_section * process_section ( struct elf_file *elf,
  * @v nsyms		Number of symbol table entries
  * @v rel		Relocation record
  * @v pe_reltab		PE relocation table to fill in
- * @v opts		Options
  */
 static void process_reloc ( struct elf_file *elf, const Elf_Shdr *shdr,
 			    const Elf_Sym *syms, unsigned int nsyms,
-			    const Elf_Rel *rel, struct pe_relocs **pe_reltab,
-			    struct options *opts ) {
+			    const Elf_Rel *rel, struct pe_relocs **pe_reltab ) {
 	unsigned int type = ELF_R_TYPE ( rel->r_info );
 	unsigned int sym = ELF_R_SYM ( rel->r_info );
 	unsigned int mrel = ELF_MREL ( elf->ehdr->e_machine, type );
@@ -755,15 +739,6 @@ static void process_reloc ( struct elf_file *elf, const Elf_Shdr *shdr,
 			 * loaded.
 			 */
 			break;
-		case ELF_MREL ( EM_X86_64, R_X86_64_32 ) :
-			/* Ignore 32-bit relocations in a hybrid
-			 * 32-bit BIOS and 64-bit UEFI binary,
-			 * otherwise fall through to treat as an
-			 * unknown type.
-			 */
-			if ( opts->hybrid )
-				break;
-			/* fallthrough */
 		default:
 			eprintf ( "Unrecognised relocation type %d\n", type );
 			exit ( 1 );
@@ -778,11 +753,9 @@ static void process_reloc ( struct elf_file *elf, const Elf_Shdr *shdr,
  * @v shdr		ELF section header
  * @v stride		Relocation record size
  * @v pe_reltab		PE relocation table to fill in
- * @v opts		Options
  */
 static void process_relocs ( struct elf_file *elf, const Elf_Shdr *shdr,
-			     size_t stride, struct pe_relocs **pe_reltab,
-			     struct options *opts ) {
+			     size_t stride, struct pe_relocs **pe_reltab ) {
 	const Elf_Shdr *symtab;
 	const Elf_Sym *syms;
 	const Elf_Rel *rel;
@@ -800,7 +773,7 @@ static void process_relocs ( struct elf_file *elf, const Elf_Shdr *shdr,
 	rel = ( elf->data + shdr->sh_offset );
 	nrels = ( shdr->sh_size / stride );
 	for ( i = 0 ; i < nrels ; i++ ) {
-		process_reloc ( elf, shdr, syms, nsyms, rel, pe_reltab, opts );
+		process_reloc ( elf, shdr, syms, nsyms, rel, pe_reltab );
 		rel = ( ( ( const void * ) rel ) + stride );
 	}
 }
@@ -941,7 +914,6 @@ static void write_pe_file ( struct pe_header *pe_header,
 			    FILE *pe ) {
 	struct pe_section *section;
 	unsigned long fpos = 0;
-	unsigned int count = 0;
 
 	/* Align length of headers */
 	fpos = pe_header->nt.OptionalHeader.SizeOfHeaders =
@@ -959,26 +931,19 @@ static void write_pe_file ( struct pe_header *pe_header,
 	}
 
 	/* Write file header */
-	if ( fwrite ( pe_header,
-		      ( offsetof ( typeof ( *pe_header ), nt.OptionalHeader ) +
-			pe_header->nt.FileHeader.SizeOfOptionalHeader ),
-		      1, pe ) != 1 ) {
+	if ( fwrite ( pe_header, sizeof ( *pe_header ), 1, pe ) != 1 ) {
 		perror ( "Could not write PE header" );
 		exit ( 1 );
 	}
 
 	/* Write section headers */
 	for ( section = pe_sections ; section ; section = section->next ) {
-		if ( section->hidden )
-			continue;
 		if ( fwrite ( &section->hdr, sizeof ( section->hdr ),
 			      1, pe ) != 1 ) {
 			perror ( "Could not write section header" );
 			exit ( 1 );
 		}
-		count++;
 	}
-	assert ( count == pe_header->nt.FileHeader.NumberOfSections );
 
 	/* Write sections */
 	for ( section = pe_sections ; section ; section = section->next ) {
@@ -1004,7 +969,6 @@ static void write_pe_file ( struct pe_header *pe_header,
  *
  * @v elf_name		ELF file name
  * @v pe_name		PE file name
- * @v opts		Options
  */
 static void elf2pe ( const char *elf_name, const char *pe_name,
 		     struct options *opts ) {
@@ -1048,13 +1012,13 @@ static void elf2pe ( const char *elf_name, const char *pe_name,
 
 			/* Process .rel relocations */
 			process_relocs ( &elf, shdr, sizeof ( Elf_Rel ),
-					 &pe_reltab, opts );
+					 &pe_reltab );
 
 		} else if ( shdr->sh_type == SHT_RELA ) {
 
 			/* Process .rela relocations */
 			process_relocs ( &elf, shdr, sizeof ( Elf_Rela ),
-					 &pe_reltab, opts );
+					 &pe_reltab );
 		}
 	}
 
@@ -1107,12 +1071,11 @@ static int parse_options ( const int argc, char **argv,
 		int option_index = 0;
 		static struct option long_options[] = {
 			{ "subsystem", required_argument, NULL, 's' },
-			{ "hybrid", no_argument, NULL, 'H' },
 			{ "help", 0, NULL, 'h' },
 			{ 0, 0, 0, 0 }
 		};
 
-		if ( ( c = getopt_long ( argc, argv, "s:Hh",
+		if ( ( c = getopt_long ( argc, argv, "s:h",
 					 long_options,
 					 &option_index ) ) == -1 ) {
 			break;
@@ -1127,9 +1090,6 @@ static int parse_options ( const int argc, char **argv,
 				exit ( 2 );
 			}
 			break;
-		case 'H':
-			opts->hybrid = 1;
-			break;
 		case 'h':
 			print_help ( argv[0] );
 			exit ( 0 );