[PATCHv8 03/15] net/lwip: implement dns cmd

[Simon Goldschmidt]

On 13.09.2023 07:56, Ilias Apalodimas wrote:
> On Fri, Sep 08, 2023 at 07:53:08PM +0600, Maxim Uvarov wrote:
>> U-Boot recently got support for an alternative network stack using LWIP.
>> Replace dns command with the LWIP variant while keeping the output and
>> error messages identical.
>>
>> Signed-off-by: Maxim Uvarov <maxim.uvarov at linaro.org>
>> ---
>>   include/net/lwip.h           | 19 +++++++++++
>>   net/lwip/Makefile            |  2 ++
>>   net/lwip/apps/dns/lwip-dns.c | 63 ++++++++++++++++++++++++++++++++++++
>>   3 files changed, 84 insertions(+)
>>   create mode 100644 include/net/lwip.h
>>   create mode 100644 net/lwip/apps/dns/lwip-dns.c
>>
>> diff --git a/include/net/lwip.h b/include/net/lwip.h
>> new file mode 100644
>> index 0000000000..ab3db1a214
>> --- /dev/null
>> +++ b/include/net/lwip.h
>> @@ -0,0 +1,19 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int argc,
>> +		char *const argv[]);
>> +
>> +/**
>> + * ulwip_dns() - creates the DNS request to resolve a domain host name
>> + *
>> + * This function creates the DNS request to resolve a domain host name. Function
>> + * can return immediately if previous request was cached or it might require
>> + * entering the polling loop for a request to a remote server.
>> + *
>> + * @name:    dns name to resolve
>> + * @varname: (optional) U-Boot variable name to store the result
>> + * Returns: ERR_OK(0) for fetching entry from the cache
>> + *          -EINPROGRESS success, can go to the polling loop
>> + *          Other value < 0, if error
>> + */
>> +int ulwip_dns(char *name, char *varname);
>> diff --git a/net/lwip/Makefile b/net/lwip/Makefile
>> index 3fd5d34564..5d8d5527c6 100644
>> --- a/net/lwip/Makefile
>> +++ b/net/lwip/Makefile
>> @@ -62,3 +62,5 @@ obj-$(CONFIG_NET) += lwip-external/src/netif/ethernet.o
>>
>>   obj-$(CONFIG_NET) += port/if.o
>>   obj-$(CONFIG_NET) += port/sys-arch.o
>> +
>> +obj-y += apps/dns/lwip-dns.o
>> diff --git a/net/lwip/apps/dns/lwip-dns.c b/net/lwip/apps/dns/lwip-dns.c
>> new file mode 100644
>> index 0000000000..b340302f2c
>> --- /dev/null
>> +++ b/net/lwip/apps/dns/lwip-dns.c
>> @@ -0,0 +1,63 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +/*
>> + * (C) Copyright 2023 Linaro Ltd. <maxim.uvarov at linaro.org>
>> + */
>> +
>> +#include <common.h>
>> +#include <command.h>
>> +#include <console.h>
>> +
>> +#include <lwip/dns.h>
>> +#include <lwip/ip_addr.h>
>> +
>> +#include <net/ulwip.h>
>> +
>> +static void dns_found_cb(const char *name, const ip_addr_t *ipaddr, void *callback_arg)
>> +{
>> +	char *varname = (char *)callback_arg;
>> +	char *ipstr = ip4addr_ntoa(ipaddr);
>> +
>> +	if (varname)
>> +		env_set(varname, ipstr);
>> +	log_info("resolved %s to %s\n",  name, ipstr);
>> +	ulwip_exit(0);
>> +}
>> +
>> +int ulwip_dns(char *name, char *varname)
>> +{
>> +	int err;
>> +	ip_addr_t ipaddr; /* not used */
>> +	ip_addr_t dns1;
>> +	ip_addr_t dns2;
>> +	char *dnsenv = env_get("dnsip");
>> +	char *dns2env = env_get("dnsip2");
>> +
>> +	if (!dnsenv && !dns2env) {
>> +		log_err("nameserver is not set with dnsip and dnsip2 vars\n");
>> +		return -ENOENT;
>> +	}
>> +
>> +	if (!dnsenv)
>> +		log_warning("dnsip var is not set\n");
>> +	if (!dns2env)
>> +		log_warning("dnsip2 var is not set\n");
>> +
>> +	dns_init();
>> +
>> +	if (ipaddr_aton(dnsenv, &dns1))
>> +		dns_setserver(0, &dns1);
>> +
>> +	if (ipaddr_aton(dns2env, &dns2))
>> +		dns_setserver(1, &dns2);
>
> env_get will return NULL if any of these is not set.  Looking at
> ipaddr_aton() of lwip that might lead to a NULL deref in ip_2_ip6()

Looking at the NULL checks in ipaddr_aton(), you found a bug in lwIP.
I'd vote to leave the above code as is and rely on the bug being fixed
in lwIP before U-Boot enables IPv6 (this is only a bug in dual-stack
mode where IPv4 and IPv6 is enabled).

Regards,
Simon

>
>> +
>> +	err = dns_gethostbyname(name, &ipaddr, dns_found_cb, varname);
>> +	if (err == ERR_OK)
>> +		dns_found_cb(name, &ipaddr, varname);
>> +
>> +	/* convert lwIP ERR_INPROGRESS to U-Boot -EINPROGRESS */
>> +	if (err == ERR_INPROGRESS)
>> +		err = -EINPROGRESS;
>> +
>> +	return err;
>> +}
>> --
>> 2.30.2
>>