[efi] Nullify interfaces and leak memory on uninstallation failure

The UEFI specification allows uninstallation of a protocol interface
to fail.  There is no sensible way for code to react to this, since
uninstallation is likely to be taking place on a code path that cannot
itself fail (e.g. a code path that is itself a failure path).

Where the protocol structure exists within a dynamically allocated
block of memory, this leads to possible use-after-free bugs.  Work
around this unfortunate design choice by nullifying the protocol
(i.e. overwriting the method pointers with no-ops) and leaking the
memory containing the protocol structure.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
This commit is contained in:
Michael Brown
2020-10-26 15:10:18 +00:00
parent 86c6c79fcd
commit 5b41b9a80f
7 changed files with 737 additions and 50 deletions

View File

@@ -41,6 +41,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
#include <ipxe/efi/efi.h>
#include <ipxe/efi/efi_snp.h>
#include <ipxe/efi/efi_pxe.h>
#include <ipxe/efi/efi_null.h>
#include <ipxe/efi/Protocol/PxeBaseCode.h>
#include <ipxe/efi/Protocol/AppleNetBoot.h>
#include <usr/ifmgmt.h>
@@ -1591,6 +1592,7 @@ int efi_pxe_install ( EFI_HANDLE handle, struct net_device *netdev ) {
struct efi_pxe *pxe;
struct in_addr ip;
BOOLEAN use_ipv6;
int leak = 0;
EFI_STATUS efirc;
int rc;
@@ -1643,14 +1645,23 @@ int efi_pxe_install ( EFI_HANDLE handle, struct net_device *netdev ) {
pxe->name, efi_handle_name ( handle ) );
return 0;
bs->UninstallMultipleProtocolInterfaces (
if ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
handle,
&efi_pxe_base_code_protocol_guid, &pxe->base,
&efi_apple_net_boot_protocol_guid, &pxe->apple,
NULL );
NULL ) ) != 0 ) {
DBGC ( pxe, "PXE %s could not uninstall: %s\n",
pxe->name, strerror ( -EEFI ( efirc ) ) );
efi_nullify_pxe ( &pxe->base );
efi_nullify_apple ( &pxe->apple );
leak = 1;
}
err_install_protocol:
ref_put ( &pxe->refcnt );
if ( ! leak )
ref_put ( &pxe->refcnt );
err_alloc:
if ( leak )
DBGC ( pxe, "PXE %s nullified and leaked\n", pxe->name );
return rc;
}
@@ -1662,6 +1673,8 @@ 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;
EFI_STATUS efirc;
/* Locate PXE base code */
pxe = efi_pxe_find ( handle );
@@ -1675,13 +1688,24 @@ void efi_pxe_uninstall ( EFI_HANDLE handle ) {
efi_pxe_stop ( &pxe->base );
/* Uninstall PXE base code protocol */
bs->UninstallMultipleProtocolInterfaces (
if ( ( efirc = bs->UninstallMultipleProtocolInterfaces (
handle,
&efi_pxe_base_code_protocol_guid, &pxe->base,
&efi_apple_net_boot_protocol_guid, &pxe->apple,
NULL );
NULL ) ) != 0 ) {
DBGC ( pxe, "PXE %s could not uninstall: %s\n",
pxe->name, strerror ( -EEFI ( efirc ) ) );
efi_nullify_pxe ( &pxe->base );
efi_nullify_apple ( &pxe->apple );
leak = 1;
}
/* Remove from list and drop list's reference */
list_del ( &pxe->list );
ref_put ( &pxe->refcnt );
if ( ! leak )
ref_put ( &pxe->refcnt );
/* Report leakage, if applicable */
if ( leak )
DBGC ( pxe, "PXE %s nullified and leaked\n", pxe->name );
}