Validate and report the selected interface's IPv4 address instead of the global ipaddr variable. Derive automatic filenames after the interface has been selected; indexed interfaces may use ipaddrN. Keep explicitly supplied filenames unchanged.
Accept physical address zero for client and receive-server transfers when the target memory layout permits it. Retain the existing range and memory reservation checks. Exercise indexed interface addresses, invalid addresses and transfers at address zero with the sandbox command fixture. Signed-off-by: James Hilliard <[email protected]> --- net/lwip/tftp.c | 75 +++++++++++++++++++++------------------------------ test/dm/tftp.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 112 insertions(+), 46 deletions(-) diff --git a/net/lwip/tftp.c b/net/lwip/tftp.c index f29a20f518e..41ea12ed486 100644 --- a/net/lwip/tftp.c +++ b/net/lwip/tftp.c @@ -55,6 +55,16 @@ struct tftp_ctx { */ static struct tftp_ctx *tftpsrv_active_ctx; +static int tftp_check_ip(struct net_lwip_ctx *net) +{ + if (ip4_addr_isany_val(*netif_ip4_addr(net->netif))) { + log_err("error: IP address for %s has to be set\n", net->dev->name); + return -EINVAL; + } + + return 0; +} + /** * store_block() - copy received data * @@ -346,7 +356,7 @@ static int tftp_loop(struct net_lwip_ctx *net, u16 blksize; err_t err; - if (!fname || !*fname || !addr) + if (!fname || !*fname) return -EINVAL; if (operation == TFTP_UPLOAD && (size > U32_MAX || addr + size < addr)) @@ -373,9 +383,9 @@ static int tftp_loop(struct net_lwip_ctx *net, net_boot_file_size = 0; printf("Using %s device\n", net->dev->name); - printf("TFTP %s server %s; our IP address is %s\n", + printf("TFTP %s server %s; our IP address is %pI4\n", ctx.upload ? "to" : "from", ipaddr_ntoa(&srvip), - env_get("ipaddr")); + &netif_ip4_addr(net->netif)->addr); printf("Filename '%s'.\n", fname); if (ctx.upload) { printf("Save address: 0x%lx\n", ctx.daddr); @@ -475,18 +485,11 @@ static void no_request(void *arg) static int tftpsrv_loop(struct net_lwip_ctx *net, ulong addr) { struct tftp_ctx ctx; - const char *ipaddr; int ret = -1; err_t err; - if (addr == 0) - return -1; - - ipaddr = env_get("ipaddr"); - if (!ipaddr || !*ipaddr) { - log_err("error: ipaddr has to be set\n"); + if (tftp_check_ip(net)) return -1; - } memset(&ctx, 0, sizeof(ctx)); ctx.done = NOT_DONE; @@ -495,7 +498,8 @@ static int tftpsrv_loop(struct net_lwip_ctx *net, ulong addr) net_boot_file_size = 0; printf("Using %s device\n", net->dev->name); - printf("Listening for TFTP transfer on %s\n", ipaddr); + printf("Listening for TFTP transfer on %pI4\n", + &netif_ip4_addr(net->netif)->addr); printf("Load address: 0x%lx\n", ctx.daddr); tftpsrv_active_ctx = &ctx; @@ -563,12 +567,6 @@ int do_tftpsrv(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) goto out; } - if (!laddr) { - log_err("error: no load address\n"); - ret = CMD_RET_FAILURE; - goto out; - } - if (net_lwip_start(&net, NET_LWIP_ADDR_ENV_STRICT)) { ret = CMD_RET_FAILURE; goto out; @@ -586,22 +584,14 @@ out: return bootm_maybe_autostart(cmdtp, argv[0]); } -static int tftp_default_filename(char *fname, size_t size) +static void tftp_default_filename(struct net_lwip_ctx *net, char *fname, + size_t size) { - const char *ipaddr = env_get("ipaddr"); - ip_addr_t addr; - - if (!ipaddr || !ipaddr_aton(ipaddr, &addr)) { - log_err("error: ipaddr has to be set\n"); - return -EINVAL; - } + const ip4_addr_t *addr = netif_ip4_addr(net->netif); snprintf(fname, size, "%02X%02X%02X%02X.img", - ip4_addr1(ip_2_ip4(&addr)), ip4_addr2(ip_2_ip4(&addr)), - ip4_addr3(ip_2_ip4(&addr)), ip4_addr4(ip_2_ip4(&addr))); + ip4_addr1(addr), ip4_addr2(addr), ip4_addr3(addr), ip4_addr4(addr)); printf("*** Warning: no boot file name; using '%s'\n", fname); - - return 0; } static int do_tftp(struct cmd_tbl *cmdtp, int flag, int argc, @@ -764,26 +754,12 @@ static int do_tftp(struct cmd_tbl *cmdtp, int flag, int argc, goto out; } - if (!fname || !*fname) { - if (tftp_default_filename(default_fname, - sizeof(default_fname))) { - ret = CMD_RET_FAILURE; - goto out; - } - fname = default_fname; - } - if (strlen(fname) > TFTP_MAX_FILENAME_LEN) { + if (fname && strlen(fname) > TFTP_MAX_FILENAME_LEN) { log_err("error: TFTP file name is too long\n"); ret = CMD_RET_FAILURE; goto out; } - if (!laddr) { - log_err("error: no load address\n"); - ret = CMD_RET_FAILURE; - goto out; - } - if (operation == TFTP_UPLOAD && (size > U32_MAX || laddr + size < laddr)) { log_err("error: TFTP upload range is too large\n"); @@ -796,6 +772,15 @@ static int do_tftp(struct cmd_tbl *cmdtp, int flag, int argc, goto out; } + if (tftp_check_ip(&net)) { + ret = CMD_RET_FAILURE; + goto out; + } + if (!fname || !*fname) { + tftp_default_filename(&net, default_fname, sizeof(default_fname)); + fname = default_fname; + } + if (tftp_loop(&net, operation, laddr, size, fname, srvip, port, srcport, timeout_msecs, max_retries) < 0) ret = CMD_RET_FAILURE; diff --git a/test/dm/tftp.c b/test/dm/tftp.c index 45612e066e5..62a0a887150 100644 --- a/test/dm/tftp.c +++ b/test/dm/tftp.c @@ -47,6 +47,7 @@ struct tftp_test_context { ulong ack_delay; unsigned int advertised_timeout; char filename[64]; + struct in_addr local_ip; u16 blocksize; u16 windowsize; u16 port; @@ -148,6 +149,7 @@ static int tftp_test_tx_handler(struct udevice *dev, void *packet, if (!ctx->requests++) ctx->request_start = get_timer(0); strlcpy(ctx->filename, (char *)(ip + 1) + 2, sizeof(ctx->filename)); + ctx->local_ip = ip->ip_src; ctx->blocksize = tftp_test_option((char *)(ip + 1), request_len, "blksize"); ctx->windowsize = tftp_test_option((char *)(ip + 1), request_len, "windowsize"); ctx->port = ntohs(ip->udp_dst); @@ -439,6 +441,79 @@ static int tftp_test_protected_autostart(struct unit_test_state *uts, return 0; } +#if IS_ENABLED(CONFIG_NET_LWIP) +static int tftp_test_addresses(struct unit_test_state *uts, + struct tftp_test_context *ctx) +{ + int i, put, ret; + + ut_assertok(env_set("autostart", "no")); + ut_assertok(env_set("ethrotate", "no")); + ut_assertok(env_set("ethact", "eth@10003000")); + ut_assertok(env_set("ipaddr5", "1.1.2.4")); + ut_assertok(env_set("netmask5", "255.255.255.0")); + ut_assertok(env_set("bootfile", NULL)); + ut_assertok(env_set("ipaddr", NULL)); + for (put = 0; put <= IS_ENABLED(CONFIG_CMD_TFTPPUT); put++) { + memset(ctx, 0, sizeof(*ctx)); + ut_assertok(run_command(put ? "tftpput 2000000 20" : "tftpboot 2000000", 0)); + ut_asserteq(1, ctx->requests); + ut_asserteq_str("01010204.img", ctx->filename); + ut_asserteq(string_to_ip("1.1.2.4").s_addr, ctx->local_ip.s_addr); + } + /* A valid global address must not hide an unset/invalid indexed one. */ + ut_assertok(env_set("ipaddr", "1.1.2.3")); + for (i = 0; i < 2; i++) { + ut_assertok(env_set("ipaddr5", i ? "invalid" : NULL)); + memset(ctx, 0, sizeof(*ctx)); + ret = run_command("tftpboot 2000000 explicit.bin", 0); + ut_asserteq(CMD_RET_FAILURE, ret); + ut_asserteq(0, ctx->requests); + ut_asserteq(0, net_boot_file_size); + } + ut_assertok(env_set("ethact", "eth@10002000")); + ut_assertok(env_set("ethrotate", NULL)); + return 0; +} +#endif + +static int tftp_test_zero_address(struct unit_test_state *uts, + struct tftp_test_context *ctx) +{ + static const char * const commands[] = { "tftpboot 0 file", "tftpboot 0", "tftpboot" }; + void *buffer = map_sysmem(0, sizeof(tftp_test_data)); + u8 saved[sizeof(tftp_test_data)]; + u8 received[sizeof(tftp_test_data)]; + int i, ret; + + ut_assertok(env_set("autostart", "no")); + ut_assertok(env_set("loadaddr", "0")); + ut_assertok(env_set("bootfile", "file")); + for (i = 0; i < ARRAY_SIZE(commands); i++) { + memcpy(saved, buffer, sizeof(saved)); + memset(ctx, 0, sizeof(*ctx)); + ret = run_command(commands[i], 0); + /* Restore sandbox RAM even if the command failed. */ + memcpy(received, buffer, sizeof(received)); + memcpy(buffer, saved, sizeof(saved)); + ut_assertok(ret); + ut_asserteq(1, ctx->requests); + ut_asserteq(0, image_load_addr); + ut_asserteq(0, env_get_hex("fileaddr", 1)); + ut_asserteq_mem(tftp_test_data, received, sizeof(received)); + } + if (IS_ENABLED(CONFIG_NET_LWIP) && IS_ENABLED(CONFIG_CMD_TFTPPUT)) { + memset(ctx, 0, sizeof(*ctx)); + ut_assertok(run_command("tftpput 0 20 file", 0)); + ut_asserteq(1, ctx->requests); + ut_asserteq(32, net_boot_file_size); + } + unmap_sysmem(buffer); + ut_assertok(env_set("loadaddr", "2000000")); + image_load_addr = TFTP_TEST_ADDR; + return 0; +} + static int tftp_test_download(struct unit_test_state *uts, struct tftp_test_context *ctx) { @@ -463,9 +538,11 @@ static int tftp_test_download(struct unit_test_state *uts, ut_assertok(tftp_test_protected_autostart(uts, ctx)); #if IS_ENABLED(CONFIG_NET_LWIP) + ut_assertok(tftp_test_addresses(uts, ctx)); ut_assertok(tftp_test_options(uts, ctx)); #endif ut_assertok(tftp_test_filenames(uts, ctx)); + ut_assertok(tftp_test_zero_address(uts, ctx)); /* A previous DHCP/download must not override the next command's env. */ ut_assertok(env_set("autostart", "no")); @@ -616,7 +693,7 @@ static int dm_test_tftp_download_only(struct unit_test_state *uts) "ipaddr", "serverip", "netmask", "gatewayip", "netretry", "tftpsrcp", "tftpdstp", "tftpserverip", "fileaddr", "filesize", "bootaddr", "loadaddr", "bootfile", "ethrotate", ".flags", - "tftpblocksize", "tftpwindowsize", + "tftpblocksize", "tftpwindowsize", "ipaddr5", "netmask5", }; char saved_bootfile[sizeof(net_boot_file_name)]; ulong saved_load_addr = image_load_addr; @@ -637,9 +714,13 @@ static int dm_test_tftp_download_only(struct unit_test_state *uts) sandbox_eth_set_tx_handler(0, tftp_test_tx_handler); sandbox_eth_set_priv(0, &ctx); + sandbox_eth_set_tx_handler(1, tftp_test_tx_handler); + sandbox_eth_set_priv(1, &ctx); ret = tftp_test_download(uts, &ctx); sandbox_eth_set_tx_handler(0, NULL); sandbox_eth_set_priv(0, NULL); + sandbox_eth_set_tx_handler(1, NULL); + sandbox_eth_set_priv(1, NULL); image_load_addr = saved_load_addr; memcpy(net_boot_file_name, saved_bootfile, sizeof(saved_bootfile)); for (i = 0; i < ARRAY_SIZE(vars); i++) { -- 2.53.0
