* [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command
@ 2020-02-28 0:05 AKASHI Takahiro
2020-02-28 18:05 ` Heinrich Schuchardt
0 siblings, 1 reply; 4+ messages in thread
From: AKASHI Takahiro @ 2020-02-28 0:05 UTC (permalink / raw)
To: u-boot
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) {
--
2.24.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command 2020-02-28 0:05 [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command AKASHI Takahiro @ 2020-02-28 18:05 ` Heinrich Schuchardt 2020-03-02 0:05 ` AKASHI Takahiro 0 siblings, 1 reply; 4+ messages in thread From: Heinrich Schuchardt @ 2020-02-28 18:05 UTC (permalink / raw) To: u-boot 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) { > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command 2020-02-28 18:05 ` Heinrich Schuchardt @ 2020-03-02 0:05 ` AKASHI Takahiro 2020-03-02 19:22 ` Heinrich Schuchardt 0 siblings, 1 reply; 4+ messages in thread From: AKASHI Takahiro @ 2020-03-02 0:05 UTC (permalink / raw) To: u-boot 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) { > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command 2020-03-02 0:05 ` AKASHI Takahiro @ 2020-03-02 19:22 ` Heinrich Schuchardt 0 siblings, 0 replies; 4+ messages in thread From: Heinrich Schuchardt @ 2020-03-02 19:22 UTC (permalink / raw) To: u-boot 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) { >>> >> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-02 19:22 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-02-28 0:05 [PATCH] cmd: efidebug: fix a failure of "boot rm" sub-command AKASHI Takahiro 2020-02-28 18:05 ` Heinrich Schuchardt 2020-03-02 0:05 ` AKASHI Takahiro 2020-03-02 19:22 ` Heinrich Schuchardt
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.