* [PATCH 0/3] net: lwip: fix use-after-free bugs in the wget and dns commands
@ 2026-08-11 18:48 Shahriyar Jalayeri
2026-08-11 18:48 ` [PATCH 1/3] net: lwip: wget: return ERR_ABRT after aborting the connection Shahriyar Jalayeri
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Shahriyar Jalayeri @ 2026-08-11 18:48 UTC (permalink / raw)
To: u-boot, Jerome Forissier
Cc: Tom Rini, David Lechner, Argus, Shahriyar Jalayeri
Three use-after-free bugs in the lwIP wget and dns command glue.
Patch 1 fixes httpc_recv_cb(), which calls altcp_abort() on a store_block()
failure but returns ERR_BUF instead of ERR_ABRT, so tcp_input() keeps using
the freed pcb.
Patch 2 fixes wget, which keeps its transfer context on the stack and hands
its address to the httpc callbacks. Ctrl-C while the host name is still
resolving leaves the connection up, and a later callback runs against the
freed stack frame, writing attacker-controlled data through store_block().
The context moves to the heap with an ownership handoff to the lwIP callback.
Patch 3 fixes the same stack-lifetime bug in the dns command, for its
dns_gethostbyname() callback argument.
A reproducer is available on request.
Signed-off-by: Shahriyar Jalayeri <shahriyar@byteray.co.uk>
---
Shahriyar Jalayeri (3):
net: lwip: wget: return ERR_ABRT after aborting the connection
net: lwip: wget: free the transfer context after an aborted request
net: lwip: dns: free the callback context after an aborted lookup
net/lwip/dns.c | 38 ++++++++++++++++++++++------
net/lwip/wget.c | 77 ++++++++++++++++++++++++++++++++++++++++++++-------------
2 files changed, 91 insertions(+), 24 deletions(-)
---
base-commit: baa64b2f892890f00a377eac4a3e685472bb56b5
change-id: 20260811-lwip-httpc-uaf-31bf85307bce
Best regards,
--
Shahriyar Jalayeri <shahriyar@byteray.co.uk>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/3] net: lwip: wget: return ERR_ABRT after aborting the connection 2026-08-11 18:48 [PATCH 0/3] net: lwip: fix use-after-free bugs in the wget and dns commands Shahriyar Jalayeri @ 2026-08-11 18:48 ` Shahriyar Jalayeri 2026-08-11 18:48 ` [PATCH 2/3] net: lwip: wget: free the transfer context after an aborted request Shahriyar Jalayeri 2026-08-11 18:48 ` [PATCH 3/3] net: lwip: dns: free the callback context after an aborted lookup Shahriyar Jalayeri 2 siblings, 0 replies; 5+ messages in thread From: Shahriyar Jalayeri @ 2026-08-11 18:48 UTC (permalink / raw) To: u-boot, Jerome Forissier Cc: Tom Rini, David Lechner, Argus, Shahriyar Jalayeri On a store_block() failure httpc_recv_cb() calls altcp_abort(), which frees the pcb, and then returns ERR_BUF. lwIP's receive-callback contract requires ERR_ABRT once tcp_abort() has been called. On any other return value tcp_input() keeps using the freed pcb (for example it stores the segment in pcb->refused_data), a use-after-free. Return ERR_ABRT so tcp_input() stops touching the pcb. Fixes: 3c656c928bd7 ("net: lwip: add wget command") Signed-off-by: Shahriyar Jalayeri <shahriyar@byteray.co.uk> --- net/lwip/wget.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/net/lwip/wget.c b/net/lwip/wget.c index 247ece18e2b..9e93765926d 100644 --- a/net/lwip/wget.c +++ b/net/lwip/wget.c @@ -205,8 +205,12 @@ static err_t httpc_recv_cb(void *arg, struct altcp_pcb *pcb, struct pbuf *pbuf, for (buf = pbuf; buf; buf = buf->next) { if (store_block(ctx, buf->payload, buf->len) < 0) { + /* + * altcp_abort() freed the pcb; the recv callback must + * return ERR_ABRT so tcp_input() stops using it. + */ altcp_abort(pcb); - ret = ERR_BUF; + ret = ERR_ABRT; goto out; } } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] net: lwip: wget: free the transfer context after an aborted request 2026-08-11 18:48 [PATCH 0/3] net: lwip: fix use-after-free bugs in the wget and dns commands Shahriyar Jalayeri 2026-08-11 18:48 ` [PATCH 1/3] net: lwip: wget: return ERR_ABRT after aborting the connection Shahriyar Jalayeri @ 2026-08-11 18:48 ` Shahriyar Jalayeri 2026-08-12 13:38 ` Jerome Forissier 2026-08-11 18:48 ` [PATCH 3/3] net: lwip: dns: free the callback context after an aborted lookup Shahriyar Jalayeri 2 siblings, 1 reply; 5+ messages in thread From: Shahriyar Jalayeri @ 2026-08-11 18:48 UTC (permalink / raw) To: u-boot, Jerome Forissier Cc: Tom Rini, David Lechner, Argus, Shahriyar Jalayeri wget_do_request() keeps the transfer context on the stack and passes its address to the lwIP httpc callbacks. If the user interrupts the transfer with Ctrl-C while the host name is still resolving, wget_handle_request() leaves its receive loop without tearing the connection down. The pending DNS lookup (which lwIP cannot cancel) later resolves, the connection is established, and httpc_recv_cb() runs against a stack frame that no longer exists. It writes attacker-controlled TCP data through store_block() to map_sysmem(ctx->daddr), with ctx read from reused stack. Allocate the context on the heap and add an 'abandoned' flag. On Ctrl-C with a request still in flight, mark it abandoned and hand ownership to the lwIP callback, which frees the context when the connection finally tears down; the receive, headers-done and result callbacks return early so a stale callback neither stores data nor touches wget_info. Fixes: 3c656c928bd7 ("net: lwip: add wget command") Signed-off-by: Shahriyar Jalayeri <shahriyar@byteray.co.uk> --- net/lwip/wget.c | 71 ++++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 16 deletions(-) diff --git a/net/lwip/wget.c b/net/lwip/wget.c index 9e93765926d..b81509dff3f 100644 --- a/net/lwip/wget.c +++ b/net/lwip/wget.c @@ -12,6 +12,7 @@ #include <lwip/errno.h> #include <lwip/timeouts.h> #include <rng.h> +#include <malloc.h> #include <mapmem.h> #include <net.h> #include <time.h> @@ -39,6 +40,7 @@ struct wget_ctx { ulong content_len; ulong hash_count; enum done_state done; + bool abandoned; }; static void wget_lwip_fill_info(struct pbuf *hdr, u16_t hdr_len, u32_t hdr_cont_len) @@ -200,6 +202,13 @@ static err_t httpc_recv_cb(void *arg, struct altcp_pcb *pcb, struct pbuf *pbuf, if (!pbuf) return ERR_BUF; + /* The caller gave up on this request; drop the connection. */ + if (ctx->abandoned) { + altcp_abort(pcb); + pbuf_free(pbuf); + return ERR_ABRT; + } + if (!ctx->start_time) ctx->start_time = get_timer(0); @@ -227,6 +236,12 @@ static void httpc_result_cb(void *arg, httpc_result_t httpc_result, struct wget_ctx *ctx = arg; ulong elapsed; + /* Last callback for an abandoned request: reclaim ctx and stop. */ + if (ctx->abandoned) { + free(ctx); + return; + } + wget_info->status_code = (u32)srv_res; if (err == ERR_BUF) { @@ -282,6 +297,9 @@ static err_t httpc_headers_done_cb(httpc_state_t *connection, void *arg, struct { struct wget_ctx *ctx = arg; + if (ctx->abandoned) + return ERR_BUF; + wget_lwip_fill_info(hdr, hdr_len, content_len); if (wget_info->check_buffer_size && (ulong)content_len > wget_info->buffer_size) @@ -376,8 +394,17 @@ static int wget_handle_request(struct wget_ctx *ctx, bool is_https, while (!ctx->done) { net_lwip_rx(udev, netif); - if (ctrlc()) + if (ctrlc()) { + /* + * A request may still be in flight (e.g. the name is + * still resolving). Hand ctx to the lwIP callback, which + * frees it once the connection tears down, instead of + * freeing it here under a live callback. + */ + if (!ctx->done) + ctx->abandoned = true; break; + } } if (ctx->done == SUCCESS) @@ -392,27 +419,26 @@ static int wget_handle_request(struct wget_ctx *ctx, bool is_https, int wget_do_request(ulong dst_addr, char *uri) { struct udevice *udev; - struct wget_ctx ctx; + struct wget_ctx *ctx; struct netif *netif; - bool is_https; + bool is_https, abandoned; int ret; - ctx.daddr = dst_addr; - ctx.saved_daddr = dst_addr; - ctx.done = NOT_DONE; - ctx.size = 0; - ctx.prevsize = 0; - ctx.start_time = 0; - ctx.content_len = 0; - ctx.hash_count = 0; + ctx = calloc(1, sizeof(*ctx)); + if (!ctx) + return -ENOMEM; + + ctx->daddr = dst_addr; + ctx->saved_daddr = dst_addr; + ctx->done = NOT_DONE; - ret = parse_url(uri, ctx.server_name, &ctx.port, &ctx.path, &is_https); + ret = parse_url(uri, ctx->server_name, &ctx->port, &ctx->path, &is_https); if (ret) - return ret; + goto out; ret = net_lwip_eth_start(); if (ret) - return ret; + goto out; if (!wget_info) wget_info = &default_wget_info; @@ -422,14 +448,27 @@ int wget_do_request(ulong dst_addr, char *uri) netif = net_lwip_new_netif(udev); if (!netif) { net_lwip_eth_stop(); - return -ENODEV; + ret = -ENODEV; + goto out; } - ret = wget_handle_request(&ctx, is_https, udev, netif); + ret = wget_handle_request(ctx, is_https, udev, netif); + + /* + * If the request was abandoned the lwIP callback still owns ctx and + * frees it when the connection tears down; do not free it here. + */ + abandoned = ctx->abandoned; net_lwip_remove_netif(netif); net_lwip_eth_stop(); + if (!abandoned) + free(ctx); + + return ret; +out: + free(ctx); return ret; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/3] net: lwip: wget: free the transfer context after an aborted request 2026-08-11 18:48 ` [PATCH 2/3] net: lwip: wget: free the transfer context after an aborted request Shahriyar Jalayeri @ 2026-08-12 13:38 ` Jerome Forissier 0 siblings, 0 replies; 5+ messages in thread From: Jerome Forissier @ 2026-08-12 13:38 UTC (permalink / raw) To: Shahriyar Jalayeri, u-boot; +Cc: Tom Rini, David Lechner, Argus, nd Hi Shahriar, On 11/08/2026 20:48, Shahriyar Jalayeri wrote: > wget_do_request() keeps the transfer context on the stack and passes its > address to the lwIP httpc callbacks. If the user interrupts the transfer > with Ctrl-C while the host name is still resolving, wget_handle_request() > leaves its receive loop without tearing the connection down. The pending > DNS lookup (which lwIP cannot cancel) later resolves, the connection is > established, and httpc_recv_cb() runs against a stack frame that no longer > exists. It writes attacker-controlled TCP data through store_block() to > map_sysmem(ctx->daddr), with ctx read from reused stack. > > Allocate the context on the heap and add an 'abandoned' flag. On Ctrl-C > with a request still in flight, mark it abandoned and hand ownership to > the lwIP callback, which frees the context when the connection finally > tears down; the receive, headers-done and result callbacks return early > so a stale callback neither stores data nor touches wget_info. > > Fixes: 3c656c928bd7 ("net: lwip: add wget command") > Signed-off-by: Shahriyar Jalayeri <shahriyar@byteray.co.uk> > --- > net/lwip/wget.c | 71 ++++++++++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 55 insertions(+), 16 deletions(-) > > diff --git a/net/lwip/wget.c b/net/lwip/wget.c > index 9e93765926d..b81509dff3f 100644 > --- a/net/lwip/wget.c > +++ b/net/lwip/wget.c > @@ -12,6 +12,7 @@ > #include <lwip/errno.h> > #include <lwip/timeouts.h> > #include <rng.h> > +#include <malloc.h> > #include <mapmem.h> > #include <net.h> > #include <time.h> > @@ -39,6 +40,7 @@ struct wget_ctx { > ulong content_len; > ulong hash_count; > enum done_state done; > + bool abandoned; > }; > > static void wget_lwip_fill_info(struct pbuf *hdr, u16_t hdr_len, u32_t hdr_cont_len) > @@ -200,6 +202,13 @@ static err_t httpc_recv_cb(void *arg, struct altcp_pcb *pcb, struct pbuf *pbuf, > if (!pbuf) > return ERR_BUF; > > + /* The caller gave up on this request; drop the connection. */ > + if (ctx->abandoned) { > + altcp_abort(pcb); > + pbuf_free(pbuf); > + return ERR_ABRT; > + } > + > if (!ctx->start_time) > ctx->start_time = get_timer(0); > > @@ -227,6 +236,12 @@ static void httpc_result_cb(void *arg, httpc_result_t httpc_result, > struct wget_ctx *ctx = arg; > ulong elapsed; > > + /* Last callback for an abandoned request: reclaim ctx and stop. */ > + if (ctx->abandoned) { > + free(ctx); > + return; > + } > + > wget_info->status_code = (u32)srv_res; > > if (err == ERR_BUF) { > @@ -282,6 +297,9 @@ static err_t httpc_headers_done_cb(httpc_state_t *connection, void *arg, struct > { > struct wget_ctx *ctx = arg; > > + if (ctx->abandoned) > + return ERR_BUF; > + > wget_lwip_fill_info(hdr, hdr_len, content_len); > > if (wget_info->check_buffer_size && (ulong)content_len > wget_info->buffer_size) > @@ -376,8 +394,17 @@ static int wget_handle_request(struct wget_ctx *ctx, bool is_https, > > while (!ctx->done) { > net_lwip_rx(udev, netif); > - if (ctrlc()) > + if (ctrlc()) { > + /* > + * A request may still be in flight (e.g. the name is > + * still resolving). Hand ctx to the lwIP callback, which > + * frees it once the connection tears down, instead of > + * freeing it here under a live callback. > + */ > + if (!ctx->done) > + ctx->abandoned = true; One more object seems to have the same lifetime issue here: httpc_connection_t conn is stack-allocated in wget_handle_request(), but httpc_get_file_dns() keeps a pointer to it in the HTTP client state and later dereferences it for the headers/result callbacks. So if Ctrl-C makes wget_handle_request() return while DNS/connect is still pending, ctx remains valid but conn does not. Could conn be moved into struct wget_ctx (or otherwise made to live as long as the request) as well? > break; > + } > } > > if (ctx->done == SUCCESS) > @@ -392,27 +419,26 @@ static int wget_handle_request(struct wget_ctx *ctx, bool is_https, > int wget_do_request(ulong dst_addr, char *uri) > { > struct udevice *udev; > - struct wget_ctx ctx; > + struct wget_ctx *ctx; > struct netif *netif; > - bool is_https; > + bool is_https, abandoned; > int ret; > > - ctx.daddr = dst_addr; > - ctx.saved_daddr = dst_addr; > - ctx.done = NOT_DONE; > - ctx.size = 0; > - ctx.prevsize = 0; > - ctx.start_time = 0; > - ctx.content_len = 0; > - ctx.hash_count = 0; > + ctx = calloc(1, sizeof(*ctx)); > + if (!ctx) > + return -ENOMEM; > + > + ctx->daddr = dst_addr; > + ctx->saved_daddr = dst_addr; > + ctx->done = NOT_DONE; > > - ret = parse_url(uri, ctx.server_name, &ctx.port, &ctx.path, &is_https); > + ret = parse_url(uri, ctx->server_name, &ctx->port, &ctx->path, &is_https); > if (ret) > - return ret; > + goto out; > > ret = net_lwip_eth_start(); > if (ret) > - return ret; > + goto out; > > if (!wget_info) > wget_info = &default_wget_info; > @@ -422,14 +448,27 @@ int wget_do_request(ulong dst_addr, char *uri) > netif = net_lwip_new_netif(udev); > if (!netif) { > net_lwip_eth_stop(); > - return -ENODEV; > + ret = -ENODEV; > + goto out; > } > > - ret = wget_handle_request(&ctx, is_https, udev, netif); > + ret = wget_handle_request(ctx, is_https, udev, netif); > + > + /* > + * If the request was abandoned the lwIP callback still owns ctx and > + * frees it when the connection tears down; do not free it here. > + */ > + abandoned = ctx->abandoned; > > net_lwip_remove_netif(netif); > net_lwip_eth_stop(); > > + if (!abandoned) > + free(ctx); > + > + return ret; > +out: > + free(ctx); > return ret; > } Thanks, -- Jerome ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 3/3] net: lwip: dns: free the callback context after an aborted lookup 2026-08-11 18:48 [PATCH 0/3] net: lwip: fix use-after-free bugs in the wget and dns commands Shahriyar Jalayeri 2026-08-11 18:48 ` [PATCH 1/3] net: lwip: wget: return ERR_ABRT after aborting the connection Shahriyar Jalayeri 2026-08-11 18:48 ` [PATCH 2/3] net: lwip: wget: free the transfer context after an aborted request Shahriyar Jalayeri @ 2026-08-11 18:48 ` Shahriyar Jalayeri 2 siblings, 0 replies; 5+ messages in thread From: Shahriyar Jalayeri @ 2026-08-11 18:48 UTC (permalink / raw) To: u-boot, Jerome Forissier Cc: Tom Rini, David Lechner, Argus, Shahriyar Jalayeri do_dns() keeps its dns_cb_arg on the stack and registers its address as the dns_gethostbyname() callback argument. lwIP has no way to cancel a pending lookup, so if the command is interrupted or times out while the name is still resolving, dns_cb() fires later and writes the result into the stack frame that has since been reused. Allocate dns_cb_arg on the heap and add an 'abandoned' flag. If the lookup did not complete, hand ownership to dns_cb(), which frees the context when it eventually fires; otherwise free it before returning. Fixes: aedcfec9ed78 ("net: lwip: add dns command") Signed-off-by: Shahriyar Jalayeri <shahriyar@byteray.co.uk> --- net/lwip/dns.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/net/lwip/dns.c b/net/lwip/dns.c index b620b0611d6..e93720963b8 100644 --- a/net/lwip/dns.c +++ b/net/lwip/dns.c @@ -4,6 +4,7 @@ #include <command.h> #include <console.h> #include <env.h> +#include <malloc.h> #include <lwip/dns.h> #include <lwip/timeouts.h> #include <net.h> @@ -15,6 +16,7 @@ struct dns_cb_arg { ip_addr_t host_ipaddr; bool done; + bool abandoned; }; static void do_dns_tmr(void *arg) @@ -26,6 +28,12 @@ static void dns_cb(const char *name, const ip_addr_t *ipaddr, void *arg) { struct dns_cb_arg *dns_cb_arg = arg; + /* Late callback for an abandoned lookup: reclaim and stop. */ + if (dns_cb_arg->abandoned) { + free(dns_cb_arg); + return; + } + dns_cb_arg->done = true; if (!ipaddr) @@ -36,7 +44,7 @@ static void dns_cb(const char *name, const ip_addr_t *ipaddr, void *arg) static int dns_loop(struct udevice *udev, const char *name, const char *var) { - struct dns_cb_arg dns_cb_arg = { }; + struct dns_cb_arg *dns_cb_arg; struct netif *netif; const char *ipstr; ip_addr_t ipaddr; @@ -52,18 +60,22 @@ static int dns_loop(struct udevice *udev, const char *name, const char *var) return CMD_RET_FAILURE; } - dns_cb_arg.done = false; + dns_cb_arg = calloc(1, sizeof(*dns_cb_arg)); + if (!dns_cb_arg) { + net_lwip_remove_netif(netif); + return CMD_RET_FAILURE; + } - ret = dns_gethostbyname(name, &ipaddr, dns_cb, &dns_cb_arg); + ret = dns_gethostbyname(name, &ipaddr, dns_cb, dns_cb_arg); if (ret == ERR_OK) { - dns_cb(name, &ipaddr, &dns_cb_arg); + dns_cb(name, &ipaddr, dns_cb_arg); } else if (ret == ERR_INPROGRESS) { start = get_timer(0); sys_timeout(DNS_RESEND_MS, do_dns_tmr, NULL); do { net_lwip_rx(udev, netif); - if (dns_cb_arg.done) + if (dns_cb_arg->done) break; if (ctrlc()) { printf("\nAbort\n"); @@ -75,15 +87,27 @@ static int dns_loop(struct udevice *udev, const char *name, const char *var) net_lwip_remove_netif(netif); - if (dns_cb_arg.done && !ip_addr_isany(&dns_cb_arg.host_ipaddr)) { - ipstr = ipaddr_ntoa(&dns_cb_arg.host_ipaddr); + /* + * A started lookup that never completed may still be pending in lwIP + * and reference dns_cb_arg. Hand ownership to dns_cb, which frees it + * when it eventually fires, instead of freeing it here. + */ + if (ret == ERR_INPROGRESS && !dns_cb_arg->done) { + dns_cb_arg->abandoned = true; + return CMD_RET_FAILURE; + } + + if (dns_cb_arg->done && !ip_addr_isany(&dns_cb_arg->host_ipaddr)) { + ipstr = ipaddr_ntoa(&dns_cb_arg->host_ipaddr); if (var) env_set(var, ipstr); else printf("%s\n", ipstr); + free(dns_cb_arg); return CMD_RET_SUCCESS; } + free(dns_cb_arg); return CMD_RET_FAILURE; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-08-12 13:39 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-11 18:48 [PATCH 0/3] net: lwip: fix use-after-free bugs in the wget and dns commands Shahriyar Jalayeri 2026-08-11 18:48 ` [PATCH 1/3] net: lwip: wget: return ERR_ABRT after aborting the connection Shahriyar Jalayeri 2026-08-11 18:48 ` [PATCH 2/3] net: lwip: wget: free the transfer context after an aborted request Shahriyar Jalayeri 2026-08-12 13:38 ` Jerome Forissier 2026-08-11 18:48 ` [PATCH 3/3] net: lwip: dns: free the callback context after an aborted lookup Shahriyar Jalayeri
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox