From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heinrich Schuchardt Date: Thu, 25 Mar 2021 11:10:26 +0100 Subject: [PATCH] efi_loader: EFI TCG2 free efi memory on protocol failure In-Reply-To: <20210324142353.1216042-1-ilias.apalodimas@linaro.org> References: <20210324142353.1216042-1-ilias.apalodimas@linaro.org> Message-ID: <28d20122-e2d4-33a8-bee2-f08895ceaeed@gmx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 24.03.21 15:23, Ilias Apalodimas wrote: > Current code doesn't free the efi allocated memory in case the protocol > failed to install > > Fixes: c8d0fd582576 ("efi_loader: Introduce eventlog support for TCG2_PROTOCOL") > Signed-off-by: Ilias Apalodimas > --- > lib/efi_loader/efi_tcg2.c | 21 +++++++++++++++++---- > 1 file changed, 17 insertions(+), 4 deletions(-) > > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index 797d6eb134f6..02de63808f9a 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -988,6 +988,7 @@ static efi_status_t create_final_event(void) > > return EFI_SUCCESS; > out: > + efi_free_pool(event_log.final_buffer); > return ret; > } > > @@ -1041,8 +1042,12 @@ static efi_status_t efi_init_event_log(void) > event_log.last_event_size = event_log.pos; > > ret = create_final_event(); Shouldn't create_final_event() should free event_log.final_buffer if efi_install_configuration_table() fails? > + if (ret != EFI_SUCCESS) > + goto out; > > + return EFI_SUCCESS; > out: > + efi_free_pool(event_log.buffer); We must set event_log.buffer to NULL to avoid double free? > return ret; > } > > @@ -1055,23 +1060,31 @@ out: > */ > efi_status_t efi_tcg2_register(void) > { > - efi_status_t ret; > + efi_status_t ret = EFI_SUCCESS; > struct udevice *dev; > > ret = platform_get_tpm2_device(&dev); > if (ret != EFI_SUCCESS) { > log_warning("Unable to find TPMv2 device\n"); > - return EFI_SUCCESS; > + ret = EFI_SUCCESS; > + goto out; > } > > ret = efi_init_event_log(); > if (ret != EFI_SUCCESS) > - return ret; > + goto fail; > > ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol, > (void *)&efi_tcg2_protocol); > - if (ret != EFI_SUCCESS) > + if (ret != EFI_SUCCESS) { > log_err("Cannot install EFI_TCG2_PROTOCOL\n"); > + goto fail; > + } > > +out: > + return ret; > +fail: In create_final_event() you have installed a configuration table EFI_TCG2_FINAL_EVENTS_TABLE_GUID. You should not free the memory of the configuration table without uninstalling the table. Should we create separate uninit function which takes care of the logic? Best regards Heinrich > + efi_free_pool(event_log.final_buffer); > + efi_free_pool(event_log.buffer); > return ret; > } >