Re: family inet[46]" and NULL for getaddrinfo()

On 2026-06-25 06:56 +09, YASUOKA Masahiko <[email protected]> wrote:
> Hi,
>
> On Tue, 23 Jun 2026 20:03:58 +0200
> Florian Obser <[email protected]> wrote:
>> On 2025-12-13 14:02 +09, YASUOKA Masahiko <[email protected]> wrote:
>>> Hello,
>>>
>>> On Wed, 17 Sep 2025 12:00:09 +0900 (JST)
>>> YASUOKA Masahiko <[email protected]> wrote:
>>>> On Tue, 16 Sep 2025 13:27:36 +0200
>>>> Florian Obser <[email protected]> wrote:
>>>>> Coming back to an old email.
>>>>> 
>>>>> On 2025-03-07 09:43 +09, YASUOKA Masahiko <[email protected]> wrote:
>>>>>> Hello,
>>>>>>
>>>>>> sshd's config has the following default values,
>>>>>>
>>>>>>   #AddressFamily any
>>>>>>   #ListenAddress 0.0.0.0
>>>>>>   #ListenAddress ::
>>>>>>
>>>>>> If we configure "family" in /etc/resolv.conf, sshd will stop listening
>>>>>> 0.0.0.0 or :: following the "family" config.
>>>>>>
>>>>>>   - "family inet4" => stop listening on ::
>>>>>>   - "family inet6" => stop listening on 0.0.0.0
>>>>>>
>>>>>> This is because getaddrinfo() doesn't convert a numeric address if the
>>>>>> family is disabled and NI_NUMERIC_HOST is not specified.
>>>>>>
>>>>>> I don't think this is correct behavior.
>>>>> 
>>>>> I don't think this is correct behaviour either.
>>>
>>> Other than this problem, getaddrinfo can receive NULL for hostname.
>>> Then returning "::" and "0.0.0.0" is expected when AI_PASSIVE.  I
>>> suppose that "family" in "resolv.conf" must not affect this behavior
>>> in this case as well, but our resolver changes the result.  (Only
>>> "0.0.0.0" is returned when "family inet4" is configured).
>>>
>>> Also, because the default sshd_config doesn't have "ListenAddress"
>>> line, the previous diff hasn't fixed the original problem of sshd.
>>>
>>> The diff fixes the problem.  It prepares the constant variables for
>>> IPv6 but not for IPv4 because I avoided to use htonl() in the
>>> initializers.
>>>
>> 
>> I think (and I've also checked with deraadt at g2k26) that using htonl()
>> in the initializers is fine, it's a local variable.
>
> Thanks for the review.  I updated the diff so it use const variables
> for IPv4 as well.
>
>> I'd prefer to have the v4 case symmetric to the v6 case.
>> 
>> I agree the change makes sense and I also like the simplification of the
>> code.
>> 
>>> We can reproduce the problem in regress/lib/libc/getaddrinfo/ by
>>> modifying "/etc/resolv.conf".
>
> ok?

I think you can drop the comment about "statically built sockaddrs". If
I'm not mistaken you had removed it from the previous version of the
diff.

Either way, OK florian

>
> Index: lib/libc/asr/getaddrinfo_async.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/asr/getaddrinfo_async.c,v
> diff -u -p -r1.67 getaddrinfo_async.c
> --- lib/libc/asr/getaddrinfo_async.c	23 Jun 2026 11:36:36 -0000	1.67
> +++ lib/libc/asr/getaddrinfo_async.c	24 Jun 2026 21:44:22 -0000
> @@ -43,7 +43,6 @@ struct match {
>  
>  static int getaddrinfo_async_run(struct asr_query *, struct asr_result *);
>  static int get_port(const char *, const char *, int);
> -static int iter_family(struct asr_query *, int);
>  static int addrinfo_add(struct asr_query *, const struct sockaddr *, const char *);
>  static int addrinfo_from_file(struct asr_query *, int,  FILE *);
>  static int addrinfo_from_pkt(struct asr_query *, char *, size_t);
> @@ -123,6 +122,26 @@ getaddrinfo_async_run(struct asr_query *
>  		struct sockaddr_in	sain;
>  		struct sockaddr_in6	sain6;
>  	} sa;
> +	const struct sockaddr_in	sockaddr_in_any = {
> +		.sin_family = AF_INET,
> +		.sin_len = sizeof(struct sockaddr_in),
> +		.sin_addr = htonl(INADDR_ANY)
> +	};
> +	const struct sockaddr_in	sockaddr_in_lo = {
> +		.sin_family = AF_INET,
> +		.sin_len = sizeof(struct sockaddr_in),
> +		.sin_addr = htonl(INADDR_LOOPBACK)
> +	};
> +	const struct sockaddr_in6	sockaddr_in6_any = {
> +		.sin6_family = AF_INET6,
> +		.sin6_len = sizeof(struct sockaddr_in6),
> +		.sin6_addr = IN6ADDR_ANY_INIT
> +	};
> +	const struct sockaddr_in6	sockaddr_in6_lo = {
> +		.sin6_family = AF_INET6,
> +		.sin6_len = sizeof(struct sockaddr_in6),
> +		.sin6_addr = IN6ADDR_LOOPBACK_INIT
> +	};
>  
>      next:
>  	switch (as->as_state) {
> @@ -231,6 +250,8 @@ getaddrinfo_async_run(struct asr_query *
>  
>  		if (!(ai->ai_flags & AI_NUMERICHOST))
>  			is_localhost = _asr_is_localhost(as->as.ai.hostname);
> +		if (as->as.ai.hostname == NULL)
> +			is_localhost = (ai->ai_flags & AI_PASSIVE)? 0 : 1;
>  		/*
>  		 * If hostname is NULL, "localhost" or falls within the
>  		 * ".localhost." domain, use local address.
> @@ -243,27 +264,26 @@ getaddrinfo_async_run(struct asr_query *
>  		 * DNS server(s).
>  		 */
>  		if (as->as.ai.hostname == NULL || is_localhost) {
> -			for (family = iter_family(as, 1);
> -			    family != -1;
> -			    family = iter_family(as, 0)) {
> -				/*
> -				 * We could use statically built sockaddrs for
> -				 * those, rather than parsing over and over.
> -				 */
> -				if (family == PF_INET)
> -					str = (ai->ai_flags & AI_PASSIVE &&
> -					    !is_localhost) ? "0.0.0.0" :
> -					    "127.0.0.1";
> -				else /* PF_INET6 */
> -					str = (ai->ai_flags & AI_PASSIVE &&
> -					    !is_localhost) ? "::" : "::1";
> -				 /* This can't fail */
> -				_asr_sockaddr_from_str(&sa.sa, family, str);
> -				if ((r = addrinfo_add(as, &sa.sa,
> -				    "localhost."))) {
> +			/*
> +			 * We could use statically built sockaddrs for
> +			 * those, rather than parsing over and over.
> +			 */
> +			if (ai->ai_family == AF_UNSPEC ||
> +			    ai->ai_family == AF_INET) {
> +				if ((r = addrinfo_add(as,
> +				    (const struct sockaddr *)((is_localhost)?
> +				    &sockaddr_in_lo : &sockaddr_in_any),
> +				    "localhost.")))
> +					ar->ar_gai_errno = r;
> +			}
> +			if (ar->ar_gai_errno == 0 &&
> +			    (ai->ai_family == AF_UNSPEC ||
> +			    ai->ai_family == AF_INET6)) {
> +				if ((r = addrinfo_add(as,
> +				    (const struct sockaddr *)((is_localhost)?
> +				    &sockaddr_in6_lo : &sockaddr_in6_any),
> +				    "localhost.")))
>  					ar->ar_gai_errno = r;
> -					break;
> -				}
>  			}
>  			if (ar->ar_gai_errno == 0 && as->as_count == 0) {
>  				ar->ar_gai_errno = EAI_NODATA;
> @@ -507,28 +527,6 @@ get_port(const char *servname, const cha
>  	endservent_r(&sed);
>  
>  	return (port);
> -}
> -
> -/*
> - * Iterate over the address families that are to be queried. Use the
> - * list on the async context, unless a specific family was given in hints.
> - */
> -static int
> -iter_family(struct asr_query *as, int first)
> -{
> -	if (first) {
> -		as->as_family_idx = 0;
> -		if (as->as.ai.hints.ai_family != PF_UNSPEC)
> -			return as->as.ai.hints.ai_family;
> -		return AS_FAMILY(as);
> -	}
> -
> -	if (as->as.ai.hints.ai_family != PF_UNSPEC)
> -		return (-1);
> -
> -	as->as_family_idx++;
> -
> -	return AS_FAMILY(as);
>  }
>  
>  /*
>

-- 
In my defence, I have been left unsupervised.