[PATCH] Filtering duplicate addresses in listen (ticket #2400)

[Sergey Kandaurov]

> On 23 Nov 2022, at 18:46, Maxim Dounin <mdounin at 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 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
> 

[..]

Looks good.

-- 
Sergey Kandaurov