[U-Boot] [PATCH 0/3] efi_loader: avoid use after free
A use after free may occur if efi_load_image() fails.
Heinrich Schuchardt (3): efi_loader: return status from efi_setup_loaded_image() efi_loader: new function efi_delete_handle() efi_loader: error handling in efi_load_image()
include/efi_loader.h | 10 +- lib/efi_loader/efi_boottime.c | 228 ++++++++++++++++++++++-------------------- 2 files changed, 129 insertions(+), 109 deletions(-)
efi_setup_loaded_image() should return an error code indicating if an error has occurred.
An error occurs if a protocol cannot be installed.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 8 +++++--- lib/efi_loader/efi_boottime.c | 11 +++++++---- 2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index c0caabddb1..78237f14ae 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -249,9 +249,11 @@ uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, int efi_memory_init(void); /* Adds new or overrides configuration table entry to the system table */ efi_status_t efi_install_configuration_table(const efi_guid_t *guid, void *table); -void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *obj, - struct efi_device_path *device_path, - struct efi_device_path *file_path); +/* Sets up a loaded image */ +efi_status_t efi_setup_loaded_image( + struct efi_loaded_image *info, struct efi_object *obj, + struct efi_device_path *device_path, + struct efi_device_path *file_path); efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, void **buffer);
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index a37fb25638..a9ba1ac394 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1170,10 +1170,12 @@ static efi_status_t EFIAPI efi_install_configuration_table_ext(efi_guid_t *guid, * @obj internal object associated with the loaded image * @device_path device path of the loaded image * @file_path file path of the loaded image + * @return status code */ -void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *obj, - struct efi_device_path *device_path, - struct efi_device_path *file_path) +efi_status_t efi_setup_loaded_image( + struct efi_loaded_image *info, struct efi_object *obj, + struct efi_device_path *device_path, + struct efi_device_path *file_path) { efi_status_t ret;
@@ -1213,9 +1215,10 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob if (ret != EFI_SUCCESS) goto failure;
- return; + return ret; failure: printf("ERROR: Failure to install protocols for loaded image\n"); + return ret; }
/*
Provide a function to remove a handle from the object list after removing all protocols.
To avoid forward declarations other functions have to move up in the coding.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 2 + lib/efi_loader/efi_boottime.c | 186 +++++++++++++++++++++++------------------- 2 files changed, 102 insertions(+), 86 deletions(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 78237f14ae..6185055e78 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -190,6 +190,8 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path); void efi_add_handle(struct efi_object *obj); /* Create handle */ efi_status_t efi_create_handle(void **handle); +/* Delete handle */ +void efi_delete_handle(struct efi_object *obj); /* Call this to validate a handle and find the EFI object for it */ struct efi_object *efi_search_obj(const void *handle); /* Find a protocol on a handle */ diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index a9ba1ac394..7c8f3134d1 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -359,6 +359,106 @@ efi_status_t efi_create_handle(void **handle) return r; }
+/* + * Find a protocol on a handle. + * + * @handle handle + * @protocol_guid GUID of the protocol + * @handler reference to the protocol + * @return status code + */ +efi_status_t efi_search_protocol(const void *handle, + const efi_guid_t *protocol_guid, + struct efi_handler **handler) +{ + struct efi_object *efiobj; + struct list_head *lhandle; + + if (!handle || !protocol_guid) + return EFI_INVALID_PARAMETER; + efiobj = efi_search_obj(handle); + if (!efiobj) + return EFI_INVALID_PARAMETER; + list_for_each(lhandle, &efiobj->protocols) { + struct efi_handler *protocol; + + protocol = list_entry(lhandle, struct efi_handler, link); + if (!guidcmp(protocol->guid, protocol_guid)) { + if (handler) + *handler = protocol; + return EFI_SUCCESS; + } + } + return EFI_NOT_FOUND; +} + +/* + * Delete protocol from a handle. + * + * @handle handle from which the protocol shall be deleted + * @protocol GUID of the protocol to be deleted + * @protocol_interface interface of the protocol implementation + * @return status code + */ +efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol, + void *protocol_interface) +{ + struct efi_handler *handler; + efi_status_t ret; + + ret = efi_search_protocol(handle, protocol, &handler); + if (ret != EFI_SUCCESS) + return ret; + if (guidcmp(handler->guid, protocol)) + return EFI_INVALID_PARAMETER; + list_del(&handler->link); + free(handler); + return EFI_SUCCESS; +} + +/* + * Delete all protocols from a handle. + * + * @handle handle from which the protocols shall be deleted + * @return status code + */ +efi_status_t efi_remove_all_protocols(const void *handle) +{ + struct efi_object *efiobj; + struct list_head *lhandle; + struct list_head *pos; + + efiobj = efi_search_obj(handle); + if (!efiobj) + return EFI_INVALID_PARAMETER; + list_for_each_safe(lhandle, pos, &efiobj->protocols) { + struct efi_handler *protocol; + efi_status_t ret; + + protocol = list_entry(lhandle, struct efi_handler, link); + + ret = efi_remove_protocol(handle, protocol->guid, + protocol->protocol_interface); + if (ret != EFI_SUCCESS) + return ret; + } + return EFI_SUCCESS; +} + +/* + * Delete handle. + * + * @handle handle to delete + */ +void efi_delete_handle(struct efi_object *obj) +{ + if (!obj) + return; + efi_remove_all_protocols(obj->handle); + list_del(&obj->link); + free(obj); +} + /* * Our event capabilities are very limited. Only a small limited * number of events is allowed to coexist. @@ -717,39 +817,6 @@ struct efi_object *efi_search_obj(const void *handle) return NULL; }
-/* - * Find a protocol on a handle. - * - * @handle handle - * @protocol_guid GUID of the protocol - * @handler reference to the protocol - * @return status code - */ -efi_status_t efi_search_protocol(const void *handle, - const efi_guid_t *protocol_guid, - struct efi_handler **handler) -{ - struct efi_object *efiobj; - struct list_head *lhandle; - - if (!handle || !protocol_guid) - return EFI_INVALID_PARAMETER; - efiobj = efi_search_obj(handle); - if (!efiobj) - return EFI_INVALID_PARAMETER; - list_for_each(lhandle, &efiobj->protocols) { - struct efi_handler *protocol; - - protocol = list_entry(lhandle, struct efi_handler, link); - if (!guidcmp(protocol->guid, protocol_guid)) { - if (handler) - *handler = protocol; - return EFI_SUCCESS; - } - } - return EFI_NOT_FOUND; -} - /* * Install new protocol on a handle. * @@ -780,59 +847,6 @@ efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol, return EFI_SUCCESS; }
-/* - * Delete protocol from a handle. - * - * @handle handle from which the protocol shall be deleted - * @protocol GUID of the protocol to be deleted - * @protocol_interface interface of the protocol implementation - * @return status code - */ -efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol, - void *protocol_interface) -{ - struct efi_handler *handler; - efi_status_t ret; - - ret = efi_search_protocol(handle, protocol, &handler); - if (ret != EFI_SUCCESS) - return ret; - if (guidcmp(handler->guid, protocol)) - return EFI_INVALID_PARAMETER; - list_del(&handler->link); - free(handler); - return EFI_SUCCESS; -} - -/* - * Delete all protocols from a handle. - * - * @handle handle from which the protocols shall be deleted - * @return status code - */ -efi_status_t efi_remove_all_protocols(const void *handle) -{ - struct efi_object *efiobj; - struct list_head *lhandle; - struct list_head *pos; - - efiobj = efi_search_obj(handle); - if (!efiobj) - return EFI_INVALID_PARAMETER; - list_for_each_safe(lhandle, pos, &efiobj->protocols) { - struct efi_handler *protocol; - efi_status_t ret; - - protocol = list_entry(lhandle, struct efi_handler, link); - - ret = efi_remove_protocol(handle, protocol->guid, - protocol->protocol_interface); - if (ret != EFI_SUCCESS) - return ret; - } - return EFI_SUCCESS; -} - /* * Install protocol interface. *
If a failure occurs when trying to load an image, it is insufficient to free() the EFI object. We must remove it from the object list, too. Otherwise a use after free will occur the next time we iterate over the object list.
Furthermore errors in setting up the image should be handled.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 7c8f3134d1..b90bd0b426 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1308,6 +1308,7 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy, { struct efi_loaded_image *info; struct efi_object *obj; + efi_status_t ret;
EFI_ENTRY("%d, %p, %p, %p, %ld, %p", boot_policy, parent_image, file_path, source_buffer, source_size, image_handle); @@ -1317,41 +1318,39 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy,
if (!source_buffer) { struct efi_device_path *dp, *fp; - efi_status_t ret;
ret = efi_load_image_from_path(file_path, &source_buffer); - if (ret != EFI_SUCCESS) { - free(info); - free(obj); - return EFI_EXIT(ret); - } - + if (ret != EFI_SUCCESS) + goto failure; /* * split file_path which contains both the device and * file parts: */ efi_dp_split_file_path(file_path, &dp, &fp); - - efi_setup_loaded_image(info, obj, dp, fp); + ret = efi_setup_loaded_image(info, obj, dp, fp); + if (ret != EFI_SUCCESS) + goto failure; } else { /* In this case, file_path is the "device" path, ie. * something like a HARDWARE_DEVICE:MEMORY_MAPPED */ - efi_setup_loaded_image(info, obj, file_path, NULL); + ret = efi_setup_loaded_image(info, obj, file_path, NULL); + if (ret != EFI_SUCCESS) + goto failure; } - info->reserved = efi_load_pe(source_buffer, info); if (!info->reserved) { - free(info); - free(obj); - return EFI_EXIT(EFI_UNSUPPORTED); + ret = EFI_UNSUPPORTED; + goto failure; } - info->system_table = &systab; info->parent_handle = parent_image; *image_handle = obj->handle; - return EFI_EXIT(EFI_SUCCESS); +failure: + free(info); + efi_delete_handle(obj); + return EFI_EXIT(ret); }
/*
participants (1)
-
Heinrich Schuchardt