Re: [ping2][PATCH] Fix incorrect getaddrinfo assertion trigger

[Allan McRae <allan at archlinux dot org>]

Ping!

On 18/09/13 18:05, Allan McRae wrote:
> Ping.
> 
> On 30/08/13 22:05, Allan McRae wrote:
>> [BS #9954]
>>
>> With the following /etc/hosts:
>> 127.0.0.1       www.my-domain.es
>> 127.0.1.1       www.my-domain.es
>> 192.168.0.1     www.my-domain.es
>>
>> Using getaddrinfo() on www.my-domain.es, trigger the following assertion:
>> ../sysdeps/posix/getaddrinfo.c:1473: rfc3484_sort: Assertion
>> `src->results[i].native == -1 || src->results[i].native == a1_native' failed.
>>
>> This is due to two different bugs:
>> - In rfc3484_sort() rule 7, src->results[i].native is assigned even if
>> src->results[i].index is -1, meaning that no interface is associated.
>> - In getaddrinfo() the source IP address used with the lo interface needs a
>> special case, as it can be any IP within 127.X.Y.Z.
>>
>> ---
>>
>> This is a patch taken from bugzilla.  I can not replicate it myself, even
>> with the attached testcase on bugzilla.  However, I have users reporting this
>> issue that also can confirm this patch fixes the assertions for them.  I
>> believe this patch is widely used by other distributions (at least Debian
>> and Fedora).
>>
>> There is a negative comment by Ulrich here:
>> https://sourceware.org/bugzilla/show_bug.cgi?id=9954#c2
>>
>> I think that comment is partially correct, in that it is hiding a bug from
>> a faulty DNS provider, in which case this patch is correct in that it
>> improves robustness.  That is why this bug does not affect most people.
>> However, I am not fully confident about that assessment...
>>
>> Can anyone replicate or confirm/correct my reasoning why most people can
>> not replicate?
>>
>>
>>
>> 20xx-xx-xx  Aurelien Jarno  <aurelien@aurel32.net>
>>
>> 	[BS #9954]
>> 	* sysdeps/posix/getaddrinfo.c (rfc3484_sort): do not assign native
>> 	result if the result has no associated interface.
>> 	* sysdeps/posix/getaddrinfo.c (getaddrinfo): correctly detect 
>> 	interface for all 127.X.Y.Z addresses.
>>
>>
>>  sysdeps/posix/getaddrinfo.c | 13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
>> index 52177e4..c0ded84 100644
>> --- a/sysdeps/posix/getaddrinfo.c
>> +++ b/sysdeps/posix/getaddrinfo.c
>> @@ -1735,13 +1735,13 @@ rfc3484_sort (const void *p1, const void *p2, void *arg)
>>  
>>  	      /* Fill in the results in all the records.  */
>>  	      for (int i = 0; i < src->nresults; ++i)
>> -		if (src->results[i].index == a1_index)
>> +		if (a1_index != -1 && src->results[i].index == a1_index)
>>  		  {
>>  		    assert (src->results[i].native == -1
>>  			    || src->results[i].native == a1_native);
>>  		    src->results[i].native = a1_native;
>>  		  }
>> -		else if (src->results[i].index == a2_index)
>> +		else if (a2_index != -1 && src->results[i].index == a2_index)
>>  		  {
>>  		    assert (src->results[i].native == -1
>>  			    || src->results[i].native == a2_native);
>> @@ -2601,7 +2601,14 @@ getaddrinfo (const char *name, const char *service,
>>  			  tmp.addr[0] = 0;
>>  			  tmp.addr[1] = 0;
>>  			  tmp.addr[2] = htonl (0xffff);
>> -			  tmp.addr[3] = sinp->sin_addr.s_addr;
>> +			  /* Special case for lo interface, the source address
>> +			     being possibly different than the interface 
>> +			     address. */
>> +			  if ((ntohl(sinp->sin_addr.s_addr) & 0xff000000)
>> +			      == 0x7f000000)
>> +			    tmp.addr[3] = htonl(0x7f000001);
>> +			  else
>> +			    tmp.addr[3] = sinp->sin_addr.s_addr;
>>  			}
>>  		      else
>>  			{
>>
> 
> 
>