| Message ID | 20210101011759.21086-2-hauke@hauke-m.de |
|---|---|
| State | Superseded |
| Delegated to: | Hauke Mehrtens |
| Headers | show |
| Series | [uhttpd,1/3] cmake: Use variables | expand |
Hi, pá 1. 1. 2021 v 2:20 odesílatel Hauke Mehrtens <hauke@hauke-m.de> 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 <hauke@hauke-m.de> > --- > 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
pá 1. 1. 2021 v 11:02 odesílatel Oldřich Jedlička <oldium.pro@gmail.com> napsal: > > Hi, > > pá 1. 1. 2021 v 2:20 odesílatel Hauke Mehrtens <hauke@hauke-m.de> 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 <hauke@hauke-m.de> > > --- > > 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
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 <oldium.pro@gmail.com> napsal: >> >> Hi, >> >> pá 1. 1. 2021 v 2:20 odesílatel Hauke Mehrtens <hauke@hauke-m.de> 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 <hauke@hauke-m.de> >>> --- .... >>> }; >>> @@ -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
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)) + 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;
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 <hauke@hauke-m.de> --- listen.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)