* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox