| Message ID | 20220903122109.6076-1-xypron.glpk@gmx.de |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/1] cmd: fix tftpput command | expand |
Hi Heinrich, On Sat, 3 Sept 2022 at 06:21, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > Calling tftpput with less than 2 arguments must lead to a failure. > > If tftpput is called with two arguments, these are the address and > the size of the file to be transferred. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > cmd/net.c | 50 +++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 37 insertions(+), 13 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> > > diff --git a/cmd/net.c b/cmd/net.c > index 3619c843d8..e1be7b431a 100644 > --- a/cmd/net.c > +++ b/cmd/net.c > @@ -189,6 +189,19 @@ static void netboot_update_env(void) > #endif > } > > +/** > + * parse_addr_size() - parse address and size arguments > + */ > +static int parse_addr_size(char * const argv[]) > +{ > + if (strict_strtoul(argv[1], 16, &image_save_addr) < 0 || > + strict_strtoul(argv[2], 16, &image_save_size) < 0) { > + printf("Invalid address/size\n"); > + return CMD_RET_USAGE; > + } > + return 0; > +} > + > static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc, > char *const argv[]) > { > @@ -199,6 +212,7 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc, > ulong addr; > > net_boot_file_name_explicit = false; > + *net_boot_file_name = '\0'; > > /* pre-set image_load_addr */ > s = env_get("loadaddr"); > @@ -207,12 +221,18 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc, > > switch (argc) { > case 1: > + if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT) > + goto err_usage; > + > /* refresh bootfile name from env */ > copy_filename(net_boot_file_name, env_get("bootfile"), > sizeof(net_boot_file_name)); > break; > > - case 2: /* > + case 2: > + if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT) > + goto err_usage; > + /* > * Only one arg - accept two forms: > * Just load address, or just boot file name. The latter > * form must be written in a format which can not be > @@ -232,28 +252,28 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc, > break; > > case 3: > - image_load_addr = hextoul(argv[1], NULL); > - net_boot_file_name_explicit = true; > - copy_filename(net_boot_file_name, argv[2], > - sizeof(net_boot_file_name)); > - > + if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT) { > + if (parse_addr_size(argv)) > + goto err_usage; > + } else { > + image_load_addr = hextoul(argv[1], NULL); > + net_boot_file_name_explicit = true; > + copy_filename(net_boot_file_name, argv[2], > + sizeof(net_boot_file_name)); > + } > break; > > #ifdef CONFIG_CMD_TFTPPUT > case 4: > - if (strict_strtoul(argv[1], 16, &image_save_addr) < 0 || > - strict_strtoul(argv[2], 16, &image_save_size) < 0) { > - printf("Invalid address/size\n"); > - return CMD_RET_USAGE; > - } > + if (parse_addr_size(argv)) > + goto err_usage; > net_boot_file_name_explicit = true; > copy_filename(net_boot_file_name, argv[3], > sizeof(net_boot_file_name)); > break; > #endif > default: > - bootstage_error(BOOTSTAGE_ID_NET_START); > - return CMD_RET_USAGE; > + goto err_usage; > } > bootstage_mark(BOOTSTAGE_ID_NET_START); > > @@ -282,6 +302,10 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc, > else > bootstage_error(BOOTSTAGE_ID_NET_DONE_ERR); > return rcode; > + > +err_usage: > + bootstage_error(BOOTSTAGE_ID_NET_START); > + return CMD_RET_USAGE; > } > > #if defined(CONFIG_CMD_PING) > -- > 2.30.2 > To me this would be better if the arg parsing all moved to a separate function which returns an error code. It would avoid the goto. Also perhaps this should be in the same series as the tftpput docs? Regards, Simon
diff --git a/cmd/net.c b/cmd/net.c index 3619c843d8..e1be7b431a 100644 --- a/cmd/net.c +++ b/cmd/net.c @@ -189,6 +189,19 @@ static void netboot_update_env(void) #endif } +/** + * parse_addr_size() - parse address and size arguments + */ +static int parse_addr_size(char * const argv[]) +{ + if (strict_strtoul(argv[1], 16, &image_save_addr) < 0 || + strict_strtoul(argv[2], 16, &image_save_size) < 0) { + printf("Invalid address/size\n"); + return CMD_RET_USAGE; + } + return 0; +} + static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc, char *const argv[]) { @@ -199,6 +212,7 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc, ulong addr; net_boot_file_name_explicit = false; + *net_boot_file_name = '\0'; /* pre-set image_load_addr */ s = env_get("loadaddr"); @@ -207,12 +221,18 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc, switch (argc) { case 1: + if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT) + goto err_usage; + /* refresh bootfile name from env */ copy_filename(net_boot_file_name, env_get("bootfile"), sizeof(net_boot_file_name)); break; - case 2: /* + case 2: + if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT) + goto err_usage; + /* * Only one arg - accept two forms: * Just load address, or just boot file name. The latter * form must be written in a format which can not be @@ -232,28 +252,28 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc, break; case 3: - image_load_addr = hextoul(argv[1], NULL); - net_boot_file_name_explicit = true; - copy_filename(net_boot_file_name, argv[2], - sizeof(net_boot_file_name)); - + if (CONFIG_IS_ENABLED(CMD_TFTPPUT) && proto == TFTPPUT) { + if (parse_addr_size(argv)) + goto err_usage; + } else { + image_load_addr = hextoul(argv[1], NULL); + net_boot_file_name_explicit = true; + copy_filename(net_boot_file_name, argv[2], + sizeof(net_boot_file_name)); + } break; #ifdef CONFIG_CMD_TFTPPUT case 4: - if (strict_strtoul(argv[1], 16, &image_save_addr) < 0 || - strict_strtoul(argv[2], 16, &image_save_size) < 0) { - printf("Invalid address/size\n"); - return CMD_RET_USAGE; - } + if (parse_addr_size(argv)) + goto err_usage; net_boot_file_name_explicit = true; copy_filename(net_boot_file_name, argv[3], sizeof(net_boot_file_name)); break; #endif default: - bootstage_error(BOOTSTAGE_ID_NET_START); - return CMD_RET_USAGE; + goto err_usage; } bootstage_mark(BOOTSTAGE_ID_NET_START); @@ -282,6 +302,10 @@ static int netboot_common(enum proto_t proto, struct cmd_tbl *cmdtp, int argc, else bootstage_error(BOOTSTAGE_ID_NET_DONE_ERR); return rcode; + +err_usage: + bootstage_error(BOOTSTAGE_ID_NET_START); + return CMD_RET_USAGE; } #if defined(CONFIG_CMD_PING)
Calling tftpput with less than 2 arguments must lead to a failure. If tftpput is called with two arguments, these are the address and the size of the file to be transferred. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- cmd/net.c | 50 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 13 deletions(-) -- 2.30.2