[U-Boot] [PATCH 0/4] efi_loader: check parameters

The patches in this series provide a more rigorous checking of function parameters.
Heinrich Schuchardt (4): efi_loader: check parameters of CreateEvent efi_loader: check parameters in memory allocation efi_loader: check parameters of GetMemoryMap efi_loader: check map_key in ExitBootServices
include/efi_loader.h | 3 +++ lib/efi_loader/efi_boottime.c | 39 ++++++++++++++++++++++++++++++++--- lib/efi_loader/efi_memory.c | 37 ++++++++++++++++++++++++--------- 3 files changed, 66 insertions(+), 13 deletions(-)

Rigorously check the TPL level and the event type.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_boottime.c | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 298e6c3bbb..396f28c570 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -192,6 +192,25 @@ static void efi_queue_event(struct efi_event *event, bool check_tpl) event->is_queued = false; }
+/** + * is_valid_tpl - check if the task priority level is valid + * + * @tpl: TPL level to check + * ReturnValue: status code + */ +efi_status_t is_valid_tpl(efi_uintn_t tpl) +{ + switch (tpl) { + case TPL_APPLICATION: + case TPL_CALLBACK: + case TPL_NOTIFY: + case TPL_HIGH_LEVEL: + return EFI_SUCCESS; + default: + return EFI_INVALID_PARAMETER; + } +} + /** * efi_signal_event - signal an EFI event * @@ -591,11 +610,21 @@ efi_status_t efi_create_event(uint32_t type, efi_uintn_t notify_tpl, if (event == NULL) return EFI_INVALID_PARAMETER;
- if ((type & EVT_NOTIFY_SIGNAL) && (type & EVT_NOTIFY_WAIT)) + switch (type) { + case 0: + case EVT_TIMER: + case EVT_NOTIFY_SIGNAL: + case EVT_TIMER | EVT_NOTIFY_SIGNAL: + case EVT_NOTIFY_WAIT: + case EVT_TIMER | EVT_NOTIFY_WAIT: + case EVT_SIGNAL_EXIT_BOOT_SERVICES: + case EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE: + break; + default: return EFI_INVALID_PARAMETER; + }
- if ((type & (EVT_NOTIFY_SIGNAL | EVT_NOTIFY_WAIT)) && - notify_function == NULL) + if (is_valid_tpl(notify_tpl) != EFI_SUCCESS) return EFI_INVALID_PARAMETER;
evt = calloc(1, sizeof(struct efi_event));

If no pointer is provided throw an error.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_memory.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 86edfc95f4..f5aecd4b41 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -293,6 +293,9 @@ efi_status_t efi_allocate_pages(int type, int memory_type, efi_status_t r = EFI_SUCCESS; uint64_t addr;
+ if (!memory) + return EFI_INVALID_PARAMETER; + switch (type) { case EFI_ALLOCATE_ANY_PAGES: /* Any page */ @@ -386,6 +389,9 @@ efi_status_t efi_allocate_pool(int pool_type, efi_uintn_t size, void **buffer) u64 num_pages = (size + sizeof(struct efi_pool_allocation) + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
+ if (!buffer) + return EFI_INVALID_PARAMETER; + if (size == 0) { *buffer = NULL; return EFI_SUCCESS;

Check the parameters of boottime service GetMemoryMap(). Return EFI_INVALID_PARAMETER where required by the UEFI spec.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_memory.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index f5aecd4b41..bad8704269 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -453,6 +453,9 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, struct list_head *lhandle; efi_uintn_t provided_map_size = *memory_map_size;
+ if (!memory_map_size) + return EFI_INVALID_PARAMETER; + list_for_each(lhandle, &efi_mem) map_entries++;
@@ -463,6 +466,9 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, if (provided_map_size < map_size) return EFI_BUFFER_TOO_SMALL;
+ if (!memory_map) + return EFI_INVALID_PARAMETER; + if (descriptor_size) *descriptor_size = sizeof(struct efi_mem_desc);
@@ -470,19 +476,18 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, *descriptor_version = EFI_MEMORY_DESCRIPTOR_VERSION;
/* Copy list into array */ - if (memory_map) { - /* Return the list in ascending order */ - memory_map = &memory_map[map_entries - 1]; - list_for_each(lhandle, &efi_mem) { - struct efi_mem_list *lmem; + /* Return the list in ascending order */ + memory_map = &memory_map[map_entries - 1]; + list_for_each(lhandle, &efi_mem) { + struct efi_mem_list *lmem;
- lmem = list_entry(lhandle, struct efi_mem_list, link); - *memory_map = lmem->desc; - memory_map--; - } + lmem = list_entry(lhandle, struct efi_mem_list, link); + *memory_map = lmem->desc; + memory_map--; }
- *map_key = 0; + if (map_key) + *map_key = 0;
return EFI_SUCCESS; }

The UEFI spec requires that the memory map key is checked in ExitBootServices().
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_loader.h | 3 +++ lib/efi_loader/efi_boottime.c | 4 ++++ lib/efi_loader/efi_memory.c | 8 +++++++- 3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/include/efi_loader.h b/include/efi_loader.h index 628dd801a8..0ad22eefa7 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -85,6 +85,9 @@ const char *__efi_nesting_dec(void); #define EFI_CACHELINE_SIZE 128 #endif
+/* Key identifying current memory map */ +extern efi_uintn_t efi_memory_map_key; + extern struct efi_runtime_services efi_runtime_services; extern struct efi_system_table systab;
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 396f28c570..105e80bb52 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1837,6 +1837,10 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
EFI_ENTRY("%p, %ld", image_handle, map_key);
+ /* Check that the caller has read the current memory map */ + if (map_key != efi_memory_map_key) + return EFI_INVALID_PARAMETER; + /* Make sure that notification functions are not called anymore */ efi_tpl = TPL_HIGH_LEVEL;
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index bad8704269..967c3f733e 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -15,6 +15,8 @@
DECLARE_GLOBAL_DATA_PTR;
+efi_uintn_t efi_memory_map_key; + struct efi_mem_list { struct list_head link; struct efi_mem_desc desc; @@ -160,9 +162,13 @@ uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, debug("%s: 0x%" PRIx64 " 0x%" PRIx64 " %d %s\n", __func__, start, pages, memory_type, overlap_only_ram ? "yes" : "no");
+ if (memory_type >= EFI_MAX_MEMORY_TYPE) + return EFI_INVALID_PARAMETER; + if (!pages) return start;
+ ++efi_memory_map_key; newlist = calloc(1, sizeof(*newlist)); newlist->desc.type = memory_type; newlist->desc.physical_start = start; @@ -487,7 +493,7 @@ efi_status_t efi_get_memory_map(efi_uintn_t *memory_map_size, }
if (map_key) - *map_key = 0; + *map_key = efi_memory_map_key;
return EFI_SUCCESS; }
participants (1)
-
Heinrich Schuchardt