Bug 12269 - nss_wins has incorrect function definitions for gethostbyname*
nss_wins has incorrect function definitions for gethostbyname*
Status: RESOLVED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Winbind
4.5.0
All Linux
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-09-16 12:40 UTC by Florian Weimer
Modified: 2016-12-01 10:24 UTC (History)
5 users (show)

See Also:


Attachments
patch for 4.5 (4.63 KB, patch)
2016-09-20 08:27 UTC, Andreas Schneider
jmcd: review-
Details
patch for 4.4 (4.63 KB, patch)
2016-09-20 08:28 UTC, Andreas Schneider
jmcd: review-
Details
patch for 4.4 (7.27 KB, patch)
2016-09-22 09:21 UTC, Andreas Schneider
jmcd: review+
Details
patch for 4.5 (7.27 KB, patch)
2016-09-22 09:21 UTC, Andreas Schneider
jmcd: review+
Details
patch for master (791 bytes, patch)
2016-11-13 16:41 UTC, Andreas Schneider
no flags Details
additional patch for 4.5 (905 bytes, patch)
2016-11-23 06:18 UTC, Andreas Schneider
jra: review+
Details
additional patch for 4.4 (905 bytes, patch)
2016-11-23 06:19 UTC, Andreas Schneider
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Weimer 2016-09-16 12:40:44 UTC
From nsswitch/wins.c:

NSS_STATUS _nss_wins_gethostbyname_r(const char *hostname, struct hostent *he,
                          char *buffer, size_t buflen, int *h_errnop);
NSS_STATUS _nss_wins_gethostbyname2_r(const char *name, int af, struct hostent *he,
                           char *buffer, size_t buflen, int *h_errnop);

These prototypes miss the int *errnop argument.

This was originally reported as a glibc bug:

  https://sourceware.org/bugzilla/show_bug.cgi?id=20612
Comment 1 Andreas Schneider 2016-09-20 08:27:03 UTC
Created attachment 12495 [details]
patch for 4.5
Comment 2 Andreas Schneider 2016-09-20 08:28:15 UTC
Created attachment 12497 [details]
patch for 4.4
Comment 3 Florian Weimer 2016-09-20 08:57:53 UTC
I think you need to set both *h_errnop and *errnop on all errors.  h_errno == NETDB_INTERNAL basically means “look at errno”, but there are some more specific error codes as well.
Comment 4 Jim McDonough 2016-09-20 17:37:10 UTC
Comment on attachment 12497 [details]
patch for 4.4

needs the h_errnop updates
Comment 5 Jim McDonough 2016-09-20 17:39:38 UTC
Comment on attachment 12495 [details]
patch for 4.5

needs the h_errnop updates
Comment 6 Andreas Schneider 2016-09-22 09:21:24 UTC
Created attachment 12503 [details]
patch for 4.4
Comment 7 Andreas Schneider 2016-09-22 09:21:45 UTC
Created attachment 12504 [details]
patch for 4.5
Comment 8 Andreas Schneider 2016-09-22 14:09:13 UTC
Karolin, please add the patches to the relevant branches. Thanks!
Comment 9 Andreas Schneider 2016-09-22 14:09:34 UTC
Florian, thanks for the report!!
Comment 10 Karolin Seeger 2016-09-23 09:41:56 UTC
Pushed to autobuild-v4-{5,4-}test.
Comment 11 Karolin Seeger 2016-10-25 07:41:19 UTC
(In reply to Karolin Seeger from comment #10)
Pushed to both branches.
Closing out bug report.

Thanks!
Comment 12 Mathieu Parent 2016-11-12 22:43:24 UTC
Hi,

It looks like this has broken something. See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=825380#15

The herrno == NETDB_INTERNAL (of _nss_wins_gethostbyname_r, when ip==NULL), leads to EAI_SYSTEM (http://sources.debian.net/src/glibc/2.24-5/sysdeps/posix/getaddrinfo.c/?hl=317#L266).

Maybe it should be *h_errnop = HOST_NOT_FOUND?
Comment 13 Mathieu Parent 2016-11-12 22:47:07 UTC
(In reply to Mathieu Parent from comment #12)

and also *errnop = ENOENT
Comment 14 Andreas Schneider 2016-11-13 16:41:51 UTC
Created attachment 12657 [details]
patch for master

Can you try with this patch? It should apply to 4.5 and 4.5 too.
Comment 15 Mathieu Parent 2016-11-14 20:22:28 UTC
(In reply to Andreas Schneider from comment #14)

Quoting original reporter: "That patch seems to have resolved the System error issue.".

My question: Why don't you set *errnop =? (additional question: is this nss API documented somewhere?)
Comment 16 Andreas Schneider 2016-11-15 06:44:22 UTC
> My question: Why don't you set *errnop =? (additional question: is this nss API
> documented somewhere?)

It seems that glibc doesn't that it in that case, because it isn't an 'error' but a 'good' return value.

The documentation is the glibc code :(
Comment 17 Florian Weimer 2016-11-15 14:22:33 UTC
The glibc manual has some documentation:

https://www.gnu.org/software/libc/manual/html_node/NSS-Module-Internals.html

The master sources in Git have some crucial fixes:

git clone https://sourceware.org/git/glibc.git

These changes to not reflect implementation changes, the documentation was simply wrong before.

Regarding which error code to return, this entirely depends on the kind failure.  Is the entry just missing?  Or is the service unavailable?  Is the lack of service just temporary, or persistent?
Comment 18 Andreas Schneider 2016-11-23 06:18:54 UTC
Created attachment 12683 [details]
additional patch for 4.5
Comment 19 Andreas Schneider 2016-11-23 06:19:50 UTC
Created attachment 12684 [details]
additional patch for 4.4
Comment 20 Jeremy Allison 2016-11-28 22:29:43 UTC
Reassigning to Karolin for inclusion in 4.5.next, 4.4.next.
Comment 21 Karolin Seeger 2016-11-30 11:19:24 UTC
(In reply to Jeremy Allison from comment #20)
Pushed to autobuild-v4-{4,5}-test.
Comment 22 Karolin Seeger 2016-12-01 10:24:37 UTC
(In reply to Karolin Seeger from comment #21)
Pushed to both branches.
Closing out bug report.

Thanks!