diff mbox series

efi_loader: remove comparisons to string literals from runtime

Message ID 20250214134646.173034-1-ilias.apalodimas@linaro.org
State New
Headers show
Series efi_loader: remove comparisons to string literals from runtime | expand

Commit Message

Ilias Apalodimas Feb. 14, 2025, 1:46 p.m. UTC 
For EFI runtime services, we manage to preserve string literals
by placing the .efi_runtime section just before .data and preserving
it when marking the runtime memory by marking surrounding boottime
code as runtime. This is ok for now but will break if we update any
linker scripts and decouple .text and .runtime sections.

So let's define the strings we used to compare in the appropriate
section for runtime services

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 lib/efi_loader/efi_var_mem.c      | 3 ++-
 lib/efi_loader/efi_variable_tee.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

Comments

Ilias Apalodimas Feb. 14, 2025, 1:49 p.m. UTC | #1 
Apologies to both for the noise,

I forgot to mark this as v2 and add a changelog....

On Fri, 14 Feb 2025 at 15:46, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> For EFI runtime services, we manage to preserve string literals
> by placing the .efi_runtime section just before .data and preserving
> it when marking the runtime memory by marking surrounding boottime
> code as runtime. This is ok for now but will break if we update any
> linker scripts and decouple .text and .runtime sections.
>
> So let's define the strings we used to compare in the appropriate
> section for runtime services
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---

Changes since v1:
- Allocate the string literals in an efi runtime section instead of the stack

Sorry!
/Ilias
>  lib/efi_loader/efi_var_mem.c      | 3 ++-
>  lib/efi_loader/efi_variable_tee.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
> index b265d95dd6ba..31180df9e3a0 100644
> --- a/lib/efi_loader/efi_var_mem.c
> +++ b/lib/efi_loader/efi_var_mem.c
> @@ -19,6 +19,7 @@
>   */
>  static struct efi_var_file __efi_runtime_data *efi_var_buf;
>  static struct efi_var_entry __efi_runtime_data *efi_current_var;
> +static const u16 __efi_runtime_rodata vtf[] = u"VarToFile";
>
>  /**
>   * efi_var_mem_compare() - compare GUID and name with a variable
> @@ -331,7 +332,7 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
>         if (timep)
>                 *timep = var->time;
>
> -       if (!u16_strcmp(variable_name, u"VarToFile"))
> +       if (!u16_strcmp(variable_name, vtf))
>                 return efi_var_collect_mem(data, data_size, EFI_VARIABLE_NON_VOLATILE);
>
>         old_size = *data_size;
> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> index 0d090d051dd4..6a1fa39bb6f3 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -41,6 +41,7 @@ static u16 mm_sp_id;
>  extern struct efi_var_file __efi_runtime_data *efi_var_buf;
>  static efi_uintn_t max_buffer_size;    /* comm + var + func + data */
>  static efi_uintn_t max_payload_size;   /* func + data */
> +static const u16 __efi_runtime_rodata pk[] = u"PK";
>
>  struct mm_connection {
>         struct udevice *tee;
> @@ -858,7 +859,7 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
>         if (alt_ret != EFI_SUCCESS)
>                 goto out;
>
> -       if (!u16_strcmp(variable_name, u"PK"))
> +       if (!u16_strcmp(variable_name, pk))
>                 alt_ret = efi_init_secure_state();
>  out:
>         free(comm_buf);
> --
> 2.47.2
>
Mark Kettenis Feb. 14, 2025, 3:21 p.m. UTC | #2 
> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Date: Fri, 14 Feb 2025 15:46:45 +0200
> 
> For EFI runtime services, we manage to preserve string literals
> by placing the .efi_runtime section just before .data and preserving
> it when marking the runtime memory by marking surrounding boottime
> code as runtime. This is ok for now but will break if we update any
> linker scripts and decouple .text and .runtime sections.
> 
> So let's define the strings we used to compare in the appropriate
> section for runtime services
> 
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>  lib/efi_loader/efi_var_mem.c      | 3 ++-
>  lib/efi_loader/efi_variable_tee.c | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)

Yes, that looks much better.

Reviewed-by: Mark Kettenis <kettenis@openbsd.org>

> diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
> index b265d95dd6ba..31180df9e3a0 100644
> --- a/lib/efi_loader/efi_var_mem.c
> +++ b/lib/efi_loader/efi_var_mem.c
> @@ -19,6 +19,7 @@
>   */
>  static struct efi_var_file __efi_runtime_data *efi_var_buf;
>  static struct efi_var_entry __efi_runtime_data *efi_current_var;
> +static const u16 __efi_runtime_rodata vtf[] = u"VarToFile";
>  
>  /**
>   * efi_var_mem_compare() - compare GUID and name with a variable
> @@ -331,7 +332,7 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
>  	if (timep)
>  		*timep = var->time;
>  
> -	if (!u16_strcmp(variable_name, u"VarToFile"))
> +	if (!u16_strcmp(variable_name, vtf))
>  		return efi_var_collect_mem(data, data_size, EFI_VARIABLE_NON_VOLATILE);
>  
>  	old_size = *data_size;
> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
> index 0d090d051dd4..6a1fa39bb6f3 100644
> --- a/lib/efi_loader/efi_variable_tee.c
> +++ b/lib/efi_loader/efi_variable_tee.c
> @@ -41,6 +41,7 @@ static u16 mm_sp_id;
>  extern struct efi_var_file __efi_runtime_data *efi_var_buf;
>  static efi_uintn_t max_buffer_size;	/* comm + var + func + data */
>  static efi_uintn_t max_payload_size;	/* func + data */
> +static const u16 __efi_runtime_rodata pk[] = u"PK";
>  
>  struct mm_connection {
>  	struct udevice *tee;
> @@ -858,7 +859,7 @@ efi_status_t efi_set_variable_int(const u16 *variable_name,
>  	if (alt_ret != EFI_SUCCESS)
>  		goto out;
>  
> -	if (!u16_strcmp(variable_name, u"PK"))
> +	if (!u16_strcmp(variable_name, pk))
>  		alt_ret = efi_init_secure_state();
>  out:
>  	free(comm_buf);
> -- 
> 2.47.2
> 
>
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
index b265d95dd6ba..31180df9e3a0 100644
--- a/lib/efi_loader/efi_var_mem.c
+++ b/lib/efi_loader/efi_var_mem.c
@@ -19,6 +19,7 @@ 
  */
 static struct efi_var_file __efi_runtime_data *efi_var_buf;
 static struct efi_var_entry __efi_runtime_data *efi_current_var;
+static const u16 __efi_runtime_rodata vtf[] = u"VarToFile";
 
 /**
  * efi_var_mem_compare() - compare GUID and name with a variable
@@ -331,7 +332,7 @@  efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
 	if (timep)
 		*timep = var->time;
 
-	if (!u16_strcmp(variable_name, u"VarToFile"))
+	if (!u16_strcmp(variable_name, vtf))
 		return efi_var_collect_mem(data, data_size, EFI_VARIABLE_NON_VOLATILE);
 
 	old_size = *data_size;
diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
index 0d090d051dd4..6a1fa39bb6f3 100644
--- a/lib/efi_loader/efi_variable_tee.c
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -41,6 +41,7 @@  static u16 mm_sp_id;
 extern struct efi_var_file __efi_runtime_data *efi_var_buf;
 static efi_uintn_t max_buffer_size;	/* comm + var + func + data */
 static efi_uintn_t max_payload_size;	/* func + data */
+static const u16 __efi_runtime_rodata pk[] = u"PK";
 
 struct mm_connection {
 	struct udevice *tee;
@@ -858,7 +859,7 @@  efi_status_t efi_set_variable_int(const u16 *variable_name,
 	if (alt_ret != EFI_SUCCESS)
 		goto out;
 
-	if (!u16_strcmp(variable_name, u"PK"))
+	if (!u16_strcmp(variable_name, pk))
 		alt_ret = efi_init_secure_state();
 out:
 	free(comm_buf);