| Message ID | 20210122014248.20514-1-takahiro.akashi@linaro.org |
|---|---|
| State | Accepted |
| Commit | 30f8222bb0016eef29afdd997bd1b3cdc486fdf9 |
| Headers | show |
| Series | cmd: efidebug: always check return code from get_variable() | expand |
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); >
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);