[PATCH] Filtering duplicate addresses in listen (ticket #2400)
Maxim Dounin
mdounin at mdounin.ru
Wed Nov 23 14:46:23 UTC 2022
Hello!
On Wed, Nov 23, 2022 at 04:56:10PM +0400, Sergey Kandaurov wrote:
> On Sun, Oct 30, 2022 at 05:41:00AM +0300, Maxim Dounin wrote:
> > # HG changeset patch
> > # User Maxim Dounin <mdounin at mdounin.ru>
> > # Date 1667097653 -10800
> > # Sun Oct 30 05:40:53 2022 +0300
> > # Node ID 55bcf8dc4ee35ccf40f5b8a7cffde63e7edb9494
> > # Parent 1ae25660c0c76edef14121ca64362f28b9d57a70
> > Filtering duplicate addresses in listen (ticket #2400).
> >
> > Due to the glibc bug[1], getaddrinfo("localhost") with AI_ADDRCONFIG
> > on a typical host with glibc and without IPv6 returns two 127.0.0.1
> > addresses, and therefore "listen localhost:80;" used to result the
>
> result in ?
Fixed, thnx.
> > "duplicate ... address and port pair" after 4f9b72a229c1.
> >
> > Fix is to explicitly filter out duplicate addresses returned during
> > resolution of a name.
> >
> > [1] https://sourceware.org/bugzilla/show_bug.cgi?id=14969
> >
> > diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
> > --- a/src/http/ngx_http_core_module.c
> > +++ b/src/http/ngx_http_core_module.c
> > @@ -3963,7 +3963,7 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
> >
> > ngx_str_t *value, size;
> > ngx_url_t u;
> > - ngx_uint_t n;
> > + ngx_uint_t n, i;
> > ngx_http_listen_opt_t lsopt;
> >
> > cscf->listen = 1;
> > @@ -4289,6 +4289,16 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
> > }
> >
> > for (n = 0; n < u.naddrs; n++) {
> > +
> > + for (i = 0; i < n; i++) {
> > + if (ngx_cmp_sockaddr(u.addrs[n].sockaddr, u.addrs[n].socklen,
> > + u.addrs[i].sockaddr, u.addrs[i].socklen, 0)
> > + == NGX_OK)
> > + {
> > + goto next;
> > + }
> > + }
> > +
> > lsopt.sockaddr = u.addrs[n].sockaddr;
> > lsopt.socklen = u.addrs[n].socklen;
> > lsopt.addr_text = u.addrs[n].name;
> > @@ -4297,6 +4307,9 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
> > if (ngx_http_add_listen(cf, cscf, &lsopt) != NGX_OK) {
> > return NGX_CONF_ERROR;
> > }
> > +
> > + next:
> > + continue;
> > }
> >
> > return NGX_CONF_OK;
> > diff --git a/src/mail/ngx_mail_core_module.c b/src/mail/ngx_mail_core_module.c
> > --- a/src/mail/ngx_mail_core_module.c
> > +++ b/src/mail/ngx_mail_core_module.c
> > @@ -308,7 +308,7 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
> > ngx_str_t *value, size;
> > ngx_url_t u;
> > ngx_uint_t i, n, m;
> > - ngx_mail_listen_t *ls, *als;
> > + ngx_mail_listen_t *ls, *als, *nls;
> > ngx_mail_module_t *module;
> > ngx_mail_core_main_conf_t *cmcf;
> >
> > @@ -333,7 +333,7 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
> >
> > cmcf = ngx_mail_conf_get_module_main_conf(cf, ngx_mail_core_module);
> >
> > - ls = ngx_array_push_n(&cmcf->listen, u.naddrs);
> > + ls = ngx_array_push(&cmcf->listen);
> > if (ls == NULL) {
> > return NGX_CONF_ERROR;
> > }
> > @@ -571,17 +571,37 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
> > als = cmcf->listen.elts;
>
> With push of additional cmcf->listen elements moved inside loop,
> als can become a stale pointer due to reallocation in ngx_array_push().
> As such, als assignment should be kept updated (here and in stream).
>
> Otherwise, looks good.
Sure, thanks for catching this. Updated with the following
additional change:
diff --git a/src/mail/ngx_mail_core_module.c b/src/mail/ngx_mail_core_module.c
--- a/src/mail/ngx_mail_core_module.c
+++ b/src/mail/ngx_mail_core_module.c
@@ -568,8 +568,6 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
return NGX_CONF_ERROR;
}
- als = cmcf->listen.elts;
-
for (n = 0; n < u.naddrs; n++) {
for (i = 0; i < n; i++) {
@@ -598,6 +596,8 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
nls->addr_text = u.addrs[n].name;
nls->wildcard = ngx_inet_wildcard(nls->sockaddr);
+ als = cmcf->listen.elts;
+
for (i = 0; i < cmcf->listen.nelts - 1; i++) {
if (ngx_cmp_sockaddr(als[i].sockaddr, als[i].socklen,
diff --git a/src/stream/ngx_stream_core_module.c b/src/stream/ngx_stream_core_module.c
--- a/src/stream/ngx_stream_core_module.c
+++ b/src/stream/ngx_stream_core_module.c
@@ -886,8 +886,6 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
#endif
}
- als = cmcf->listen.elts;
-
for (n = 0; n < u.naddrs; n++) {
for (i = 0; i < n; i++) {
@@ -916,6 +914,8 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
nls->addr_text = u.addrs[n].name;
nls->wildcard = ngx_inet_wildcard(nls->sockaddr);
+ als = cmcf->listen.elts;
+
for (i = 0; i < cmcf->listen.nelts - 1; i++) {
if (nls->type != als[i].type) {
continue;
Full patch:
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1669213808 -10800
# Wed Nov 23 17:30:08 2022 +0300
# Node ID 4cc2bfeff46c7b7ee2b098f39ebfc1e44754df1e
# Parent b809f53d3f5bd04df36ac338845289d8e60a888b
Filtering duplicate addresses in listen (ticket #2400).
Due to the glibc bug[1], getaddrinfo("localhost") with AI_ADDRCONFIG
on a typical host with glibc and without IPv6 returns two 127.0.0.1
addresses, and therefore "listen localhost:80;" used to result in
"duplicate ... address and port pair" after 4f9b72a229c1.
Fix is to explicitly filter out duplicate addresses returned during
resolution of a name.
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=14969
diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c
+++ b/src/http/ngx_http_core_module.c
@@ -3963,7 +3963,7 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
ngx_str_t *value, size;
ngx_url_t u;
- ngx_uint_t n;
+ ngx_uint_t n, i;
ngx_http_listen_opt_t lsopt;
cscf->listen = 1;
@@ -4289,6 +4289,16 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
}
for (n = 0; n < u.naddrs; n++) {
+
+ for (i = 0; i < n; i++) {
+ if (ngx_cmp_sockaddr(u.addrs[n].sockaddr, u.addrs[n].socklen,
+ u.addrs[i].sockaddr, u.addrs[i].socklen, 0)
+ == NGX_OK)
+ {
+ goto next;
+ }
+ }
+
lsopt.sockaddr = u.addrs[n].sockaddr;
lsopt.socklen = u.addrs[n].socklen;
lsopt.addr_text = u.addrs[n].name;
@@ -4297,6 +4307,9 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
if (ngx_http_add_listen(cf, cscf, &lsopt) != NGX_OK) {
return NGX_CONF_ERROR;
}
+
+ next:
+ continue;
}
return NGX_CONF_OK;
diff --git a/src/mail/ngx_mail_core_module.c b/src/mail/ngx_mail_core_module.c
--- a/src/mail/ngx_mail_core_module.c
+++ b/src/mail/ngx_mail_core_module.c
@@ -308,7 +308,7 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
ngx_str_t *value, size;
ngx_url_t u;
ngx_uint_t i, n, m;
- ngx_mail_listen_t *ls, *als;
+ ngx_mail_listen_t *ls, *als, *nls;
ngx_mail_module_t *module;
ngx_mail_core_main_conf_t *cmcf;
@@ -333,7 +333,7 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
cmcf = ngx_mail_conf_get_module_main_conf(cf, ngx_mail_core_module);
- ls = ngx_array_push_n(&cmcf->listen, u.naddrs);
+ ls = ngx_array_push(&cmcf->listen);
if (ls == NULL) {
return NGX_CONF_ERROR;
}
@@ -568,20 +568,40 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
return NGX_CONF_ERROR;
}
- als = cmcf->listen.elts;
+ for (n = 0; n < u.naddrs; n++) {
- for (n = 0; n < u.naddrs; n++) {
- ls[n] = ls[0];
+ for (i = 0; i < n; i++) {
+ if (ngx_cmp_sockaddr(u.addrs[n].sockaddr, u.addrs[n].socklen,
+ u.addrs[i].sockaddr, u.addrs[i].socklen, 0)
+ == NGX_OK)
+ {
+ goto next;
+ }
+ }
- ls[n].sockaddr = u.addrs[n].sockaddr;
- ls[n].socklen = u.addrs[n].socklen;
- ls[n].addr_text = u.addrs[n].name;
- ls[n].wildcard = ngx_inet_wildcard(ls[n].sockaddr);
+ if (n != 0) {
+ nls = ngx_array_push(&cmcf->listen);
+ if (nls == NULL) {
+ return NGX_CONF_ERROR;
+ }
+
+ *nls = *ls;
- for (i = 0; i < cmcf->listen.nelts - u.naddrs + n; i++) {
+ } else {
+ nls = ls;
+ }
+
+ nls->sockaddr = u.addrs[n].sockaddr;
+ nls->socklen = u.addrs[n].socklen;
+ nls->addr_text = u.addrs[n].name;
+ nls->wildcard = ngx_inet_wildcard(nls->sockaddr);
+
+ als = cmcf->listen.elts;
+
+ for (i = 0; i < cmcf->listen.nelts - 1; i++) {
if (ngx_cmp_sockaddr(als[i].sockaddr, als[i].socklen,
- ls[n].sockaddr, ls[n].socklen, 1)
+ nls->sockaddr, nls->socklen, 1)
!= NGX_OK)
{
continue;
@@ -589,9 +609,12 @@ ngx_mail_core_listen(ngx_conf_t *cf, ngx
ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
"duplicate \"%V\" address and port pair",
- &ls[n].addr_text);
+ &nls->addr_text);
return NGX_CONF_ERROR;
}
+
+ next:
+ continue;
}
return NGX_CONF_OK;
diff --git a/src/stream/ngx_stream_core_module.c b/src/stream/ngx_stream_core_module.c
--- a/src/stream/ngx_stream_core_module.c
+++ b/src/stream/ngx_stream_core_module.c
@@ -578,7 +578,7 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
ngx_str_t *value, size;
ngx_url_t u;
ngx_uint_t i, n, backlog;
- ngx_stream_listen_t *ls, *als;
+ ngx_stream_listen_t *ls, *als, *nls;
ngx_stream_core_main_conf_t *cmcf;
cscf->listen = 1;
@@ -602,7 +602,7 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
cmcf = ngx_stream_conf_get_module_main_conf(cf, ngx_stream_core_module);
- ls = ngx_array_push_n(&cmcf->listen, u.naddrs);
+ ls = ngx_array_push(&cmcf->listen);
if (ls == NULL) {
return NGX_CONF_ERROR;
}
@@ -886,23 +886,43 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
#endif
}
- als = cmcf->listen.elts;
+ for (n = 0; n < u.naddrs; n++) {
- for (n = 0; n < u.naddrs; n++) {
- ls[n] = ls[0];
+ for (i = 0; i < n; i++) {
+ if (ngx_cmp_sockaddr(u.addrs[n].sockaddr, u.addrs[n].socklen,
+ u.addrs[i].sockaddr, u.addrs[i].socklen, 0)
+ == NGX_OK)
+ {
+ goto next;
+ }
+ }
- ls[n].sockaddr = u.addrs[n].sockaddr;
- ls[n].socklen = u.addrs[n].socklen;
- ls[n].addr_text = u.addrs[n].name;
- ls[n].wildcard = ngx_inet_wildcard(ls[n].sockaddr);
+ if (n != 0) {
+ nls = ngx_array_push(&cmcf->listen);
+ if (nls == NULL) {
+ return NGX_CONF_ERROR;
+ }
+
+ *nls = *ls;
- for (i = 0; i < cmcf->listen.nelts - u.naddrs + n; i++) {
- if (ls[n].type != als[i].type) {
+ } else {
+ nls = ls;
+ }
+
+ nls->sockaddr = u.addrs[n].sockaddr;
+ nls->socklen = u.addrs[n].socklen;
+ nls->addr_text = u.addrs[n].name;
+ nls->wildcard = ngx_inet_wildcard(nls->sockaddr);
+
+ als = cmcf->listen.elts;
+
+ for (i = 0; i < cmcf->listen.nelts - 1; i++) {
+ if (nls->type != als[i].type) {
continue;
}
if (ngx_cmp_sockaddr(als[i].sockaddr, als[i].socklen,
- ls[n].sockaddr, ls[n].socklen, 1)
+ nls->sockaddr, nls->socklen, 1)
!= NGX_OK)
{
continue;
@@ -910,9 +930,12 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
"duplicate \"%V\" address and port pair",
- &ls[n].addr_text);
+ &nls->addr_text);
return NGX_CONF_ERROR;
}
+
+ next:
+ continue;
}
return NGX_CONF_OK;
--
Maxim Dounin
http://mdounin.ru/
More information about the nginx-devel
mailing list