On 22.07.20 00:50, Ilias Apalodimas wrote:

The current code does not trim the output buffer correctly. In fact it doesn't trim the buffer at all, since it calculates a wrong value for it, which isn't even applied. So let's remove the unused temporary size variable and trim the buffer correctly. Since we are editing efi_get_next_variable_name_int(), fix an indentation error along the way.

Fixes: f042e47e8fb43 ("efi_loader: Implement EFI variable handling via OP-TEE") Signed-off-by: Ilias Apalodimas ilias.apalodimas@linaro.org

---

lib/efi_loader/efi_variable_tee.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index c0423489388a..0e5b4479d936 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -410,7 +410,6 @@ efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, efi_uintn_t payload_size; efi_uintn_t out_name_size; efi_uintn_t in_name_size;

• efi_uintn_t tmp_dsize; u8 *comm_buf = NULL; efi_status_t ret;

@@ -433,13 +432,8 @@ efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, }

/* Trim output buffer size */

• tmp_dsize = *variable_name_size;
• if (in_name_size + tmp_dsize >

• 	max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE) {
  

• tmp_dsize = max_payload_size -
  

• 		MM_VARIABLE_GET_NEXT_HEADER_SIZE -
  

• 		in_name_size;
  

• }

• if (out_name_size > max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE)

• out_name_size = max_payload_size - MM_VARIABLE_GET_NEXT_HEADER_SIZE;
  
  

  payload_size = MM_VARIABLE_GET_NEXT_HEADER_SIZE + out_name_size; comm_buf = setup_mm_hdr((void **)&var_getnext, payload_size,

@@ -465,8 +459,7 @@ efi_status_t efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, goto out;

guidcpy(guid, &var_getnext->guid);

• memcpy(variable_name, (u8 *)var_getnext->name,

•       var_getnext->name_size);
  

• memcpy(variable_name, (u8 *)var_getnext->name, var_getnext->name_size);

var_getnext->name is already a pointer. There is no need to convert this to (u8 *) as memcpy expects (void *) and will accept any pointer.

I will remove that conversion when merging.

Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de

out: free(comm_buf);