[efi] Skip interface uninstallation during shutdown

iPXE seems to be almost alone in the UEFI world in attempting to shut
down cleanly, free resources, and leave hardware in a well-defined
reset state before handing over to the booted operating system.

The UEFI driver model does allow for graceful shutdown via
uninstallation of protocol interfaces.  However, virtually no other
UEFI drivers do this, and the external code paths that react to
uninstallation are consequently poorly tested.  This leads to a
proliferation of bugs found in UEFI implementations in the wild, as
described in commits such as 1295b4a ("[efi] Allow initialisation via
SNP interface even while claimed") or b6e2ea0 ("[efi] Veto the HP
XhciDxe Driver").

Try to avoid triggering such bugs by unconditionally skipping the
protocol interface uninstallation during UEFI boot services shutdown,
leaving the interfaces present but nullified and deliberately leaking
the containing memory.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

4 files changed, 23 insertions, 18 deletions

diff --git a/src/interface/efi/efi_block.c b/src/interface/efi/efi_block.c
index eeae8fc..74cf7c0 100644
--- a/src/interface/efi/efi_block.c
+++ b/src/interface/efi/efi_block.c
@@ -358,7 +358,7 @@ static void efi_block_unhook ( unsigned int drive ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 	struct san_device *sandev;
 	struct efi_block_data *block;
-	int leak = 0;
+	int leak = efi_shutdown_in_progress;
 	EFI_STATUS efirc;
 
 	/* Find SAN device */
@@ -370,11 +370,12 @@ static void efi_block_unhook ( unsigned int drive ) {
 	block = sandev->priv;
 
 	/* Uninstall protocols */
-	if ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
+	if ( ( ! efi_shutdown_in_progress ) &&
+	     ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
 			block->handle,
 			&efi_block_io_protocol_guid, &block->block_io,
 			&efi_device_path_protocol_guid, block->path,
-			NULL ) ) != 0 ) {
+			NULL ) ) != 0 ) ) {
 		DBGC ( sandev, "EFIBLK %#02x could not uninstall protocols: "
 		       "%s\n", sandev->drive, strerror ( -EEFI ( efirc ) ) );
 		leak = 1;
@@ -395,7 +396,7 @@ static void efi_block_unhook ( unsigned int drive ) {
 		sandev_put ( sandev );
 
 	/* Report leakage, if applicable */
-	if ( leak ) {
+	if ( leak && ( ! efi_shutdown_in_progress ) ) {
 		DBGC ( sandev, "EFIBLK %#02x nullified and leaked\n",
 		       sandev->drive );
 	}
diff --git a/src/interface/efi/efi_pxe.c b/src/interface/efi/efi_pxe.c
index 5c7bb95..e2be3ff 100644
--- a/src/interface/efi/efi_pxe.c
+++ b/src/interface/efi/efi_pxe.c
@@ -1673,7 +1673,7 @@ int efi_pxe_install ( EFI_HANDLE handle, struct net_device *netdev ) {
 void efi_pxe_uninstall ( EFI_HANDLE handle ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 	struct efi_pxe *pxe;
-	int leak = 0;
+	int leak = efi_shutdown_in_progress;
 	EFI_STATUS efirc;
 
 	/* Locate PXE base code */
@@ -1688,11 +1688,12 @@ void efi_pxe_uninstall ( EFI_HANDLE handle ) {
 	efi_pxe_stop ( &pxe->base );
 
 	/* Uninstall PXE base code protocol */
-	if ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
+	if ( ( ! efi_shutdown_in_progress ) &&
+	     ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
 			handle,
 			&efi_pxe_base_code_protocol_guid, &pxe->base,
 			&efi_apple_net_boot_protocol_guid, &pxe->apple,
-			NULL ) ) != 0 ) {
+			NULL ) ) != 0 ) ) {
 		DBGC ( pxe, "PXE %s could not uninstall: %s\n",
 		       pxe->name, strerror ( -EEFI ( efirc ) ) );
 		leak = 1;
@@ -1706,6 +1707,6 @@ void efi_pxe_uninstall ( EFI_HANDLE handle ) {
 		ref_put ( &pxe->refcnt );
 
 	/* Report leakage, if applicable */
-	if ( leak )
+	if ( leak && ( ! efi_shutdown_in_progress ) )
 		DBGC ( pxe, "PXE %s nullified and leaked\n", pxe->name );
 }
diff --git a/src/interface/efi/efi_snp.c b/src/interface/efi/efi_snp.c
index c08383e..6649eb1 100644
--- a/src/interface/efi/efi_snp.c
+++ b/src/interface/efi/efi_snp.c
@@ -1873,7 +1873,7 @@ static void efi_snp_notify ( struct net_device *netdev ) {
 static void efi_snp_remove ( struct net_device *netdev ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 	struct efi_snp_device *snpdev;
-	int leak = 0;
+	int leak = efi_shutdown_in_progress;
 	EFI_STATUS efirc;
 
 	/* Locate SNP device */
@@ -1892,7 +1892,8 @@ static void efi_snp_remove ( struct net_device *netdev ) {
 			    efi_image_handle, snpdev->handle );
 	bs->CloseProtocol ( snpdev->handle, &efi_nii31_protocol_guid,
 			    efi_image_handle, snpdev->handle );
-	if ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
+	if ( ( ! efi_shutdown_in_progress ) &&
+	     ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
 			snpdev->handle,
 			&efi_simple_network_protocol_guid, &snpdev->snp,
 			&efi_device_path_protocol_guid, snpdev->path,
@@ -1900,7 +1901,7 @@ static void efi_snp_remove ( struct net_device *netdev ) {
 			&efi_nii31_protocol_guid, &snpdev->nii,
 			&efi_component_name2_protocol_guid, &snpdev->name2,
 			&efi_load_file_protocol_guid, &snpdev->load_file,
-			NULL ) ) != 0 ) {
+			NULL ) ) != 0 ) ) {
 		DBGC ( snpdev, "SNPDEV %p could not uninstall: %s\n",
 		       snpdev, strerror ( -EEFI ( efirc ) ) );
 		leak = 1;
@@ -1918,7 +1919,7 @@ static void efi_snp_remove ( struct net_device *netdev ) {
 	}
 
 	/* Report leakage, if applicable */
-	if ( leak )
+	if ( leak && ( ! efi_shutdown_in_progress ) )
 		DBGC ( snpdev, "SNPDEV %p nullified and leaked\n", snpdev );
 }
 
diff --git a/src/interface/efi/efi_snp_hii.c b/src/interface/efi/efi_snp_hii.c
index 8ca8e69..5d5f80c 100644
--- a/src/interface/efi/efi_snp_hii.c
+++ b/src/interface/efi/efi_snp_hii.c
@@ -797,7 +797,7 @@ int efi_snp_hii_install ( struct efi_snp_device *snpdev ) {
  */
 int efi_snp_hii_uninstall ( struct efi_snp_device *snpdev ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-	int leak = 0;
+	int leak = efi_shutdown_in_progress;
 	EFI_STATUS efirc;
 
 	/* Do nothing if HII database protocol is not supported */
@@ -806,10 +806,11 @@ int efi_snp_hii_uninstall ( struct efi_snp_device *snpdev ) {
 
 	/* Uninstall protocols and remove package list */
 	efi_child_del ( snpdev->handle, snpdev->hii_child_handle );
-	if ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
+	if ( ( ! efi_shutdown_in_progress ) &&
+	     ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
 			snpdev->hii_child_handle,
 			&efi_hii_config_access_protocol_guid, &snpdev->hii,
-			NULL ) ) != 0 ) {
+			NULL ) ) != 0 ) ) {
 		DBGC ( snpdev, "SNPDEV %p could not uninstall HII protocol: "
 		       "%s\n", snpdev, strerror ( -EEFI ( efirc ) ) );
 		leak = 1;
@@ -817,10 +818,11 @@ int efi_snp_hii_uninstall ( struct efi_snp_device *snpdev ) {
 	efi_nullify_hii ( &snpdev->hii );
 	if ( ! leak )
 		efihii->RemovePackageList ( efihii, snpdev->hii_handle );
-	if ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
+	if ( ( ! efi_shutdown_in_progress ) &&
+	     ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
 			snpdev->hii_child_handle,
 			&efi_device_path_protocol_guid, snpdev->hii_child_path,
-			NULL ) ) != 0 ) {
+			NULL ) ) != 0 ) ) {
 		DBGC ( snpdev, "SNPDEV %p could not uninstall HII path: %s\n",
 		       snpdev, strerror ( -EEFI ( efirc ) ) );
 		leak = 1;
@@ -833,7 +835,7 @@ int efi_snp_hii_uninstall ( struct efi_snp_device *snpdev ) {
 	}
 
 	/* Report leakage, if applicable */
-	if ( leak )
+	if ( leak && ( ! efi_shutdown_in_progress ) )
 		DBGC ( snpdev, "SNPDEV %p HII nullified and leaked\n", snpdev );
 	return leak;
 }