The compatibility wrapper returns shell status: zero on success and one on failure. The bubt caller expects the legacy net_loop contract, where a positive result means a nonempty download and failures are negative. This rejects successful images and can accept stale size information after a failed command.
Clear the previous size before calling TFTP, translate command failure to a negative error and return whether a successful transfer is nonempty. Pass the caller's image_load_addr and net_boot_file_name explicitly instead of using shell environment defaults. Clear the size at command entry as well, including failures before transport initialization. Publish completion after teardown, checking both fileaddr and filesize updates before exposing the new size or EFI boot device. A failed metadata update must fail the client or receive-server command, not leave its caller believing a usable image was downloaded. Add sandbox coverage for nonempty and empty transfers, server errors, read-only metadata variables, recovery after failure, explicit caller arguments and failure before transport initialization. Signed-off-by: James Hilliard <[email protected]> --- net/lwip/net-lwip.c | 13 +++-- net/lwip/tftp.c | 45 +++++++-------- test/dm/Makefile | 1 + test/dm/lwip_net_loop.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 173 insertions(+), 29 deletions(-) diff --git a/net/lwip/net-lwip.c b/net/lwip/net-lwip.c index 2afe74cc6a5..ac8d3906fd9 100644 --- a/net/lwip/net-lwip.c +++ b/net/lwip/net-lwip.c @@ -4,6 +4,7 @@ #include <command.h> #include <env.h> +#include <image.h> #include <dm/device.h> #include <dm/uclass.h> #include <hexdump.h> @@ -700,12 +701,16 @@ void net_process_received_packet(uchar *in_packet, int len) int net_loop(enum proto_t protocol) { - char *argv[1]; - switch (protocol) { case TFTPGET: - argv[0] = "tftpboot"; - return do_tftpb(NULL, 0, 1, argv); + /* Match the legacy loop, not the shell command's return value. + * In particular, bubt treats a positive result as downloaded data. + * Its caller supplies the load address and filename directly. + */ + net_boot_file_size = 0; + if (tftpb_run(image_load_addr, net_boot_file_name)) + return -EINVAL; + return !!net_boot_file_size; default: return -EINVAL; } diff --git a/net/lwip/tftp.c b/net/lwip/tftp.c index 025b65a3a82..e58e1995cb3 100644 --- a/net/lwip/tftp.c +++ b/net/lwip/tftp.c @@ -153,7 +153,6 @@ static void tftp_close(void *handle) return; } ctx->done = SUCCESS; - net_boot_file_size = ctx->size; tftp_tsize = tftp_transfer_size(ctx); if (tftp_tsize) { @@ -173,11 +172,26 @@ static void tftp_close(void *handle) } puts("\ndone\n"); printf("Bytes transferred = %lu (%lx hex)\n", ctx->size, ctx->size); +} +/* Publish completion only after teardown and the fallible environment writes. */ +static int tftp_complete(struct tftp_ctx *ctx, ulong addr, const char *fname) +{ + if (ctx->done != SUCCESS) + return -EIO; + if (!ctx->upload && env_set_hex("fileaddr", addr)) { + log_err("fileaddr not updated\n"); + return -EIO; + } if (env_set_hex("filesize", ctx->size)) { log_err("filesize not updated\n"); - return; + return -EIO; } + net_boot_file_size = ctx->size; + if (!ctx->upload) + efi_set_bootdev("Net", "", fname, map_sysmem(addr, 0), ctx->size); + + return 0; } static int tftp_read(void *handle, void *buf, int bytes) @@ -328,7 +342,6 @@ static int tftp_loop(struct net_lwip_ctx *net, struct tftp_ctx ctx; bool initialized = false; u16 blksize; - int ret = -1; err_t err; if (!fname || !*fname || !addr) @@ -427,19 +440,7 @@ out_cleanup: if (initialized) tftp_cleanup(); - if (ctx.done == SUCCESS) { - if (!ctx.upload) { - if (env_set_hex("fileaddr", addr)) { - log_err("fileaddr not updated\n"); - return -1; - } - efi_set_bootdev("Net", "", fname, map_sysmem(addr, 0), - ctx.size); - } - ret = 0; - } - - return ret; + return tftp_complete(&ctx, addr, fname); } static void no_request(void *arg) @@ -512,15 +513,7 @@ out_cleanup: tftp_cleanup(); - if (ctx.done == SUCCESS) { - if (env_set_hex("fileaddr", addr)) { - log_err("fileaddr not updated\n"); - goto out; - } - efi_set_bootdev("Net", "", ctx.fname, map_sysmem(addr, 0), - ctx.size); - ret = 0; - } + ret = tftp_complete(&ctx, addr, ctx.fname); out: tftpsrv_active_ctx = NULL; @@ -536,6 +529,7 @@ int do_tftpsrv(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) ulong laddr; ulong addr; + net_boot_file_size = 0; if (!IS_ENABLED(CONFIG_CMD_TFTPSRV)) return CMD_RET_FAILURE; @@ -619,6 +613,7 @@ static int do_tftp(struct cmd_tbl *cmdtp, int flag, int argc, const char *ep; ulong value; + net_boot_file_size = 0; laddr = env_get_ulong("loadaddr", 16, image_load_addr); if (operation == TFTP_UPLOAD) { diff --git a/test/dm/Makefile b/test/dm/Makefile index fa6ad7fb38b..f43d519980d 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -66,6 +66,7 @@ obj-y += lwip_bootp.o endif ifeq ($(CONFIG_NET_LWIP)$(CONFIG_CMD_TFTPBOOT),yy) obj-y += lwip_tftp.o +obj-y += lwip_net_loop.o endif ifeq ($(CONFIG_NET_LWIP)$(CONFIG_NETCONSOLE),yy) obj-y += netconsole.o diff --git a/test/dm/lwip_net_loop.c b/test/dm/lwip_net_loop.c new file mode 100644 index 00000000000..2e646e40020 --- /dev/null +++ b/test/dm/lwip_net_loop.c @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* Copyright 2026 James Hilliard <[email protected]> */ + +#include <dm.h> +#include <env.h> +#include <image.h> +#include <malloc.h> +#include <net.h> +#include <asm/eth.h> +#include <asm/unaligned.h> +#include <dm/test.h> +#include <test/ut.h> + +static int net_loop_reply(struct udevice *dev, void *packet, unsigned int len) +{ + struct eth_sandbox_priv *priv = dev_get_priv(dev); + int mode = *(int *)priv->priv; + struct ethernet_hdr *eth = packet, *reply; + struct ip_udp_hdr *ip, *rip; + u8 *data; + unsigned int size = mode ? 5 : 4; + + if (!sandbox_eth_arp_req_to_reply(dev, packet, len)) + return 0; + if (len < ETHER_HDR_SIZE + IP_UDP_HDR_SIZE + 2 || + ntohs(eth->et_protlen) != PROT_IP) + return 0; + ip = (void *)(eth + 1); + if (ip->ip_p != IPPROTO_UDP || ntohs(ip->udp_dst) != 69 || + get_unaligned_be16(ip + 1) != 1) + return 0; + if (strcmp((char *)(ip + 1) + 2, "test.bin")) + mode = -1; + if (priv->recv_packets >= PKTBUFSRX) + return -ENOSPC; + reply = (void *)priv->recv_packet_buffer[priv->recv_packets]; + memcpy(reply->et_dest, eth->et_src, ARP_HLEN); + memcpy(reply->et_src, priv->fake_host_hwaddr, ARP_HLEN); + reply->et_protlen = htons(PROT_IP); + rip = (void *)(reply + 1); + memset(rip, 0, IP_UDP_HDR_SIZE); + rip->ip_hl_v = 0x45; + rip->ip_len = htons(IP_UDP_HDR_SIZE + size); + rip->ip_ttl = 64; + rip->ip_p = IPPROTO_UDP; + rip->ip_src = ip->ip_dst; + rip->ip_dst = ip->ip_src; + rip->udp_src = htons(1234); + rip->udp_dst = ip->udp_src; + rip->udp_len = htons(UDP_HDR_SIZE + size); + rip->ip_sum = compute_ip_checksum(rip, IP_HDR_SIZE); + data = (void *)(rip + 1); + put_unaligned_be16(mode < 0 ? 5 : 3, data); + put_unaligned_be16(1, data + 2); + data[4] = 0; + priv->recv_packet_length[priv->recv_packets++] = + ETHER_HDR_SIZE + IP_UDP_HDR_SIZE + size; + return 0; +} + +static int net_loop_check(struct unit_test_state *uts, int *mode) +{ + static const char * const flags[] = { "filesize:xr", "fileaddr:xr" }; + int i; + + ut_assertok(env_set("ethact", "eth@10002000")); + ut_assertok(env_set("ipaddr", "1.1.2.3")); + ut_assertok(env_set("netmask", "255.255.255.0")); + ut_assertok(env_set("serverip", "1.1.2.2")); + ut_assertok(env_set("tftpserverip", NULL)); + ut_assertok(env_set("tftpdstp", NULL)); + ut_assertok(env_set("loadaddr", "2000000")); + ut_assertok(env_set("bootfile", "other.bin")); + ut_assertok(env_set("autostart", "no")); + strcpy(net_boot_file_name, "test.bin"); + image_load_addr = 0x2100000; + for (*mode = -1; *mode <= 1; (*mode)++) { + net_boot_file_size = 1234; + ut_asserteq(*mode < 0 ? -EINVAL : *mode, net_loop(TFTPGET)); + ut_asserteq(*mode > 0 ? 1 : 0, net_boot_file_size); + if (*mode >= 0) + ut_asserteq_str("2100000", env_get("fileaddr")); + } + for (i = 0; i < ARRAY_SIZE(flags); i++) { + /* A complete transfer with unpublishable metadata is still a failure. */ + *mode = 1; + ut_assertok(env_set(".flags", flags[i])); + net_boot_file_size = 1234; + ut_asserteq(-EINVAL, net_loop(TFTPGET)); + ut_asserteq(0, net_boot_file_size); + ut_assertok(env_set(".flags", NULL)); + ut_asserteq(1, net_loop(TFTPGET)); + ut_asserteq(1, net_boot_file_size); + } + /* Failure before the transport starts must not expose the last size. */ + ut_assertok(env_set("serverip", NULL)); + net_boot_file_size = 1234; + ut_asserteq(-EINVAL, net_loop(TFTPGET)); + ut_asserteq(0, net_boot_file_size); + return 0; +} + +static int dm_test_lwip_net_loop(struct unit_test_state *uts) +{ + static const char * const vars[] = { + "ethact", "ipaddr", "netmask", "serverip", "tftpserverip", + "loadaddr", "bootfile", "filesize", "fileaddr", "autostart", "tftpdstp", + ".flags", + }; + char bootfile[sizeof(net_boot_file_name)]; + ulong load_addr = image_load_addr; + u32 size = net_boot_file_size; + char *saved[ARRAY_SIZE(vars)] = {}; + int i, mode, ret = -ENOMEM; + + for (i = 0; i < ARRAY_SIZE(vars); i++) { + const char *value = env_get(vars[i]); + + if (value) { + saved[i] = strdup(value); + if (!saved[i]) + goto out; + } + } + memcpy(bootfile, net_boot_file_name, sizeof(bootfile)); + sandbox_eth_set_tx_handler(0, net_loop_reply); + sandbox_eth_set_priv(0, &mode); + ret = net_loop_check(uts, &mode); + sandbox_eth_set_tx_handler(0, NULL); + sandbox_eth_set_priv(0, NULL); + env_set(".flags", NULL); + memcpy(net_boot_file_name, bootfile, sizeof(bootfile)); + image_load_addr = load_addr; + net_boot_file_size = size; + for (i = 0; i < ARRAY_SIZE(vars); i++) + if (env_set(vars[i], saved[i])) + ret = -EINVAL; +out: + for (i = 0; i < ARRAY_SIZE(vars); i++) + free(saved[i]); + return ret; +} +DM_TEST(dm_test_lwip_net_loop, UTF_SCAN_FDT); -- 2.53.0
