[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>

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/image/efi_image.c

@@ -147,38 +147,38 @@ static int efi_image_exec ( struct image *image ) {
 	/* 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;
 	}
 
 	/* 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 ( 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;
 	}
@@ -186,8 +186,8 @@ 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;
 	}
@@ -199,8 +199,8 @@ static int efi_image_exec ( 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 {
@@ -220,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;
 	}
 
@@ -251,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();
 	}
 
@@ -324,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 {

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/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
@@ -368,8 +358,8 @@ efi_file_open ( EFI_FILE_PROTOCOL *this, EFI_FILE_PROTOCOL **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, 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 );
@@ -711,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;
@@ -762,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;
@@ -782,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
  *
@@ -918,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;
 }
 
 /**
@@ -1070,6 +1016,7 @@ 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;
@@ -1132,21 +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 */
-	if ( have_images() &&
-	     ( ( rc = efi_file_path_install ( &efi_file_initrd ) ) != 0 ) ) {
-		goto err_initrd_install;
+	/* 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:
@@ -1173,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/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;
 }