From 5b41b9a80ffb365376d8d522675a8d248a8717ab Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Mon, 26 Oct 2020 15:10:18 +0000 Subject: [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 --- src/interface/efi/efi_snp.c | 56 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 44 insertions(+), 12 deletions(-) (limited to 'src/interface/efi/efi_snp.c') diff --git a/src/interface/efi/efi_snp.c b/src/interface/efi/efi_snp.c index 91e796a2e..5285d322b 100644 --- a/src/interface/efi/efi_snp.c +++ b/src/interface/efi/efi_snp.c @@ -36,6 +36,7 @@ FILE_LICENCE ( GPL2_OR_LATER ); #include #include #include +#include #include #include #include @@ -1626,6 +1627,7 @@ static int efi_snp_probe ( struct net_device *netdev ) { struct efi_snp_device *snpdev; unsigned int ifcnt; void *interface; + int leak = 0; EFI_STATUS efirc; int rc; @@ -1794,7 +1796,7 @@ static int efi_snp_probe ( struct net_device *netdev ) { list_del ( &snpdev->list ); if ( snpdev->package_list ) - efi_snp_hii_uninstall ( snpdev ); + leak |= efi_snp_hii_uninstall ( snpdev ); efi_child_del ( efidev->device, snpdev->handle ); err_efi_child_add: bs->CloseProtocol ( snpdev->handle, &efi_nii31_protocol_guid, @@ -1803,7 +1805,7 @@ static int efi_snp_probe ( struct net_device *netdev ) { bs->CloseProtocol ( snpdev->handle, &efi_nii_protocol_guid, efi_image_handle, snpdev->handle ); err_open_nii: - bs->UninstallMultipleProtocolInterfaces ( + if ( ( efirc = bs->UninstallMultipleProtocolInterfaces ( snpdev->handle, &efi_simple_network_protocol_guid, &snpdev->snp, &efi_device_path_protocol_guid, snpdev->path, @@ -1811,17 +1813,30 @@ static int efi_snp_probe ( 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 ); + NULL ) ) != 0 ) { + DBGC ( snpdev, "SNPDEV %p could not uninstall: %s\n", + snpdev, strerror ( -EEFI ( efirc ) ) ); + efi_nullify_snp ( &snpdev->snp ); + efi_nullify_nii ( &snpdev->nii ); + efi_nullify_name2 ( &snpdev->name2 ); + efi_nullify_load_file ( &snpdev->load_file ); + leak = 1; + } err_install_protocol_interface: - free ( snpdev->path ); + if ( ! leak ) + free ( snpdev->path ); err_path: bs->CloseEvent ( snpdev->snp.WaitForPacket ); err_create_event: err_ll_addr_len: - netdev_put ( netdev ); - free ( snpdev ); + if ( ! leak ) { + netdev_put ( netdev ); + free ( snpdev ); + } err_alloc_snp: err_no_efidev: + if ( leak ) + DBGC ( snpdev, "SNPDEV %p nullified and leaked\n", snpdev ); return rc; } @@ -1858,6 +1873,8 @@ 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; + EFI_STATUS efirc; /* Locate SNP device */ snpdev = efi_snp_demux ( netdev ); @@ -1869,13 +1886,13 @@ static void efi_snp_remove ( struct net_device *netdev ) { /* Uninstall the SNP */ list_del ( &snpdev->list ); if ( snpdev->package_list ) - efi_snp_hii_uninstall ( snpdev ); + leak |= efi_snp_hii_uninstall ( snpdev ); efi_child_del ( snpdev->efidev->device, snpdev->handle ); bs->CloseProtocol ( snpdev->handle, &efi_nii_protocol_guid, efi_image_handle, snpdev->handle ); bs->CloseProtocol ( snpdev->handle, &efi_nii31_protocol_guid, efi_image_handle, snpdev->handle ); - bs->UninstallMultipleProtocolInterfaces ( + if ( ( efirc = bs->UninstallMultipleProtocolInterfaces ( snpdev->handle, &efi_simple_network_protocol_guid, &snpdev->snp, &efi_device_path_protocol_guid, snpdev->path, @@ -1883,11 +1900,26 @@ 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 ); - free ( snpdev->path ); + NULL ) ) != 0 ) { + DBGC ( snpdev, "SNPDEV %p could not uninstall: %s\n", + snpdev, strerror ( -EEFI ( efirc ) ) ); + efi_nullify_snp ( &snpdev->snp ); + efi_nullify_nii ( &snpdev->nii ); + efi_nullify_name2 ( &snpdev->name2 ); + efi_nullify_load_file ( &snpdev->load_file ); + leak = 1; + } + if ( ! leak ) + free ( snpdev->path ); bs->CloseEvent ( snpdev->snp.WaitForPacket ); - netdev_put ( snpdev->netdev ); - free ( snpdev ); + if ( ! leak ) { + netdev_put ( snpdev->netdev ); + free ( snpdev ); + } + + /* Report leakage, if applicable */ + if ( leak ) + DBGC ( snpdev, "SNPDEV %p nullified and leaked\n", snpdev ); } /** SNP driver */ -- cgit v1.2.3-55-g7522