Re: [PATCHv8 03/15] net/lwip: implement dns cmd
On Wed, 13 Sept 2023 at 14:32, Simon Goldschmidt wrote:
>
>
> 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
> >> ---
> >> 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 00..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 00..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.
> >> + */
> >> +
> >> +#include
> >> +#include
> >> +#include
> >> +
> >> +#include
> >> +#include
> >> +
> >> +#include
> >> +
> >> +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
>
>
Yes, I looked for an ipv4 case with null check. But I think here we can go
with:
if (dns2env && ipaddr_aton(dns2env, &dns2))
> >
> >> +
> >> +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
> >>
>
Re: [PATCHv8 03/15] net/lwip: implement dns cmd
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
---
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 00..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 00..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.
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+
+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
Re: [PATCHv8 03/15] net/lwip: implement dns cmd
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
> ---
> 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 00..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 00..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.
> + */
> +
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +#include
> +
> +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()
> +
> + 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
>
[PATCHv8 03/15] net/lwip: implement dns cmd
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
---
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 00..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 00..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.
+ */
+
+#include
+#include
+#include
+
+#include
+#include
+
+#include
+
+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);
+
+ 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
