> 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.

--
Sergey Kandaurov

_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org