* [PATCH] efi_loader: EFI TCG2 free efi memory on protocol failure
@ 2021-03-24 14:23 Ilias Apalodimas
2021-03-25 10:10 ` Heinrich Schuchardt
0 siblings, 1 reply; 4+ messages in thread
From: Ilias Apalodimas @ 2021-03-24 14:23 UTC (permalink / raw)
To: u-boot
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 <ilias.apalodimas@linaro.org>
---
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();
+ if (ret != EFI_SUCCESS)
+ goto out;
+ return EFI_SUCCESS;
out:
+ efi_free_pool(event_log.buffer);
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:
+ efi_free_pool(event_log.final_buffer);
+ efi_free_pool(event_log.buffer);
return ret;
}
--
2.31.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH] efi_loader: EFI TCG2 free efi memory on protocol failure
2021-03-24 14:23 [PATCH] efi_loader: EFI TCG2 free efi memory on protocol failure Ilias Apalodimas
@ 2021-03-25 10:10 ` Heinrich Schuchardt
2021-03-25 10:31 ` Ilias Apalodimas
0 siblings, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2021-03-25 10:10 UTC (permalink / raw)
To: u-boot
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 <ilias.apalodimas@linaro.org>
> ---
> 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;
> }
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] efi_loader: EFI TCG2 free efi memory on protocol failure
2021-03-25 10:10 ` Heinrich Schuchardt
@ 2021-03-25 10:31 ` Ilias Apalodimas
0 siblings, 0 replies; 4+ messages in thread
From: Ilias Apalodimas @ 2021-03-25 10:31 UTC (permalink / raw)
To: u-boot
On Thu, Mar 25, 2021 at 11:10:26AM +0100, Heinrich Schuchardt wrote:
> 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 <ilias.apalodimas@linaro.org>
> > ---
> > 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?
>
I am not sure I undetstand you here.
create_final_event() frees event_log.final_buffer if installing the config
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?
Yea sure
>
> > 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?
I guess I could gather all the frees/uninstalles in one function and call that
explicitly
>
> Best regards
>
> Heinrich
>
> > + efi_free_pool(event_log.final_buffer);
> > + efi_free_pool(event_log.buffer);
> > return ret;
> > }
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] efi_loader: EFI TCG2 free efi memory on protocol failure
@ 2021-03-25 11:31 Ilias Apalodimas
0 siblings, 0 replies; 4+ messages in thread
From: Ilias Apalodimas @ 2021-03-25 11:31 UTC (permalink / raw)
To: u-boot
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 <ilias.apalodimas@linaro.org>
---
changes since v2:
- remove the table using efi_install_configuration_table with NULL
changes since v1:
- remove the installed config table as well
- gather all cleanups in a single function and call that instead
lib/efi_loader/efi_tcg2.c | 40 +++++++++++++++++++++++++++++++--------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 797d6eb134f6..35ae8ed5962e 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -957,6 +957,23 @@ out:
return ret;
}
+/**
+ * tcg2_uninit - remove the final event table and free efi memory on failures
+ */
+void tcg2_uninit(void)
+{
+ efi_status_t ret;
+
+ ret = efi_install_configuration_table(&efi_guid_final_events, NULL);
+ if (ret != EFI_SUCCESS)
+ log_err("Failed to delete final events config table\n");
+
+ efi_free_pool(event_log.buffer);
+ event_log.buffer = NULL;
+ efi_free_pool(event_log.final_buffer);
+ event_log.final_buffer = NULL;
+}
+
/**
* create_final_event() - Create the final event and install the config
* defined by the TCG EFI spec
@@ -983,10 +1000,6 @@ static efi_status_t create_final_event(void)
event_log.final_pos = sizeof(*final_event);
ret = efi_install_configuration_table(&efi_guid_final_events,
final_event);
- if (ret != EFI_SUCCESS)
- goto out;
-
- return EFI_SUCCESS;
out:
return ret;
}
@@ -1041,8 +1054,12 @@ static efi_status_t efi_init_event_log(void)
event_log.last_event_size = event_log.pos;
ret = create_final_event();
+ if (ret != EFI_SUCCESS)
+ goto out;
+ return EFI_SUCCESS;
out:
+ tcg2_uninit();
return ret;
}
@@ -1055,23 +1072,30 @@ 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:
+ tcg2_uninit();
return ret;
}
--
2.31.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-25 11:31 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-24 14:23 [PATCH] efi_loader: EFI TCG2 free efi memory on protocol failure Ilias Apalodimas
2021-03-25 10:10 ` Heinrich Schuchardt
2021-03-25 10:31 ` Ilias Apalodimas
-- strict thread matches above, loose matches on Subject: below --
2021-03-25 11:31 Ilias Apalodimas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox