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

diff --git a/src/interface/efi/efi_block.c b/src/interface/efi/efi_block.c
index eeae8fca54ef70699dabceb6f9fcf6b1d9c7fb02..74cf7c0c0766486bbc51f077f6b62eb1457e55cb 100644 (file)
--- 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 5c7bb950ce76142605104dd8e14278a76739b87a..e2be3ffe0b16fe2c96e6132f043570b6f25ce0ac 100644 (file)
--- 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 c08383e49c7da7c2769a2fdf0298b97e93347a8b..6649eb1b03039028c8c03b1e9b3e3f2c949e7625 100644 (file)
--- 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 8ca8e69173babfda58a631e47a169967ea13ebea..5d5f80cd7991e3048fa85c8273d2651886bb99a5 100644 (file)
--- 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;
 }