Hello!
On Wed, Nov 23, 2022 at 07:03:32PM +0400, Sergey Kandaurov wrote:
>
> > On 23 Nov 2022, at 18:46, Maxim Dounin <mdounin@mdounin.ru> wrote:
> >
> > 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@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@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
> >
>
> [..]
>
> Looks good.
Pushed to http://mdounin.ru/hg/nginx, thanks.
--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org
