[U-Boot] [RFC] efi_loader: memory leak in bootefi

[Alexander Graf]

Hi Heinrich,

On 07/06/2017 05:43 PM, Heinrich Schuchardt wrote:
> Hello Alex,
>
> in bootefi.c do_bootefi_exec we build the efi_obj_list. This includes
> allocation of memory for some handlers (e.g. in efi_gop_register).
>
> After returning from the efi appliation we have no clean up code to
> release these objects.
>
> We do not remove the list elements from efi_obj_list.
>
> Furthermore we rely on a lot of static initializations e.g. for
> protocols. We know that this data may be changed by the application but
> we do not care to restore the original state.
>
> So if an application registers protocols and exits without unregistering
> we will offer invalid function pointers to the next efi application to
> be started.
>
> I suggest the following:
>
> In structure struct efi_object we add a function pointer to a clean-up
> function which takes as only argument the efi_object:
>
> struct efi_object {
>          struct list_head link;
>          struct efi_handler protocols[4];
> 	void (*cleanup)(struct efi_object *obj);
>          void *handle;
> };
>
> A clean up function may look like this:
> void efi_gop_cleanup(struct efi_object *obj) {
> 	free(obj);
> }
>
> When returning from the EFI application we would work our way from the
> tail to the head of the object list:
>
> while (list is not empty) {
> 	Remove last object from list.
> 	Call cleanup function of removed object.
> }
>
> We should get rid of the static structures loaded_image_info_obj
> and boot_efi_obj. Let's use register functions with calloc here too.
>
> Would you agree to this design?

The basic long-term plan was to instead integrate the efi object model 
with the DM object model and slowly make every DM object potentially an 
EFI object. That way we would only initialize the objects once on bootup.

The fact that then an application may modify pointers is actually 
intended by UEFI. A driver may want to intercept what another driver 
does or route things through itself. It works the same way in edk2 as 
far as I can tell :).


Alex