Re: [PATCH] Test status before h_errno in gaih_inet

[Florian Weimer <fweimer at redhat dot com>]

On 07/25/2016 06:50 PM, Stan Shebs wrote:
> Per feedback on my previous attempt to fix getaddrinfo failures
> after recovering from a failure:
>
> https://sourceware.org/ml/libc-alpha/2016-07/msg00341.html
>
> here is a simpler patch that checks status before checking
> h_errno.
>
> The patch basically adds back a test that was previously
> present.  I am not 100% convinced that it catches all cases
> of NSS status being set in a way that ought to be reported
> as an error coming from getaddrinfo, but I can't find
> any actual examples.
>
> 2016-07-25  Stan Shebs  <stanshebs@google.com>
>
>         * sysdeps/posix/getaddrinfo.c (gaih_inet): Test status before
>         looking at h_errno.
>         * posix/tst-getaddrinfo6.c: New test.
>         * posix/Makefile (tests): Add tst-getaddrinfo6.

It's unclear whether the test case is intended to run against nss_files, 
nss_dns, or both.  You should call __nss_configure_lookup to make your 
selection explicit.  This also avoids accidentally running against a 
system nscd daemon.

I'm still trying to figure out what the expected error reporting 
behavior for functions returning enum nss_status is.  There seems to be 
some expectation that enum nss_status != NSS_STATUS_SUCCESS implies that 
h_errno is valid, but this code in getaddrinfo.c itself contradicts that:

      status = NSS_STATUS_UNAVAIL;
      /* Could not load any of the lookup functions.  Indicate
         an internal error if the failure was due to a system
	 error other than the file not being found.  We use the
	 errno from the last failed callback.  */
      if (errno != 0 && errno != ENOENT)
	__set_h_errno (NETDB_INTERNAL);

It would have to set h_errno unconditionally in order to preserve the 
invariant.

The _nss_files_gethostbyname3_r implementation in nss_files calls 
internal_setent, but does not update *herrnop for status != 
NSS_STATUS_SUCCESS.  This is in contrast to _nss_files_gethostbyname4_r, 
which does.

I need to dig further and write up what I find, but I suspect that we 
may have to set h_errno to 0 temporarily to obtain maximum compatibility 
with existing NSS modules, and base the error check on that.

Thanks,
Florian