1. Patchew
  2. linux
  3. [PATCH v2 0/4] efi: stmm: Fix for incorrect buffer allocation and cleanups
  4. View patch

[PATCH v2 3/4] efi: stmm: Drop unused EFI error from setup_mm_hdr arguments

Jan Kiszka posted 4 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v2 3/4] efi: stmm: Drop unused EFI error from setup_mm_hdr arguments
Posted by Jan Kiszka 1 month, 1 week ago 
From: Jan Kiszka <jan.kiszka@siemens.com>

No caller evaluates it.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++++++++----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/firmware/efi/stmm/tee_stmm_efi.c b/drivers/firmware/efi/stmm/tee_stmm_efi.c
index 8501056ade8a..c2bc8467b099 100644
--- a/drivers/firmware/efi/stmm/tee_stmm_efi.c
+++ b/drivers/firmware/efi/stmm/tee_stmm_efi.c
@@ -154,11 +154,9 @@ static efi_status_t mm_communicate(u8 *comm_buf, size_t payload_size)
  * @dptr:		pointer address to store allocated buffer
  * @payload_size:	payload size
  * @func:		standAloneMM function number
- * @ret:		EFI return code
  * Return:		pointer to corresponding StandAloneMM function buffer or NULL
  */
-static void *setup_mm_hdr(u8 **dptr, size_t payload_size, size_t func,
-			  efi_status_t *ret)
+static void *setup_mm_hdr(u8 **dptr, size_t payload_size, size_t func)
 {
 	const efi_guid_t mm_var_guid = EFI_MM_VARIABLE_GUID;
 	struct efi_mm_communicate_header *mm_hdr;
@@ -173,16 +171,13 @@ static void *setup_mm_hdr(u8 **dptr, size_t payload_size, size_t func,
 	if (max_buffer_size &&
 	    max_buffer_size < (MM_COMMUNICATE_HEADER_SIZE +
 			       MM_VARIABLE_COMMUNICATE_SIZE + payload_size)) {
-		*ret = EFI_INVALID_PARAMETER;
 		return NULL;
 	}
 
 	comm_buf = alloc_pages_exact(COMM_BUF_SIZE(payload_size),
 				     GFP_KERNEL | __GFP_ZERO);
-	if (!comm_buf) {
-		*ret = EFI_OUT_OF_RESOURCES;
+	if (!comm_buf)
 		return NULL;
-	}
 
 	mm_hdr = (struct efi_mm_communicate_header *)comm_buf;
 	memcpy(&mm_hdr->header_guid, &mm_var_guid, sizeof(mm_hdr->header_guid));
@@ -192,7 +187,6 @@ static void *setup_mm_hdr(u8 **dptr, size_t payload_size, size_t func,
 	var_hdr->function = func;
 	if (dptr)
 		*dptr = comm_buf;
-	*ret = EFI_SUCCESS;
 
 	return var_hdr->data;
 }
@@ -215,8 +209,7 @@ static efi_status_t get_max_payload(size_t *size)
 
 	payload_size = sizeof(*var_payload);
 	var_payload = setup_mm_hdr(&comm_buf, payload_size,
-				   SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE,
-				   &ret);
+				   SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE);
 	if (!var_payload)
 		return EFI_DEVICE_ERROR;
 
@@ -262,7 +255,7 @@ static efi_status_t get_property_int(u16 *name, size_t name_size,
 
 	smm_property = setup_mm_hdr(
 		&comm_buf, payload_size,
-		SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET, &ret);
+		SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET);
 	if (!smm_property)
 		return EFI_DEVICE_ERROR;
 
@@ -318,7 +311,7 @@ static efi_status_t tee_get_variable(u16 *name, efi_guid_t *vendor,
 
 	payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + tmp_dsize;
 	var_acc = setup_mm_hdr(&comm_buf, payload_size,
-			       SMM_VARIABLE_FUNCTION_GET_VARIABLE, &ret);
+			       SMM_VARIABLE_FUNCTION_GET_VARIABLE);
 	if (!var_acc)
 		return EFI_DEVICE_ERROR;
 
@@ -383,8 +376,7 @@ static efi_status_t tee_get_next_variable(unsigned long *name_size,
 
 	payload_size = MM_VARIABLE_GET_NEXT_HEADER_SIZE + out_name_size;
 	var_getnext = setup_mm_hdr(&comm_buf, payload_size,
-				   SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME,
-				   &ret);
+				SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);
 	if (!var_getnext)
 		return EFI_DEVICE_ERROR;
 
@@ -440,7 +432,7 @@ static efi_status_t tee_set_variable(efi_char16_t *name, efi_guid_t *vendor,
 	 * the properties, if the allocation fails
 	 */
 	var_acc = setup_mm_hdr(&comm_buf, payload_size,
-			       SMM_VARIABLE_FUNCTION_SET_VARIABLE, &ret);
+			       SMM_VARIABLE_FUNCTION_SET_VARIABLE);
 	if (!var_acc)
 		return EFI_DEVICE_ERROR;
 
@@ -495,8 +487,7 @@ static efi_status_t tee_query_variable_info(u32 attributes,
 
 	payload_size = sizeof(*mm_query_info);
 	mm_query_info = setup_mm_hdr(&comm_buf, payload_size,
-				SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO,
-				&ret);
+				SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO);
 	if (!mm_query_info)
 		return EFI_DEVICE_ERROR;
 
-- 
2.43.0
Re: [PATCH v2 3/4] efi: stmm: Drop unused EFI error from setup_mm_hdr arguments
Posted by Ilias Apalodimas 1 month, 1 week ago 
Hi Jan

On Sat, 23 Aug 2025 at 20:22, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> No caller evaluates it.

We need a better description on this one, other than that I am fine
with the patch.
Callers were never evaluating the ret, they just used the return value
in case of an error

With that fixed
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  drivers/firmware/efi/stmm/tee_stmm_efi.c | 25 ++++++++----------------
>  1 file changed, 8 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/firmware/efi/stmm/tee_stmm_efi.c b/drivers/firmware/efi/stmm/tee_stmm_efi.c
> index 8501056ade8a..c2bc8467b099 100644
> --- a/drivers/firmware/efi/stmm/tee_stmm_efi.c
> +++ b/drivers/firmware/efi/stmm/tee_stmm_efi.c
> @@ -154,11 +154,9 @@ static efi_status_t mm_communicate(u8 *comm_buf, size_t payload_size)
>   * @dptr:              pointer address to store allocated buffer
>   * @payload_size:      payload size
>   * @func:              standAloneMM function number
> - * @ret:               EFI return code
>   * Return:             pointer to corresponding StandAloneMM function buffer or NULL
>   */
> -static void *setup_mm_hdr(u8 **dptr, size_t payload_size, size_t func,
> -                         efi_status_t *ret)
> +static void *setup_mm_hdr(u8 **dptr, size_t payload_size, size_t func)
>  {
>         const efi_guid_t mm_var_guid = EFI_MM_VARIABLE_GUID;
>         struct efi_mm_communicate_header *mm_hdr;
> @@ -173,16 +171,13 @@ static void *setup_mm_hdr(u8 **dptr, size_t payload_size, size_t func,
>         if (max_buffer_size &&
>             max_buffer_size < (MM_COMMUNICATE_HEADER_SIZE +
>                                MM_VARIABLE_COMMUNICATE_SIZE + payload_size)) {
> -               *ret = EFI_INVALID_PARAMETER;
>                 return NULL;
>         }
>
>         comm_buf = alloc_pages_exact(COMM_BUF_SIZE(payload_size),
>                                      GFP_KERNEL | __GFP_ZERO);
> -       if (!comm_buf) {
> -               *ret = EFI_OUT_OF_RESOURCES;
> +       if (!comm_buf)
>                 return NULL;
> -       }
>
>         mm_hdr = (struct efi_mm_communicate_header *)comm_buf;
>         memcpy(&mm_hdr->header_guid, &mm_var_guid, sizeof(mm_hdr->header_guid));
> @@ -192,7 +187,6 @@ static void *setup_mm_hdr(u8 **dptr, size_t payload_size, size_t func,
>         var_hdr->function = func;
>         if (dptr)
>                 *dptr = comm_buf;
> -       *ret = EFI_SUCCESS;
>
>         return var_hdr->data;
>  }
> @@ -215,8 +209,7 @@ static efi_status_t get_max_payload(size_t *size)
>
>         payload_size = sizeof(*var_payload);
>         var_payload = setup_mm_hdr(&comm_buf, payload_size,
> -                                  SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE,
> -                                  &ret);
> +                                  SMM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE);
>         if (!var_payload)
>                 return EFI_DEVICE_ERROR;
>
> @@ -262,7 +255,7 @@ static efi_status_t get_property_int(u16 *name, size_t name_size,
>
>         smm_property = setup_mm_hdr(
>                 &comm_buf, payload_size,
> -               SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET, &ret);
> +               SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET);
>         if (!smm_property)
>                 return EFI_DEVICE_ERROR;
>
> @@ -318,7 +311,7 @@ static efi_status_t tee_get_variable(u16 *name, efi_guid_t *vendor,
>
>         payload_size = MM_VARIABLE_ACCESS_HEADER_SIZE + name_size + tmp_dsize;
>         var_acc = setup_mm_hdr(&comm_buf, payload_size,
> -                              SMM_VARIABLE_FUNCTION_GET_VARIABLE, &ret);
> +                              SMM_VARIABLE_FUNCTION_GET_VARIABLE);
>         if (!var_acc)
>                 return EFI_DEVICE_ERROR;
>
> @@ -383,8 +376,7 @@ static efi_status_t tee_get_next_variable(unsigned long *name_size,
>
>         payload_size = MM_VARIABLE_GET_NEXT_HEADER_SIZE + out_name_size;
>         var_getnext = setup_mm_hdr(&comm_buf, payload_size,
> -                                  SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME,
> -                                  &ret);
> +                               SMM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME);
>         if (!var_getnext)
>                 return EFI_DEVICE_ERROR;
>
> @@ -440,7 +432,7 @@ static efi_status_t tee_set_variable(efi_char16_t *name, efi_guid_t *vendor,
>          * the properties, if the allocation fails
>          */
>         var_acc = setup_mm_hdr(&comm_buf, payload_size,
> -                              SMM_VARIABLE_FUNCTION_SET_VARIABLE, &ret);
> +                              SMM_VARIABLE_FUNCTION_SET_VARIABLE);
>         if (!var_acc)
>                 return EFI_DEVICE_ERROR;
>
> @@ -495,8 +487,7 @@ static efi_status_t tee_query_variable_info(u32 attributes,
>
>         payload_size = sizeof(*mm_query_info);
>         mm_query_info = setup_mm_hdr(&comm_buf, payload_size,
> -                               SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO,
> -                               &ret);
> +                               SMM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO);
>         if (!mm_query_info)
>                 return EFI_DEVICE_ERROR;
>
> --
> 2.43.0
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.