Re: [musl] Incompatible behaviour of res_query(3) w.r.t. NXDOMAIN

[Rich Felker <dalias@libc.org>]

[-- Attachment #1: Type: text/plain, Size: 1300 bytes --]

On Mon, Aug 24, 2020 at 06:09:23PM -0400, Rich Felker wrote:
> On Mon, Aug 24, 2020 at 11:51:52PM +0200, Daniel Neri wrote:
> > On 24 Aug 2020, at 23:32, Rich Felker <dalias@libc.org> wrote:
> > > 
> > > Does such a distinction exist? I thought res_query was just equivalent
> > > to res_mkquery+res_send and that calling res_send directly would get
> > > you the same errors.
> > 
> > I thought so too, but I’ve been reading the musl implementation. ;-)
> > 
> > After looking more at the other implementations, I think Florian is
> > correct though: it’s more like res_mkquery+res_send+setting h_errno
> > and the return value based on the RCODE of the response.
> 
> If this is indeed the case and the right behavior can be obtained
> reliably by ignoring res_query and using res_mkquery+res_send, I have
> no fundamental objection to changing this. However we should have a
> plan for moving h_errno to be thread-local and figuring out what
> breakage if any there could be for apps built with it global.
> 
> Thankfully, it looks like we're already using (*__h_errno_location())
> even though it was not thread-local, so existing apps built against
> current musl headers should be immediately compatible with changing it
> to be thread-local.

I have the attached patches queued now.

Rich

[-- Attachment #2: 0001-make-h_errno-thread-local.patch --]
[-- Type: text/plain, Size: 1413 bytes --]

From 9d0b8b92a508c328e7eac774847f001f80dfb5ff Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
Date: Mon, 24 Aug 2020 21:38:49 -0400
Subject: [PATCH 1/2] make h_errno thread-local

the framework to do this always existed but it was deemed unnecessary
because the only [ex-]standard functions using h_errno were not
thread-safe anyway. however, some of the nonstandard res_* functions
are also supposed to set h_errno to indicate the cause of error, and
were unable to do so because it was not thread-safe. this change is a
prerequisite for fixing them.
---
 src/internal/pthread_impl.h | 1 +
 src/network/h_errno.c       | 6 ++----
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/internal/pthread_impl.h b/src/internal/pthread_impl.h
index 5742dfc5..5749a336 100644
--- a/src/internal/pthread_impl.h
+++ b/src/internal/pthread_impl.h
@@ -43,6 +43,7 @@ struct pthread {
 		long off;
 		volatile void *volatile pending;
 	} robust_list;
+	int h_errno_val;
 	volatile int timer_id;
 	locale_t locale;
 	volatile int killlock[1];
diff --git a/src/network/h_errno.c b/src/network/h_errno.c
index 4f700cea..8677a92b 100644
--- a/src/network/h_errno.c
+++ b/src/network/h_errno.c
@@ -1,9 +1,7 @@
 #include <netdb.h>
-
-#undef h_errno
-int h_errno;
+#include "pthread_impl.h"
 
 int *__h_errno_location(void)
 {
-	return &h_errno;
+	return &__pthread_self()->h_errno_val;
 }
-- 
2.21.0


[-- Attachment #3: 0002-report-res_query-failures-including-nxdomain-nodata-.patch --]
[-- Type: text/plain, Size: 1554 bytes --]

From 19f8642494b7d27b2ceed5c14d4a0b27cb749afe Mon Sep 17 00:00:00 2001
From: Rich Felker <dalias@aerifal.cx>
Date: Mon, 24 Aug 2020 21:56:48 -0400
Subject: [PATCH 2/2] report res_query failures, including nxdomain/nodata, via
 h_errno

while it's not clearly documented anywhere, this is the historical
behavior which some applications expect. applications which need to
see the response packet in these cases, for example to distinguish
between nonexistence in a secure vs insecure zone, must already use
res_mkquery with res_send in order to be portable, since most if not
all other implementations of res_query don't provide it.
---
 src/network/res_query.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/network/res_query.c b/src/network/res_query.c
index 2f4da2e2..506dc231 100644
--- a/src/network/res_query.c
+++ b/src/network/res_query.c
@@ -1,3 +1,4 @@
+#define _BSD_SOURCE
 #include <resolv.h>
 #include <netdb.h>
 
@@ -6,7 +7,20 @@ int res_query(const char *name, int class, int type, unsigned char *dest, int le
 	unsigned char q[280];
 	int ql = __res_mkquery(0, name, class, type, 0, 0, 0, q, sizeof q);
 	if (ql < 0) return ql;
-	return __res_send(q, ql, dest, len);
+	int r = __res_send(q, ql, dest, len);
+	if (r<12) {
+		h_errno = TRY_AGAIN;
+		return -1;
+	}
+	if ((dest[3] & 15) == 3) {
+		h_errno = HOST_NOT_FOUND;
+		return -1;
+	}
+	if ((dest[3] & 15) == 0 && !dest[6] && !dest[7]) {
+		h_errno = NO_DATA;
+		return -1;
+	}
+	return r;
 }
 
 weak_alias(res_query, res_search);
-- 
2.21.0