diff mbox series

efi_loader: fix FinalEvents table if an EFI uses GetEventLog

Message ID 20211117091041.93577-1-ilias.apalodimas@linaro.org
State Superseded
Headers show
Series efi_loader: fix FinalEvents table if an EFI uses GetEventLog | expand

Commit Message

Ilias Apalodimas Nov. 17, 2021, 9:10 a.m. UTC
As described in the TCG spec [1] in sections 7.1.1 and 7.1.2 the FinalEvent
table should include events after GetEventLog has been called.  This
currently works for us as long as the kernel is the only EFI application
calling that.  Specifically we only implement what's described in 7.1.1.

So refactor the code a bit and support EFI application calling GetEventLog.
Events will now be logged in both the EventLog and FinalEvent table as long
as ExitBootServices haven't been invoked.

[1] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 lib/efi_loader/efi_tcg2.c | 90 ++++++++++++++++++++++++++-------------
 1 file changed, 61 insertions(+), 29 deletions(-)

Comments

Heinrich Schuchardt Nov. 17, 2021, 10:01 a.m. UTC | #1
On 11/17/21 10:10, Ilias Apalodimas wrote:
> As described in the TCG spec [1] in sections 7.1.1 and 7.1.2 the FinalEvent
> table should include events after GetEventLog has been called.  This
> currently works for us as long as the kernel is the only EFI application
> calling that.  Specifically we only implement what's described in 7.1.1.
>
> So refactor the code a bit and support EFI application calling GetEventLog.
> Events will now be logged in both the EventLog and FinalEvent table as long
> as ExitBootServices haven't been invoked.
>
> [1] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   lib/efi_loader/efi_tcg2.c | 90 ++++++++++++++++++++++++++-------------
>   1 file changed, 61 insertions(+), 29 deletions(-)
>
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 189e4a5ba59c..215f4b2b04b8 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -34,6 +34,7 @@ struct event_log_buffer {
>   	size_t final_pos; /* final events config table position */
>   	size_t last_event_size;
>   	bool get_event_called;
> +	bool ebs_called;

Please, add documentation for the elements of the structure. Not every
reader will be aware of ebs_called referring to ExitBootServices().

>   	bool truncated;
>   };
>
> @@ -186,39 +187,29 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
>   	return EFI_SUCCESS;
>   }
>
> -/* tcg2_agile_log_append - Append an agile event to out eventlog
> +/* put_event - Append an agile event to an eventlog
>    *
>    * @pcr_index:		PCR index
>    * @event_type:		type of event added
>    * @digest_list:	list of digest algorithms to add
>    * @size:		size of event
>    * @event:		event to add
> + * @log:		log buffer to append the event
>    *
> - * @Return: status code
>    */
> -static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
> -					  struct tpml_digest_values *digest_list,
> -					  u32 size, u8 event[])
> +static void put_event(u32 pcr_index, u32 event_type,
> +		      struct tpml_digest_values *digest_list, u32 size,
> +		      u8 event[], void *log)
>   {
> -	void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
>   	size_t pos;
>   	size_t i;
>   	u32 event_size;
>
> -	if (event_log.get_event_called)
> -		log = (void *)((uintptr_t)event_log.final_buffer +
> -			       event_log.final_pos);
> -
>   	/*
>   	 * size refers to the length of event[] only, we need to check against
>   	 * the final tcg_pcr_event2 size
>   	 */
>   	event_size = size + tcg_event_final_size(digest_list);
> -	if (event_log.pos + event_size > TPM2_EVENT_LOG_SIZE ||
> -	    event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE) {
> -		event_log.truncated = true;
> -		return EFI_VOLUME_FULL;
> -	}
>
>   	put_unaligned_le32(pcr_index, log);
>   	pos = offsetof(struct tcg_pcr_event2, event_type);
> @@ -242,25 +233,64 @@ static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
>   	memcpy((void *)((uintptr_t)log + pos), event, size);
>   	pos += size;
>
> -	/* make sure the calculated buffer is what we checked against */
> +	/*
> +	 * make sure the calculated buffer is what we checked against
> +	 * This check should never fail.  It checks the code above is
> +	 * calculating the right length for the event we are adding
> +	 * */
>   	if (pos != event_size)
> -		return EFI_INVALID_PARAMETER;
> +		log_err("Appending to the EventLog failed\n");
>
> -	/* if GetEventLog hasn't been called update the normal log */
> -	if (!event_log.get_event_called) {
> -		event_log.pos += pos;
> -		event_log.last_event_size = pos;
> -	} else {
> -	/* if GetEventLog has been called update config table log */
> -		struct efi_tcg2_final_events_table *final_event;
> +}
>
> -		final_event =
> -			(struct efi_tcg2_final_events_table *)(event_log.final_buffer);
> -		final_event->number_of_events++;
> -		event_log.final_pos += pos;
> +/* tcg2_agile_log_append - Append an agile event to an eventlog
> + *
> + * @pcr_index:		PCR index
> + * @event_type:		type of event added
> + * @digest_list:	list of digest algorithms to add
> + * @size:		size of event
> + * @event:		event to add
> + * @log:		log buffer to append the event
> + *
> + * @Return: status code
> + */
> +static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
> +					  struct tpml_digest_values *digest_list,
> +					  u32 size, u8 event[])
> +{
> +	void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
> +	u32 event_size = size + tcg_event_final_size(digest_list);
> +	struct efi_tcg2_final_events_table *final_event;
> +	efi_status_t ret = EFI_SUCCESS;
> +
> +	/* if ExitBootServices hasn't been called update the normal log */
> +	if (!event_log.ebs_called) {
> +		if (event_log.truncated ||
> +		    event_log.pos + event_size > TPM2_EVENT_LOG_SIZE) {
> +			event_log.truncated = true;
> +			return EFI_VOLUME_FULL;
> +		}
> +		put_event(pcr_index, event_type, digest_list, size, event, log);
> +		event_log.pos += event_size;
> +		event_log.last_event_size = event_size;
>   	}
>
> -	return EFI_SUCCESS;
> +	if (!event_log.get_event_called)
> +		return ret;
> +
> +	/* if GetEventLog has been called update FinalEventLog as well */
> +	if (event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE)
> +		return EFI_VOLUME_FULL;
> +
> +	log = (void *)((uintptr_t)event_log.final_buffer + event_log.final_pos);
> +	put_event(pcr_index, event_type, digest_list, size, event, log);
> +
> +	final_event =
> +		(struct efi_tcg2_final_events_table *)event_log.final_buffer;
> +	final_event->number_of_events++;
> +	event_log.final_pos += event_size;
> +
> +	return ret;
>   }
>
>   /**
> @@ -1303,6 +1333,7 @@ static efi_status_t efi_init_event_log(void)
>   	event_log.pos = 0;
>   	event_log.last_event_size = 0;
>   	event_log.get_event_called = false;
> +	event_log.ebs_called = false;
>   	event_log.truncated = false;
>
>   	/*
> @@ -1792,6 +1823,7 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
>
>   	EFI_ENTRY("%p, %p", event, context);

This is called in EFI_EVENT_GROUP_EXIT_BOOT_SERVICES.

This implies that whatever happens in
EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES is measured normally. Does
this conform to the TCG2 standard?

>
> +	event_log.ebs_called = true;

How should a failed call to ExitBootServices() be handled?
E.g. invalid memory map?

Best regards

Heinrich

>   	ret = platform_get_tpm2_device(&dev);
>   	if (ret != EFI_SUCCESS)
>   		goto out;
>
Ilias Apalodimas Nov. 17, 2021, 10:12 a.m. UTC | #2
Hi Heinrich,

On Wed, Nov 17, 2021 at 11:01:55AM +0100, Heinrich Schuchardt wrote:
> On 11/17/21 10:10, Ilias Apalodimas wrote:
> > As described in the TCG spec [1] in sections 7.1.1 and 7.1.2 the FinalEvent
> > table should include events after GetEventLog has been called.  This
> > currently works for us as long as the kernel is the only EFI application
> > calling that.  Specifically we only implement what's described in 7.1.1.
> > 
> > So refactor the code a bit and support EFI application calling GetEventLog.
> > Events will now be logged in both the EventLog and FinalEvent table as long
> > as ExitBootServices haven't been invoked.
> > 
> > [1] https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-rev13-160330final.pdf
> > 
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >   lib/efi_loader/efi_tcg2.c | 90 ++++++++++++++++++++++++++-------------
> >   1 file changed, 61 insertions(+), 29 deletions(-)
> > 
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index 189e4a5ba59c..215f4b2b04b8 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -34,6 +34,7 @@ struct event_log_buffer {
> >   	size_t final_pos; /* final events config table position */
> >   	size_t last_event_size;
> >   	bool get_event_called;
> > +	bool ebs_called;
> 
> Please, add documentation for the elements of the structure. Not every
> reader will be aware of ebs_called referring to ExitBootServices().

Sure

> 
> >   	bool truncated;
> >   };
> > 
> > @@ -186,39 +187,29 @@ static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
> >   	return EFI_SUCCESS;
> > 

[...]

> >   /**
> > @@ -1303,6 +1333,7 @@ static efi_status_t efi_init_event_log(void)
> >   	event_log.pos = 0;
> >   	event_log.last_event_size = 0;
> >   	event_log.get_event_called = false;
> > +	event_log.ebs_called = false;
> >   	event_log.truncated = false;
> > 
> >   	/*
> > @@ -1792,6 +1823,7 @@ efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
> > 
> >   	EFI_ENTRY("%p, %p", event, context);
> 
> This is called in EFI_EVENT_GROUP_EXIT_BOOT_SERVICES.
> 
> This implies that whatever happens in
> EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES is measured normally. Does
> this conform to the TCG2 standard?

Yes I think so.  My understanding of 7.1.2 diagram in the spec is:
- Log all events to the EventLog buffer if GetEventLog() hasn't been called
- Log all events to the EventLog buffer *and* the FinalEvent config table if 
  GetEventLog() has been called
- If you are in EBS(), you don't know if the firmware has cleaned up the
  EventLog buffer, so log these events in the FinalEvent config table only.
> 
> > 
> > +	event_log.ebs_called = true;
> 
> How should a failed call to ExitBootServices() be handled?
> E.g. invalid memory map?

Good question.  We also have efi_tcg2_notify_exit_boot_services_failed().
If the EventLog buffer hasn't been destroyed from memory we can switch the 
ebs_called = false?

Cheers
/Ilias
> 
> Best regards
> 
> Heinrich
> 
> >   	ret = platform_get_tpm2_device(&dev);
> >   	if (ret != EFI_SUCCESS)
> >   		goto out;
> > 
>
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 189e4a5ba59c..215f4b2b04b8 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -34,6 +34,7 @@  struct event_log_buffer {
 	size_t final_pos; /* final events config table position */
 	size_t last_event_size;
 	bool get_event_called;
+	bool ebs_called;
 	bool truncated;
 };
 
@@ -186,39 +187,29 @@  static efi_status_t tcg2_pcr_extend(struct udevice *dev, u32 pcr_index,
 	return EFI_SUCCESS;
 }
 
-/* tcg2_agile_log_append - Append an agile event to out eventlog
+/* put_event - Append an agile event to an eventlog
  *
  * @pcr_index:		PCR index
  * @event_type:		type of event added
  * @digest_list:	list of digest algorithms to add
  * @size:		size of event
  * @event:		event to add
+ * @log:		log buffer to append the event
  *
- * @Return: status code
  */
-static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
-					  struct tpml_digest_values *digest_list,
-					  u32 size, u8 event[])
+static void put_event(u32 pcr_index, u32 event_type,
+		      struct tpml_digest_values *digest_list, u32 size,
+		      u8 event[], void *log)
 {
-	void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
 	size_t pos;
 	size_t i;
 	u32 event_size;
 
-	if (event_log.get_event_called)
-		log = (void *)((uintptr_t)event_log.final_buffer +
-			       event_log.final_pos);
-
 	/*
 	 * size refers to the length of event[] only, we need to check against
 	 * the final tcg_pcr_event2 size
 	 */
 	event_size = size + tcg_event_final_size(digest_list);
-	if (event_log.pos + event_size > TPM2_EVENT_LOG_SIZE ||
-	    event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE) {
-		event_log.truncated = true;
-		return EFI_VOLUME_FULL;
-	}
 
 	put_unaligned_le32(pcr_index, log);
 	pos = offsetof(struct tcg_pcr_event2, event_type);
@@ -242,25 +233,64 @@  static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
 	memcpy((void *)((uintptr_t)log + pos), event, size);
 	pos += size;
 
-	/* make sure the calculated buffer is what we checked against */
+	/*
+	 * make sure the calculated buffer is what we checked against
+	 * This check should never fail.  It checks the code above is
+	 * calculating the right length for the event we are adding
+	 * */
 	if (pos != event_size)
-		return EFI_INVALID_PARAMETER;
+		log_err("Appending to the EventLog failed\n");
 
-	/* if GetEventLog hasn't been called update the normal log */
-	if (!event_log.get_event_called) {
-		event_log.pos += pos;
-		event_log.last_event_size = pos;
-	} else {
-	/* if GetEventLog has been called update config table log */
-		struct efi_tcg2_final_events_table *final_event;
+}
 
-		final_event =
-			(struct efi_tcg2_final_events_table *)(event_log.final_buffer);
-		final_event->number_of_events++;
-		event_log.final_pos += pos;
+/* tcg2_agile_log_append - Append an agile event to an eventlog
+ *
+ * @pcr_index:		PCR index
+ * @event_type:		type of event added
+ * @digest_list:	list of digest algorithms to add
+ * @size:		size of event
+ * @event:		event to add
+ * @log:		log buffer to append the event
+ *
+ * @Return: status code
+ */
+static efi_status_t tcg2_agile_log_append(u32 pcr_index, u32 event_type,
+					  struct tpml_digest_values *digest_list,
+					  u32 size, u8 event[])
+{
+	void *log = (void *)((uintptr_t)event_log.buffer + event_log.pos);
+	u32 event_size = size + tcg_event_final_size(digest_list);
+	struct efi_tcg2_final_events_table *final_event;
+	efi_status_t ret = EFI_SUCCESS;
+
+	/* if ExitBootServices hasn't been called update the normal log */
+	if (!event_log.ebs_called) {
+		if (event_log.truncated ||
+		    event_log.pos + event_size > TPM2_EVENT_LOG_SIZE) {
+			event_log.truncated = true;
+			return EFI_VOLUME_FULL;
+		}
+		put_event(pcr_index, event_type, digest_list, size, event, log);
+		event_log.pos += event_size;
+		event_log.last_event_size = event_size;
 	}
 
-	return EFI_SUCCESS;
+	if (!event_log.get_event_called)
+		return ret;
+
+	/* if GetEventLog has been called update FinalEventLog as well */
+	if (event_log.final_pos + event_size > TPM2_EVENT_LOG_SIZE)
+		return EFI_VOLUME_FULL;
+
+	log = (void *)((uintptr_t)event_log.final_buffer + event_log.final_pos);
+	put_event(pcr_index, event_type, digest_list, size, event, log);
+
+	final_event =
+		(struct efi_tcg2_final_events_table *)event_log.final_buffer;
+	final_event->number_of_events++;
+	event_log.final_pos += event_size;
+
+	return ret;
 }
 
 /**
@@ -1303,6 +1333,7 @@  static efi_status_t efi_init_event_log(void)
 	event_log.pos = 0;
 	event_log.last_event_size = 0;
 	event_log.get_event_called = false;
+	event_log.ebs_called = false;
 	event_log.truncated = false;
 
 	/*
@@ -1792,6 +1823,7 @@  efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context)
 
 	EFI_ENTRY("%p, %p", event, context);
 
+	event_log.ebs_called = true;
 	ret = platform_get_tpm2_device(&dev);
 	if (ret != EFI_SUCCESS)
 		goto out;