[PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command
There is a wrong usage of utf8_utf16_strncpy() in "boot rm" command, and then it will end up with a failure of this command due to a wrong value of an interim variable ("var_name16").
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- cmd/efidebug.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 576e95b395dc..3a50dafbbca6 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -641,7 +641,7 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, int id, i; char *endp; char var_name[9]; - u16 var_name16[9]; + u16 var_name16[9], *p; efi_status_t ret;
if (argc == 1) @@ -654,7 +654,8 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, return CMD_RET_FAILURE;
sprintf(var_name, "Boot%04X", id); - utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9); + p = var_name16; + utf8_utf16_strncpy(&p, var_name, 9);
ret = EFI_CALL(RT->set_variable(var_name16, &guid, 0, 0, NULL)); if (ret) {
On 2/28/20 1:05 AM, AKASHI Takahiro wrote:
There is a wrong usage of utf8_utf16_strncpy() in "boot rm" command, and then it will end up with a failure of this command due to a wrong value of an interim variable ("var_name16").
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/efidebug.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 576e95b395dc..3a50dafbbca6 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -641,7 +641,7 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, int id, i; char *endp; char var_name[9];
- u16 var_name16[9];
u16 var_name16[9], *p; efi_status_t ret;
if (argc == 1)
@@ -654,7 +654,8 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, return CMD_RET_FAILURE;
sprintf(var_name, "Boot%04X", id);
utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
p = var_name16;utf8_utf16_strncpy(&p, var_name, 9);
This is duplicating code in do_efi_boot_add(). Please, consider putting the following codeblock into separate function:
id = (int)simple_strtoul(argv[1], &endp, 16); if (*endp != '\0' || id > 0xffff) return CMD_RET_USAGE;
sprintf(var_name, "Boot%04X", id); p = var_name16; utf8_utf16_strncpy(&p, var_name, 9);
try_load_entry() has another implementation.
Best regards
Heinrich
ret = EFI_CALL(RT->set_variable(var_name16, &guid, 0, 0, NULL)); if (ret) {
On Fri, Feb 28, 2020 at 07:05:39PM +0100, Heinrich Schuchardt wrote:
On 2/28/20 1:05 AM, AKASHI Takahiro wrote:
There is a wrong usage of utf8_utf16_strncpy() in "boot rm" command, and then it will end up with a failure of this command due to a wrong value of an interim variable ("var_name16").
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/efidebug.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 576e95b395dc..3a50dafbbca6 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -641,7 +641,7 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, int id, i; char *endp; char var_name[9];
- u16 var_name16[9];
u16 var_name16[9], *p; efi_status_t ret;
if (argc == 1)
@@ -654,7 +654,8 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, return CMD_RET_FAILURE;
sprintf(var_name, "Boot%04X", id);
utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
p = var_name16;utf8_utf16_strncpy(&p, var_name, 9);This is duplicating code in do_efi_boot_add(). Please, consider putting the following codeblock into separate function:
No.
id = (int)simple_strtoul(argv[1], &endp, 16); if (*endp != '\0' || id > 0xffff) return CMD_RET_USAGE;
sprintf(var_name, "Boot%04X", id); p = var_name16; utf8_utf16_strncpy(&p, var_name, 9);
Frankly, I don't like this function's prototype because it is different from "normal" corresponding C libraries. Moreover, as far as I notice, there is no use case where the first parameter which is modified after calling is used in any code.
Thanks, -Takahiro Akashi
try_load_entry() has another implementation.
Best regards
Heinrich
ret = EFI_CALL(RT->set_variable(var_name16, &guid, 0, 0, NULL)); if (ret) {
On 3/2/20 1:05 AM, AKASHI Takahiro wrote:
On Fri, Feb 28, 2020 at 07:05:39PM +0100, Heinrich Schuchardt wrote:
On 2/28/20 1:05 AM, AKASHI Takahiro wrote:
There is a wrong usage of utf8_utf16_strncpy() in "boot rm" command, and then it will end up with a failure of this command due to a wrong value of an interim variable ("var_name16").
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
cmd/efidebug.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c index 576e95b395dc..3a50dafbbca6 100644 --- a/cmd/efidebug.c +++ b/cmd/efidebug.c @@ -641,7 +641,7 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, int id, i; char *endp; char var_name[9];
- u16 var_name16[9];
u16 var_name16[9], *p; efi_status_t ret;
if (argc == 1)
@@ -654,7 +654,8 @@ static int do_efi_boot_rm(cmd_tbl_t *cmdtp, int flag, return CMD_RET_FAILURE;
sprintf(var_name, "Boot%04X", id);
utf8_utf16_strncpy((u16 **)&var_name16, var_name, 9);
p = var_name16;utf8_utf16_strncpy(&p, var_name, 9);This is duplicating code in do_efi_boot_add(). Please, consider putting the following codeblock into separate function:
No.
Factoring out a separate function does not reduce the code size.
Hence Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
id = (int)simple_strtoul(argv[1], &endp, 16); if (*endp != '\0' || id > 0xffff) return CMD_RET_USAGE;
sprintf(var_name, "Boot%04X", id); p = var_name16; utf8_utf16_strncpy(&p, var_name, 9);
Frankly, I don't like this function's prototype because it is different from "normal" corresponding C libraries. Moreover, as far as I notice, there is no use case where the first parameter which is modified after calling is used in any code.
Thanks, -Takahiro Akashi
try_load_entry() has another implementation.
Best regards
Heinrich
ret = EFI_CALL(RT->set_variable(var_name16, &guid, 0, 0, NULL)); if (ret) {
participants (2)
-
AKASHI Takahiro -
Heinrich Schuchardt