| Message ID | 20230908135320.7066-4-maxim.uvarov@linaro.org |
|---|---|
| State | Changes Requested |
| Delegated to: | Ramon Fried |
| Headers | show |
| Series | net/lwip: add lwip library for the network stack | expand |
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@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@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() > + > + 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 >
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@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@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 >>
On Wed, 13 Sept 2023 at 14:32, Simon Goldschmidt <goldsimon@gmx.de> 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 <maxim.uvarov@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@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 > > 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 > >> >
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@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); + + 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; +}
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@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