[PATCH] cmd: efidebug: always check return code from get_variable()

[Heinrich Schuchardt]

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 at linaro.org>

Reviewed-by: Heinrich Schuchardt <xypron.glpk at 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);
>