diff mbox series

efi_loader: Check for pointers before appending to the eventlog

Message ID 20211123162425.657178-1-ilias.apalodimas@linaro.org
State New
Headers show
Series efi_loader: Check for pointers before appending to the eventlog | expand

Commit Message

Ilias Apalodimas Nov. 23, 2021, 4:24 p.m. UTC
The TCG protocol returns EFI_SUCCESS even when it fails to install.
If the protocol is not installed there should be no calls to
tcg2_agile_log_append().  However there are calls to this function
occurring outside the TCG protocol invocation (e.g tcg2_measure_pe_image).

Let's deal with this and shield the code from crashing while complaining,
so buggy code can be spotted and fixed.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
Heinrich this is rebased on top of https://lore.kernel.org/u-boot/20211123115335.125252-1-ruchika.gupta@linaro.org/
 lib/efi_loader/efi_tcg2.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Ilias Apalodimas Nov. 25, 2021, 11:05 a.m. UTC | #1
Hi Heinrich,

On Tue, 23 Nov 2021 at 18:24, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> The TCG protocol returns EFI_SUCCESS even when it fails to install.
> If the protocol is not installed there should be no calls to
> tcg2_agile_log_append().  However there are calls to this function
> occurring outside the TCG protocol invocation (e.g tcg2_measure_pe_image).
>
> Let's deal with this and shield the code from crashing while complaining,
> so buggy code can be spotted and fixed.
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
> Heinrich this is rebased on top of https://lore.kernel.org/u-boot/20211123115335.125252-1-ruchika.gupta@linaro.org/
>  lib/efi_loader/efi_tcg2.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 133fe8291a92..57a1f37695f6 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -311,6 +311,17 @@ static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
>         struct efi_tcg2_final_events_table *final_event;
>         efi_status_t ret = EFI_SUCCESS;
>
> +       /*
> +        * This should never happen.  This function should only be invoked if
> +        * the TCG2 protocol has been installed.  However since we always
> +        * return EFI_SUCCESS from efi_tcg2_register shield callers against
> +        * crashing and complain
> +        */
> +       if (!event_log.final_buffer || !event_log.buffer) {
> +               log_err("EFI TCG2 protocol not installed\n");
> +               return EFI_INVALID_PARAMETER;
> +       }
> +
>         /* if ExitBootServices hasn't been called update the normal log */
>         if (!event_log.ebs_called) {
>                 if (event_log.truncated ||
> --
> 2.34.0
>

With the latest patch sent from Ruchika [1], this doesn't work
properly anymore.  I'll rebase/fix it once that one is applied.

[1] https://lore.kernel.org/u-boot/CAC_iWjLPfwos2_x9e536jxwufQ3g7JhuzyEBf8s70MBEqfOadA@mail.gmail.com/

Regards
/Ilias
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 133fe8291a92..57a1f37695f6 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -311,6 +311,17 @@  static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
 	struct efi_tcg2_final_events_table *final_event;
 	efi_status_t ret = EFI_SUCCESS;
 
+	/*
+	 * This should never happen.  This function should only be invoked if
+	 * the TCG2 protocol has been installed.  However since we always
+	 * return EFI_SUCCESS from efi_tcg2_register shield callers against
+	 * crashing and complain
+	 */
+	if (!event_log.final_buffer || !event_log.buffer) {
+		log_err("EFI TCG2 protocol not installed\n");
+		return EFI_INVALID_PARAMETER;
+	}
+
 	/* if ExitBootServices hasn't been called update the normal log */
 	if (!event_log.ebs_called) {
 		if (event_log.truncated ||