[efi] Allow initialisation via SNP interface even while claimed

iPXE will currently fail all SNP interface methods with EFI_NOT_READY
while the network devices are claimed for use by iPXE's own network
stack.

As of commit c70b3e0 ("[efi] Always enable recursion when calling
ConnectController()"), this exposes latent UEFI firmware bugs on some
systems at the point of calling ExitBootServices().

With recursion enabled, the MnpDxe driver will immediately attempt to
consume the SNP protocol instance provided by iPXE.  Since the network
devices are claimed by iPXE at this point, the calls by MnpDxe to
Start() and Initialize() will both fail with EFI_NOT_READY.

This unfortunately triggers a broken error-handling code path in the
Ip6Dxe driver.  Specifically: Ip6DriverBindingStart() will call
Ip6CreateService(), which will call Ip6ServiceConfigMnp(), which will
return an error.  The subsequent error handling code path in
Ip6CreateService() simply calls Ip6CleanService().  The code in
Ip6CleanService() will attempt to leave the all-nodes multicast group,
which will fail since the group was never joined.  This will result in
Ip6CleanService() returning an error and omitting most of the required
clean-up operations.  In particular, the MNP protocol instance will
remain opened with BY_DRIVER attributes even though the Ip6Dxe driver
start method has failed.

When ExitBootServices() is eventually called, iPXE will attempt to
uninstall the SNP protocol instance.  This results in the UEFI core
calling Ip6DriverBindingStop(), which will fail since there is no
EFI_IP6_SERVICE_BINDING_PROTOCOL instance installed on the handle.

A failure during a call to UninstallMultipleProtocolInterfaces() will
result in the UEFI core attempting to reinstall any successfully
uninstalled protocols.  This is an intrinsically unsafe operation, and
represents a fundamental design flaw in UEFI.  Failure code paths
cannot be required to themselves handle failures, since there is no
well-defined correct outcome of such a situation.

With a current build of OVMF, this results in some unexpected debug
messages occurring at the time that the loaded operating system calls
ExitBootServices().  With the UEFI firmware in Hyper-V, the result is
an immediate reboot.

Work around these UEFI design and implementation flaws by allowing the
calls to our EFI_SIMPLE_NETWORK_PROTOCOL instance's Start() and
Initialize() methods to return success even when the network devices
are claimed for exclusive use by iPXE.  This is sufficient to allow
MnpDxe to believe that it has successfully initialised the device, and
thereby avoids the problematic failure code paths in Ip6Dxe.

Debugged-by: Aaron Heusser <aaron_heusser@hotmail.com>
Debugged-by: Pico Mitchell <pico@randomapplications.com>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

1 files changed, 14 insertions, 7 deletions

diff --git a/src/interface/efi/efi_snp.c b/src/interface/efi/efi_snp.c
index c1ce90427..0a280eef4 100644
--- a/src/interface/efi/efi_snp.c
+++ b/src/interface/efi/efi_snp.c
@@ -192,9 +192,11 @@ efi_snp_start ( EFI_SIMPLE_NETWORK_PROTOCOL *snp ) {
 
 	DBGC ( snpdev, "SNPDEV %p START\n", snpdev );
 
-	/* Fail if net device is currently claimed for use by iPXE */
-	if ( efi_snp_claimed )
-		return EFI_NOT_READY;
+	/* Allow start even if net device is currently claimed by iPXE */
+	if ( efi_snp_claimed ) {
+		DBGC ( snpdev, "SNPDEV %p allowing start while claimed\n",
+		       snpdev );
+	}
 
 	snpdev->started = 1;
 	efi_snp_set_state ( snpdev );
@@ -244,10 +246,16 @@ efi_snp_initialize ( EFI_SIMPLE_NETWORK_PROTOCOL *snp,
 	       snpdev, ( ( unsigned long ) extra_rx_bufsize ),
 	       ( ( unsigned long ) extra_tx_bufsize ) );
 
-	/* Fail if net device is currently claimed for use by iPXE */
+	/* Do nothing if net device is currently claimed for use by
+	 * iPXE.  Do not return an error, because this will cause
+	 * MnpDxe et al to fail to install the relevant child handles
+	 * and to leave behind a partially initialised device handle
+	 * that can cause a later system crash.
+	 */
 	if ( efi_snp_claimed ) {
-		rc = -EAGAIN;
-		goto err_claimed;
+		DBGC ( snpdev, "SNPDEV %p ignoring initialization while "
+		       "claimed\n", snpdev );
+		return 0;
 	}
 
 	/* Raise TPL */
@@ -263,7 +271,6 @@ efi_snp_initialize ( EFI_SIMPLE_NETWORK_PROTOCOL *snp,
 
  err_open:
 	efi_restore_tpl ( &tpl );
- err_claimed:
 	return EFIRC ( rc );
 }