On 1/22/21 2:42 AM, AKASHI Takahiro wrote:

CID 316364 says:

...

Null pointer dereferences (FORWARD_NULL) printf("Result total size: 0x%x\n", result->variable_total_size);

at do_efi_capsule_res().

The code is basically safe because a buffer for "result" is allocated by malloc() and filled up by the second get_variable(), which fails any way if the allocation has failed.

But the first (and second) get_variable() possibly returns an error other than EFI_SUCCESS. We always need to check the return code from get_variable() before accessing the data in "result".

While this change won't suppress CID 316364, the resulting code is much safer.

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

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

---

cmd/efidebug.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 9a2d4ddd5ef4..83bc2196a5a9 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -189,14 +189,16 @@ static int do_efi_capsule_res(struct cmd_tbl *cmdtp, int flag, ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size, NULL)); if (ret == EFI_BUFFER_TOO_SMALL) { result = malloc(size);

• if (!result)
  

• 	return CMD_RET_FAILURE;
  

  ret = EFI_CALL(RT->get_variable(var_name16, &guid, NULL, &size, result));

• if (ret != EFI_SUCCESS) {
  

• 	free(result);
  

• 	printf("Failed to get %ls\n", var_name16);
  

• }
• if (ret != EFI_SUCCESS) {

• free(result);
  

• printf("Failed to get %ls\n", var_name16);
  
  

• 	return CMD_RET_FAILURE;
  

• }
  

• return CMD_RET_FAILURE;
  

  }

  printf("Result total size: 0x%x\n", result->variable_total_size);