* [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
* [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
* 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
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 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.