Hi Heinrich,
On Fri, 23 Nov 2018 at 00:53, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/22/18 9:50 PM, Simon Glass wrote:
Hi Heinrich,
On Mon, 19 Nov 2018 at 12:41, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/15/18 12:11 AM, Simon Glass wrote:
This function can be used from do_bootefi_exec() so that we use mostly the same code for a normal EFI application and an EFI test.
Rename the function and use it in both places.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v14:
- Go back to the horrible long variable names
- Hopefully correct error paths in do_bootefi_exec()
Changes in v13:
- Drop 'efi_loader: Drop setup_ok' as we have an existing patch for that
- Drop patches previously applied
Changes in v12: None Changes in v11:
- Drop patches previously applied
Changes in v9: None Changes in v7:
- Drop patch "efi: Init the 'rows' and 'cols' variables"
- Drop patches previous applied
Changes in v5:
- Rebase to master
Changes in v4:
- Rebase to master
Changes in v3:
- Add new patch to rename bootefi_test_finish() to bootefi_run_finish()
cmd/bootefi.c | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index ab7ada9f2d6..a627f689f95 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -350,6 +350,20 @@ static efi_status_t bootefi_run_prepare(const char *load_options_path, return 0; }
+/**
- bootefi_run_finish() - finish up after running an EFI test
- @loaded_image_info: Pointer to a struct which holds the loaded image info
- @image_objj: Pointer to a struct which holds the loaded image object
- */
+static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj,
struct efi_loaded_image *loaded_image_info)+{
efi_restore_gd();free(loaded_image_info->load_options);efi_delete_handle(&image_obj->header);+}
/**
- do_bootefi_exec() - execute EFI binary
@@ -390,11 +404,11 @@ static efi_status_t do_bootefi_exec(void *efi, */ ret = efi_create_handle(&mem_handle); if (ret != EFI_SUCCESS)
goto exit;You are leaking a device path.
I cannot see anywhere where these are freed. Are you sure this is supported?
According to the UEFI standard handles are deleted when the last protocol is removed. See efi_uninstall_protocol_interface(). I am aware that our cleanup after running binaries is still incomplete.
In this function you could use efi_delete_handle() in the error handler.
Actually the code is pretty impenetrable.
efi_delete_handle() takes an efi_handle_t which, confusingly, is a pointer to an efi_object. Also we should not typedef structs in U-Boot, particularly not with a different name.
In this case efi_create_handle() failed so we don't need to delete that. We need to delete the efi_dp_from_mem() object. But the type of that is struct efi_device_path, so I cannot call efi_delete_handle() with it.
My suggestion is that if these patches land, you take a look at it and try to figure out how to finish improving the error handling in this function (which I started on patch v14 of this series). If I had my way this code would be much, much simpler to read.
Regards, Simon