| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAC_iWjK69CjJq_ia0RG7DD5-EaQtVkBE_66RxaW1PCDaa_skSw@mail.gmail.com>
Date: Mon, 25 Aug 2025 11:17:45 +0300
From: Ilias Apalodimas <ilias.apalodimas@...aro.org>
To: Jan Kiszka <jan.kiszka@...mens.com>
Cc: Ard Biesheuvel <ardb@...nel.org>, Masahisa Kojima <kojima.masahisa@...ionext.com>,
linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
Sumit Garg <sumit.garg@...nel.org>, Jens Wiklander <jens.wiklander@...aro.org>,
Hua Qian Li <huaqian.li@...mens.com>
Subject: Re: [PATCH v2 3/4] efi: stmm: Drop unused EFI error from setup_mm_hdr arguments
Hi Jan
On Sat, 23 Aug 2025 at 20:22, Jan Kiszka <jan.kiszka@...mens.com> wrote:
>
> From: Jan Kiszka <jan.kiszka@...mens.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@...aro.org>
>
> Signed-off-by: Jan Kiszka <jan.kiszka@...mens.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
>
Powered by blists - more mailing lists