Re: [PATCH uhttpd 2/3] listen: Copy only parts of ai_addr
Hi,
pá 1. 1. 2021 v 2:20 odesílatel Hauke Mehrtens napsal:
>
> ai_addr has different sizes for IPv4 and IPv6. Only copy the parts which
> are actually used and not the full array, to not copy and uninitialized
> memory.
>
> This fixes a warning found with the address sanitizer.
>
> Signed-off-by: Hauke Mehrtens
> ---
> listen.c | 15 ++-
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/listen.c b/listen.c
> index 2a54888..558e35e 100644
> --- a/listen.c
> +++ b/listen.c
> @@ -29,7 +29,10 @@ struct listener {
> struct uloop_fd fd;
> int socket;
> int n_clients;
> - struct sockaddr_in6 addr;
> + union {
> + struct sockaddr_in6 addr6;
> + struct sockaddr_in addr;
> + };
> bool tls;
> bool blocked;
> };
> @@ -189,7 +192,9 @@ int uh_socket_bind(const char *host, const char *port,
> bool tls)
>
> l->fd.fd = sock;
> l->tls = tls;
> - l->addr = *(struct sockaddr_in6 *)p->ai_addr;
> + if (p->ai_addrlen < sizeof(l->addr))
I think that this sizeof takes the smallest part of the (anonymous)
union - sockaddr_in addr, not the whole union size. You either need to
name the union and sizeof the whole content, or manually select the
biggest entry for use in sizeof, which is addr6.
> + goto error;
> + memcpy(&l->addr, p->ai_addr, p->ai_addrlen);
> list_add_tail(&l->list, &listeners);
> bound++;
>
> @@ -211,13 +216,13 @@ int uh_first_tls_port(int family)
> int tls_port = -1;
>
> list_for_each_entry(l, &listeners, list) {
> - if (!l->tls || l->addr.sin6_family != family)
> + if (!l->tls || l->addr.sin_family != family)
> continue;
>
> - if (tls_port != -1 && ntohs(l->addr.sin6_port) != 443)
> + if (tls_port != -1 && ntohs(l->addr.sin_port) != 443)
> continue;
>
> - tls_port = ntohs(l->addr.sin6_port);
> + tls_port = ntohs(l->addr.sin_port);
> }
>
> return tls_port;
> --
> 2.20.1
>
>
> ___
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [PATCH uhttpd 2/3] listen: Copy only parts of ai_addr
pá 1. 1. 2021 v 11:02 odesílatel Oldřich Jedlička napsal:
>
> Hi,
>
> pá 1. 1. 2021 v 2:20 odesílatel Hauke Mehrtens napsal:
> >
> > ai_addr has different sizes for IPv4 and IPv6. Only copy the parts which
> > are actually used and not the full array, to not copy and uninitialized
> > memory.
> >
> > This fixes a warning found with the address sanitizer.
> >
> > Signed-off-by: Hauke Mehrtens
> > ---
> > listen.c | 15 ++-
> > 1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/listen.c b/listen.c
> > index 2a54888..558e35e 100644
> > --- a/listen.c
> > +++ b/listen.c
> > @@ -29,7 +29,10 @@ struct listener {
> > struct uloop_fd fd;
> > int socket;
> > int n_clients;
> > - struct sockaddr_in6 addr;
> > + union {
> > + struct sockaddr_in6 addr6;
> > + struct sockaddr_in addr;
> > + };
> > bool tls;
> > bool blocked;
> > };
> > @@ -189,7 +192,9 @@ int uh_socket_bind(const char *host, const char *port,
> > bool tls)
> >
> > l->fd.fd = sock;
> > l->tls = tls;
> > - l->addr = *(struct sockaddr_in6 *)p->ai_addr;
> > + if (p->ai_addrlen < sizeof(l->addr))
>
> I think that this sizeof takes the smallest part of the (anonymous)
> union - sockaddr_in addr, not the whole union size. You either need to
> name the union and sizeof the whole content, or manually select the
> biggest entry for use in sizeof, which is addr6.
Also strict less-than doesn't look correct.
>
> > + goto error;
> > + memcpy(&l->addr, p->ai_addr, p->ai_addrlen);
> > list_add_tail(&l->list, &listeners);
> > bound++;
> >
> > @@ -211,13 +216,13 @@ int uh_first_tls_port(int family)
> > int tls_port = -1;
> >
> > list_for_each_entry(l, &listeners, list) {
> > - if (!l->tls || l->addr.sin6_family != family)
> > + if (!l->tls || l->addr.sin_family != family)
> > continue;
> >
> > - if (tls_port != -1 && ntohs(l->addr.sin6_port) != 443)
> > + if (tls_port != -1 && ntohs(l->addr.sin_port) != 443)
> > continue;
> >
> > - tls_port = ntohs(l->addr.sin6_port);
> > + tls_port = ntohs(l->addr.sin_port);
> > }
> >
> > return tls_port;
> > --
> > 2.20.1
> >
> >
> > ___
> > openwrt-devel mailing list
> > openwrt-devel@lists.openwrt.org
> > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: [PATCH uhttpd 2/3] listen: Copy only parts of ai_addr
On 1/1/21 11:22 AM, Oldřich Jedlička wrote: pá 1. 1. 2021 v 11:02 odesílatel Oldřich Jedlička napsal: Hi, pá 1. 1. 2021 v 2:20 odesílatel Hauke Mehrtens napsal: ai_addr has different sizes for IPv4 and IPv6. Only copy the parts which are actually used and not the full array, to not copy and uninitialized memory. This fixes a warning found with the address sanitizer. Signed-off-by: Hauke Mehrtens --- }; @@ -189,7 +192,9 @@ int uh_socket_bind(const char *host, const char *port, bool tls) l->fd.fd = sock; l->tls = tls; - l->addr = *(struct sockaddr_in6 *)p->ai_addr; + if (p->ai_addrlen < sizeof(l->addr)) I think that this sizeof takes the smallest part of the (anonymous) union - sockaddr_in addr, not the whole union size. You either need to name the union and sizeof the whole content, or manually select the biggest entry for use in sizeof, which is addr6. Also strict less-than doesn't look correct. Yes you are right, I will send an update. I changed this code multiple times and had this laying around for some time now. Hauke ___ openwrt-devel mailing list openwrt-devel@lists.openwrt.org https://lists.openwrt.org/mailman/listinfo/openwrt-devel
