| Message ID | 20200721225037.450919-1-ilias.apalodimas@linaro.org |
|---|---|
| State | Accepted |
| Commit | db94dfbd525943b1bf4ecda81477cedfe70fc50e |
| Headers | show |
| Series | efi_loader: Trim output buffer size correctly for tee variables | expand |
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); >
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); out: free(comm_buf);
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(-) -- 2.28.0.rc1