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 <[email protected]>
---
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
