Do not fail if one of the two responses to AF_UNSPEC fails (BZ #14308)

Message ID 20140415162902.GA8469@spoyarek.pnq.redhat.com
State Committed
Headers

Commit Message

Siddhesh Poyarekar April 15, 2014, 4:29 p.m. UTC
  AF_UNSPEC results in sending two queries in parallel, one for the A
record and the other for the AAAA record.  If one of these is a
referral, then the query fails, which is wrong.  It should return at
least the one successful response.

The fix has two parts.  The first part makes the referral fall back to
the SERVFAIL path, which results in using the successful response.
There is a bug in that path however, due to which the second part is
necessary.  The bug here is that if the first response is a failure
and the second succeeds, __libc_res_nsearch does not detect that and
assumes a failure.  The case where the first response is a success and
the second fails, works correctly.

This condition is produced by buggy routers, so here's a crude
interposable library that can simulate such a condition.  The library
overrides the recvfrom syscall and modifies the header of the packet
received to reproduce this scenario.  It has two key variables:
mod_packet and first_error.

The mod_packet variable when set to 0, results in odd packets being
modified to be a referral.  When set to 1, even packets are modified
to be a referral.

The first_error causes the first response to be a failure so that a
domain-appended search is performed to test the second part of the
__libc_nsearch fix.

The driver for this fix is a simple getaddrinfo program that does an
AF_UNSPEC query.  I have omitted this since it should be easy to
implement.

I have tested this on x86_64.

The interceptor library source:

/* Override recvfrom and modify the header of the first DNS response to make it
   a referral and reproduce bz #845218.  We have to resort to this ugly hack
   because we cannot make bind return the buggy response of a referral for the
   AAAA record and an authoritative response for the A record.  */
 #define _GNU_SOURCE
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <stdio.h>
 #include <stdbool.h>
 #include <endian.h>
 #include <dlfcn.h>
 #include <stdlib.h>

/* Lifted from resolv/arpa/nameser_compat.h.  */
typedef struct {
    unsigned        id :16;         /*%< query identification number */
 #if BYTE_ORDER == BIG_ENDIAN
    /* fields in third byte */
    unsigned        qr: 1;          /*%< response flag */
    unsigned        opcode: 4;      /*%< purpose of message */
    unsigned        aa: 1;          /*%< authoritive answer */
    unsigned        tc: 1;          /*%< truncated message */
    unsigned        rd: 1;          /*%< recursion desired */
    /* fields
     * in
     * fourth
     * byte
     * */
    unsigned        ra: 1;          /*%< recursion available */
    unsigned        unused :1;      /*%< unused bits (MBZ as of 4.9.3a3) */
    unsigned        ad: 1;          /*%< authentic data from named */
    unsigned        cd: 1;          /*%< checking disabled by resolver */
    unsigned        rcode :4;       /*%< response code */
 #endif
 #if BYTE_ORDER == LITTLE_ENDIAN || BYTE_ORDER == PDP_ENDIAN
    /* fields
     * in
     * third
     * byte
     * */
    unsigned        rd :1;          /*%< recursion desired */
    unsigned        tc :1;          /*%< truncated message */
    unsigned        aa :1;          /*%< authoritive answer */
    unsigned        opcode :4;      /*%< purpose of message */
    unsigned        qr :1;          /*%< response flag */
    /* fields
     * in
     * fourth
     * byte
     * */
    unsigned        rcode :4;       /*%< response code */
    unsigned        cd: 1;          /*%< checking disabled by resolver */
    unsigned        ad: 1;          /*%< authentic data from named */
    unsigned        unused :1;      /*%< unused bits (MBZ as of 4.9.3a3) */
    unsigned        ra :1;          /*%< recursion available */
 #endif
    /* remaining
     * bytes
     * */
    unsigned        qdcount :16;    /*%< number of question entries */
    unsigned        ancount :16;    /*%< number of answer entries */
    unsigned        nscount :16;    /*%< number of authority entries */
    unsigned        arcount :16;    /*%< number of resource entries */
} HEADER;

static int done = 0;

/* Packets to modify.  0 for the odd packets and 1 for even packets.  */
static const int mod_packet = 0;

/* Set to true if the first request should result in an error, resulting in a
   search query.  */
static bool first_error = true;

static ssize_t (*real_recvfrom) (int sockfd, void *buf, size_t len, int flags,
			  struct sockaddr *src_addr, socklen_t *addrlen);

void
__attribute__ ((constructor))
init (void)
{
  real_recvfrom = dlsym (RTLD_NEXT, "recvfrom");

  if (real_recvfrom == NULL)
    {
      printf ("Failed to get reference to recvfrom: %s\n", dlerror ());
      printf ("Cannot simulate test\n");
      abort ();
    }
}

/* Modify the second packet that we receive to set the header in a manner as to
   reproduce BZ #845218.  */
static void
mod_buf (HEADER *h, int port)
{
  if (done % 2 == mod_packet || (first_error && done == 1))
    {
      printf ("(Modifying header)");

      if (first_error && done == 1)
	h->rcode = 3;
      else
	h->rcode = 0;	/* NOERROR == 0.  */
      h->ancount = 0;
      h->aa = 0;
      h->ra = 0;
      h->arcount = 0;
    }
  done++;
}

ssize_t
recvfrom (int sockfd, void *buf, size_t len, int flags,
	  struct sockaddr *src_addr, socklen_t *addrlen)
{
  ssize_t ret = real_recvfrom (sockfd, buf, len, flags, src_addr, addrlen);
  int port = htons (((struct sockaddr_in *) src_addr)->sin_port);
  struct in_addr addr = ((struct sockaddr_in *) src_addr)->sin_addr;
  const char *host = inet_ntoa (addr);
  printf ("\n*** From %s:%d: ", host, port);

  mod_buf (buf, port);

  printf ("returned %zd\n", ret);
  return ret;
}


	[BZ #14308]
	* resolv/res_query.c (__libc_res_nsearch): Return if at least
	one response is valid.
	* resolv/res_send.c (send_dg): Check for validity of other
	response if the current response is a referral.

---
 resolv/res_query.c | 7 +++++--
 resolv/res_send.c  | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)
  

Comments

Siddhesh Poyarekar April 16, 2014, 5:59 a.m. UTC | #1
Forgot one detail:

2014-04-16  Siddhesh Poyarekar  <siddhesh@redhat.com>
	    Atsushi Onoe  <atsushi@onoe.org>
> 
> 	[BZ #14308]
> 	* resolv/res_query.c (__libc_res_nsearch): Return if at least
> 	one response is valid.
> 	* resolv/res_send.c (send_dg): Check for validity of other
> 	response if the current response is a referral.
> 
> ---
>  resolv/res_query.c | 7 +++++--
>  resolv/res_send.c  | 2 +-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/resolv/res_query.c b/resolv/res_query.c
> index a9db837..4e6612c 100644
> --- a/resolv/res_query.c
> +++ b/resolv/res_query.c
> @@ -382,7 +382,9 @@ __libc_res_nsearch(res_state statp,
>  					      answer, anslen, answerp,
>  					      answerp2, nanswerp2, resplen2,
>  					      answerp2_malloced);
> -		if (ret > 0 || trailing_dot)
> +		if (ret > 0 || trailing_dot
> +		    /* If the second response is valid then we use that.  */
> +		    || (ret == 0 && answerp2 != NULL && resplen2 > 0))
>  			return (ret);
>  		saved_herrno = h_errno;
>  		tried_as_is++;
> @@ -422,7 +424,8 @@ __libc_res_nsearch(res_state statp,
>  						      answer, anslen, answerp,
>  						      answerp2, nanswerp2,
>  						      resplen2, answerp2_malloced);
> -			if (ret > 0)
> +			if (ret > 0 || (ret == 0 && answerp2 != NULL
> +					&& resplen2 > 0))
>  				return (ret);
>  
>  			if (answerp && *answerp != answer) {
> diff --git a/resolv/res_send.c b/resolv/res_send.c
> index 60743df..3273d55 100644
> --- a/resolv/res_send.c
> +++ b/resolv/res_send.c
> @@ -1351,6 +1351,7 @@ send_dg(res_state statp,
>  				(*thisresplenp > *thisanssizp)
>  				? *thisanssizp : *thisresplenp);
>  
> +		next_ns:
>  			if (recvresp1 || (buf2 != NULL && recvresp2)) {
>  			  *resplen2 = 0;
>  			  return resplen;
> @@ -1368,7 +1369,6 @@ send_dg(res_state statp,
>  			    goto wait;
>  			  }
>  
> -		next_ns:
>  			__res_iclose(statp, false);
>  			/* don't retry if called from dig */
>  			if (!statp->pfcode)
> -- 
> 1.8.3.1
>
  
Siddhesh Poyarekar April 16, 2014, 7:30 a.m. UTC | #2
Incidentally, this also fixes bz #12994.

Siddhesh

On Tue, Apr 15, 2014 at 09:59:02PM +0530, Siddhesh Poyarekar wrote:
> AF_UNSPEC results in sending two queries in parallel, one for the A
> record and the other for the AAAA record.  If one of these is a
> referral, then the query fails, which is wrong.  It should return at
> least the one successful response.
> 
> The fix has two parts.  The first part makes the referral fall back to
> the SERVFAIL path, which results in using the successful response.
> There is a bug in that path however, due to which the second part is
> necessary.  The bug here is that if the first response is a failure
> and the second succeeds, __libc_res_nsearch does not detect that and
> assumes a failure.  The case where the first response is a success and
> the second fails, works correctly.
> 
> This condition is produced by buggy routers, so here's a crude
> interposable library that can simulate such a condition.  The library
> overrides the recvfrom syscall and modifies the header of the packet
> received to reproduce this scenario.  It has two key variables:
> mod_packet and first_error.
> 
> The mod_packet variable when set to 0, results in odd packets being
> modified to be a referral.  When set to 1, even packets are modified
> to be a referral.
> 
> The first_error causes the first response to be a failure so that a
> domain-appended search is performed to test the second part of the
> __libc_nsearch fix.
> 
> The driver for this fix is a simple getaddrinfo program that does an
> AF_UNSPEC query.  I have omitted this since it should be easy to
> implement.
> 
> I have tested this on x86_64.
> 
> The interceptor library source:
> 
> /* Override recvfrom and modify the header of the first DNS response to make it
>    a referral and reproduce bz #845218.  We have to resort to this ugly hack
>    because we cannot make bind return the buggy response of a referral for the
>    AAAA record and an authoritative response for the A record.  */
>  #define _GNU_SOURCE
>  #include <sys/types.h>
>  #include <sys/socket.h>
>  #include <netinet/in.h>
>  #include <arpa/inet.h>
>  #include <stdio.h>
>  #include <stdbool.h>
>  #include <endian.h>
>  #include <dlfcn.h>
>  #include <stdlib.h>
> 
> /* Lifted from resolv/arpa/nameser_compat.h.  */
> typedef struct {
>     unsigned        id :16;         /*%< query identification number */
>  #if BYTE_ORDER == BIG_ENDIAN
>     /* fields in third byte */
>     unsigned        qr: 1;          /*%< response flag */
>     unsigned        opcode: 4;      /*%< purpose of message */
>     unsigned        aa: 1;          /*%< authoritive answer */
>     unsigned        tc: 1;          /*%< truncated message */
>     unsigned        rd: 1;          /*%< recursion desired */
>     /* fields
>      * in
>      * fourth
>      * byte
>      * */
>     unsigned        ra: 1;          /*%< recursion available */
>     unsigned        unused :1;      /*%< unused bits (MBZ as of 4.9.3a3) */
>     unsigned        ad: 1;          /*%< authentic data from named */
>     unsigned        cd: 1;          /*%< checking disabled by resolver */
>     unsigned        rcode :4;       /*%< response code */
>  #endif
>  #if BYTE_ORDER == LITTLE_ENDIAN || BYTE_ORDER == PDP_ENDIAN
>     /* fields
>      * in
>      * third
>      * byte
>      * */
>     unsigned        rd :1;          /*%< recursion desired */
>     unsigned        tc :1;          /*%< truncated message */
>     unsigned        aa :1;          /*%< authoritive answer */
>     unsigned        opcode :4;      /*%< purpose of message */
>     unsigned        qr :1;          /*%< response flag */
>     /* fields
>      * in
>      * fourth
>      * byte
>      * */
>     unsigned        rcode :4;       /*%< response code */
>     unsigned        cd: 1;          /*%< checking disabled by resolver */
>     unsigned        ad: 1;          /*%< authentic data from named */
>     unsigned        unused :1;      /*%< unused bits (MBZ as of 4.9.3a3) */
>     unsigned        ra :1;          /*%< recursion available */
>  #endif
>     /* remaining
>      * bytes
>      * */
>     unsigned        qdcount :16;    /*%< number of question entries */
>     unsigned        ancount :16;    /*%< number of answer entries */
>     unsigned        nscount :16;    /*%< number of authority entries */
>     unsigned        arcount :16;    /*%< number of resource entries */
> } HEADER;
> 
> static int done = 0;
> 
> /* Packets to modify.  0 for the odd packets and 1 for even packets.  */
> static const int mod_packet = 0;
> 
> /* Set to true if the first request should result in an error, resulting in a
>    search query.  */
> static bool first_error = true;
> 
> static ssize_t (*real_recvfrom) (int sockfd, void *buf, size_t len, int flags,
> 			  struct sockaddr *src_addr, socklen_t *addrlen);
> 
> void
> __attribute__ ((constructor))
> init (void)
> {
>   real_recvfrom = dlsym (RTLD_NEXT, "recvfrom");
> 
>   if (real_recvfrom == NULL)
>     {
>       printf ("Failed to get reference to recvfrom: %s\n", dlerror ());
>       printf ("Cannot simulate test\n");
>       abort ();
>     }
> }
> 
> /* Modify the second packet that we receive to set the header in a manner as to
>    reproduce BZ #845218.  */
> static void
> mod_buf (HEADER *h, int port)
> {
>   if (done % 2 == mod_packet || (first_error && done == 1))
>     {
>       printf ("(Modifying header)");
> 
>       if (first_error && done == 1)
> 	h->rcode = 3;
>       else
> 	h->rcode = 0;	/* NOERROR == 0.  */
>       h->ancount = 0;
>       h->aa = 0;
>       h->ra = 0;
>       h->arcount = 0;
>     }
>   done++;
> }
> 
> ssize_t
> recvfrom (int sockfd, void *buf, size_t len, int flags,
> 	  struct sockaddr *src_addr, socklen_t *addrlen)
> {
>   ssize_t ret = real_recvfrom (sockfd, buf, len, flags, src_addr, addrlen);
>   int port = htons (((struct sockaddr_in *) src_addr)->sin_port);
>   struct in_addr addr = ((struct sockaddr_in *) src_addr)->sin_addr;
>   const char *host = inet_ntoa (addr);
>   printf ("\n*** From %s:%d: ", host, port);
> 
>   mod_buf (buf, port);
> 
>   printf ("returned %zd\n", ret);
>   return ret;
> }
> 
> 
> 	[BZ #14308]
> 	* resolv/res_query.c (__libc_res_nsearch): Return if at least
> 	one response is valid.
> 	* resolv/res_send.c (send_dg): Check for validity of other
> 	response if the current response is a referral.
> 
> ---
>  resolv/res_query.c | 7 +++++--
>  resolv/res_send.c  | 2 +-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/resolv/res_query.c b/resolv/res_query.c
> index a9db837..4e6612c 100644
> --- a/resolv/res_query.c
> +++ b/resolv/res_query.c
> @@ -382,7 +382,9 @@ __libc_res_nsearch(res_state statp,
>  					      answer, anslen, answerp,
>  					      answerp2, nanswerp2, resplen2,
>  					      answerp2_malloced);
> -		if (ret > 0 || trailing_dot)
> +		if (ret > 0 || trailing_dot
> +		    /* If the second response is valid then we use that.  */
> +		    || (ret == 0 && answerp2 != NULL && resplen2 > 0))
>  			return (ret);
>  		saved_herrno = h_errno;
>  		tried_as_is++;
> @@ -422,7 +424,8 @@ __libc_res_nsearch(res_state statp,
>  						      answer, anslen, answerp,
>  						      answerp2, nanswerp2,
>  						      resplen2, answerp2_malloced);
> -			if (ret > 0)
> +			if (ret > 0 || (ret == 0 && answerp2 != NULL
> +					&& resplen2 > 0))
>  				return (ret);
>  
>  			if (answerp && *answerp != answer) {
> diff --git a/resolv/res_send.c b/resolv/res_send.c
> index 60743df..3273d55 100644
> --- a/resolv/res_send.c
> +++ b/resolv/res_send.c
> @@ -1351,6 +1351,7 @@ send_dg(res_state statp,
>  				(*thisresplenp > *thisanssizp)
>  				? *thisanssizp : *thisresplenp);
>  
> +		next_ns:
>  			if (recvresp1 || (buf2 != NULL && recvresp2)) {
>  			  *resplen2 = 0;
>  			  return resplen;
> @@ -1368,7 +1369,6 @@ send_dg(res_state statp,
>  			    goto wait;
>  			  }
>  
> -		next_ns:
>  			__res_iclose(statp, false);
>  			/* don't retry if called from dig */
>  			if (!statp->pfcode)
> -- 
> 1.8.3.1
>
  
Andreas Schwab April 16, 2014, 8:57 a.m. UTC | #3
Siddhesh Poyarekar <siddhesh@redhat.com> writes:

> Incidentally, this also fixes bz #12994.

And 13651?

Andreas.
  
Siddhesh Poyarekar April 16, 2014, 9:11 a.m. UTC | #4
On Wed, Apr 16, 2014 at 10:57:31AM +0200, Andreas Schwab wrote:
> Siddhesh Poyarekar <siddhesh@redhat.com> writes:
> 
> > Incidentally, this also fixes bz #12994.
> 
> And 13651?
> 

Oh yeah, in fact I think it's a duplicate.

Siddhesh
  
Siddhesh Poyarekar April 22, 2014, 9:46 a.m. UTC | #5
Ping!

On Tue, Apr 15, 2014 at 09:59:02PM +0530, Siddhesh Poyarekar wrote:
> AF_UNSPEC results in sending two queries in parallel, one for the A
> record and the other for the AAAA record.  If one of these is a
> referral, then the query fails, which is wrong.  It should return at
> least the one successful response.
> 
> The fix has two parts.  The first part makes the referral fall back to
> the SERVFAIL path, which results in using the successful response.
> There is a bug in that path however, due to which the second part is
> necessary.  The bug here is that if the first response is a failure
> and the second succeeds, __libc_res_nsearch does not detect that and
> assumes a failure.  The case where the first response is a success and
> the second fails, works correctly.
> 
> This condition is produced by buggy routers, so here's a crude
> interposable library that can simulate such a condition.  The library
> overrides the recvfrom syscall and modifies the header of the packet
> received to reproduce this scenario.  It has two key variables:
> mod_packet and first_error.
> 
> The mod_packet variable when set to 0, results in odd packets being
> modified to be a referral.  When set to 1, even packets are modified
> to be a referral.
> 
> The first_error causes the first response to be a failure so that a
> domain-appended search is performed to test the second part of the
> __libc_nsearch fix.
> 
> The driver for this fix is a simple getaddrinfo program that does an
> AF_UNSPEC query.  I have omitted this since it should be easy to
> implement.
> 
> I have tested this on x86_64.
> 
> The interceptor library source:
> 
> /* Override recvfrom and modify the header of the first DNS response to make it
>    a referral and reproduce bz #845218.  We have to resort to this ugly hack
>    because we cannot make bind return the buggy response of a referral for the
>    AAAA record and an authoritative response for the A record.  */
>  #define _GNU_SOURCE
>  #include <sys/types.h>
>  #include <sys/socket.h>
>  #include <netinet/in.h>
>  #include <arpa/inet.h>
>  #include <stdio.h>
>  #include <stdbool.h>
>  #include <endian.h>
>  #include <dlfcn.h>
>  #include <stdlib.h>
> 
> /* Lifted from resolv/arpa/nameser_compat.h.  */
> typedef struct {
>     unsigned        id :16;         /*%< query identification number */
>  #if BYTE_ORDER == BIG_ENDIAN
>     /* fields in third byte */
>     unsigned        qr: 1;          /*%< response flag */
>     unsigned        opcode: 4;      /*%< purpose of message */
>     unsigned        aa: 1;          /*%< authoritive answer */
>     unsigned        tc: 1;          /*%< truncated message */
>     unsigned        rd: 1;          /*%< recursion desired */
>     /* fields
>      * in
>      * fourth
>      * byte
>      * */
>     unsigned        ra: 1;          /*%< recursion available */
>     unsigned        unused :1;      /*%< unused bits (MBZ as of 4.9.3a3) */
>     unsigned        ad: 1;          /*%< authentic data from named */
>     unsigned        cd: 1;          /*%< checking disabled by resolver */
>     unsigned        rcode :4;       /*%< response code */
>  #endif
>  #if BYTE_ORDER == LITTLE_ENDIAN || BYTE_ORDER == PDP_ENDIAN
>     /* fields
>      * in
>      * third
>      * byte
>      * */
>     unsigned        rd :1;          /*%< recursion desired */
>     unsigned        tc :1;          /*%< truncated message */
>     unsigned        aa :1;          /*%< authoritive answer */
>     unsigned        opcode :4;      /*%< purpose of message */
>     unsigned        qr :1;          /*%< response flag */
>     /* fields
>      * in
>      * fourth
>      * byte
>      * */
>     unsigned        rcode :4;       /*%< response code */
>     unsigned        cd: 1;          /*%< checking disabled by resolver */
>     unsigned        ad: 1;          /*%< authentic data from named */
>     unsigned        unused :1;      /*%< unused bits (MBZ as of 4.9.3a3) */
>     unsigned        ra :1;          /*%< recursion available */
>  #endif
>     /* remaining
>      * bytes
>      * */
>     unsigned        qdcount :16;    /*%< number of question entries */
>     unsigned        ancount :16;    /*%< number of answer entries */
>     unsigned        nscount :16;    /*%< number of authority entries */
>     unsigned        arcount :16;    /*%< number of resource entries */
> } HEADER;
> 
> static int done = 0;
> 
> /* Packets to modify.  0 for the odd packets and 1 for even packets.  */
> static const int mod_packet = 0;
> 
> /* Set to true if the first request should result in an error, resulting in a
>    search query.  */
> static bool first_error = true;
> 
> static ssize_t (*real_recvfrom) (int sockfd, void *buf, size_t len, int flags,
> 			  struct sockaddr *src_addr, socklen_t *addrlen);
> 
> void
> __attribute__ ((constructor))
> init (void)
> {
>   real_recvfrom = dlsym (RTLD_NEXT, "recvfrom");
> 
>   if (real_recvfrom == NULL)
>     {
>       printf ("Failed to get reference to recvfrom: %s\n", dlerror ());
>       printf ("Cannot simulate test\n");
>       abort ();
>     }
> }
> 
> /* Modify the second packet that we receive to set the header in a manner as to
>    reproduce BZ #845218.  */
> static void
> mod_buf (HEADER *h, int port)
> {
>   if (done % 2 == mod_packet || (first_error && done == 1))
>     {
>       printf ("(Modifying header)");
> 
>       if (first_error && done == 1)
> 	h->rcode = 3;
>       else
> 	h->rcode = 0;	/* NOERROR == 0.  */
>       h->ancount = 0;
>       h->aa = 0;
>       h->ra = 0;
>       h->arcount = 0;
>     }
>   done++;
> }
> 
> ssize_t
> recvfrom (int sockfd, void *buf, size_t len, int flags,
> 	  struct sockaddr *src_addr, socklen_t *addrlen)
> {
>   ssize_t ret = real_recvfrom (sockfd, buf, len, flags, src_addr, addrlen);
>   int port = htons (((struct sockaddr_in *) src_addr)->sin_port);
>   struct in_addr addr = ((struct sockaddr_in *) src_addr)->sin_addr;
>   const char *host = inet_ntoa (addr);
>   printf ("\n*** From %s:%d: ", host, port);
> 
>   mod_buf (buf, port);
> 
>   printf ("returned %zd\n", ret);
>   return ret;
> }
> 
> 
> 	[BZ #14308]
> 	* resolv/res_query.c (__libc_res_nsearch): Return if at least
> 	one response is valid.
> 	* resolv/res_send.c (send_dg): Check for validity of other
> 	response if the current response is a referral.
> 
> ---
>  resolv/res_query.c | 7 +++++--
>  resolv/res_send.c  | 2 +-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/resolv/res_query.c b/resolv/res_query.c
> index a9db837..4e6612c 100644
> --- a/resolv/res_query.c
> +++ b/resolv/res_query.c
> @@ -382,7 +382,9 @@ __libc_res_nsearch(res_state statp,
>  					      answer, anslen, answerp,
>  					      answerp2, nanswerp2, resplen2,
>  					      answerp2_malloced);
> -		if (ret > 0 || trailing_dot)
> +		if (ret > 0 || trailing_dot
> +		    /* If the second response is valid then we use that.  */
> +		    || (ret == 0 && answerp2 != NULL && resplen2 > 0))
>  			return (ret);
>  		saved_herrno = h_errno;
>  		tried_as_is++;
> @@ -422,7 +424,8 @@ __libc_res_nsearch(res_state statp,
>  						      answer, anslen, answerp,
>  						      answerp2, nanswerp2,
>  						      resplen2, answerp2_malloced);
> -			if (ret > 0)
> +			if (ret > 0 || (ret == 0 && answerp2 != NULL
> +					&& resplen2 > 0))
>  				return (ret);
>  
>  			if (answerp && *answerp != answer) {
> diff --git a/resolv/res_send.c b/resolv/res_send.c
> index 60743df..3273d55 100644
> --- a/resolv/res_send.c
> +++ b/resolv/res_send.c
> @@ -1351,6 +1351,7 @@ send_dg(res_state statp,
>  				(*thisresplenp > *thisanssizp)
>  				? *thisanssizp : *thisresplenp);
>  
> +		next_ns:
>  			if (recvresp1 || (buf2 != NULL && recvresp2)) {
>  			  *resplen2 = 0;
>  			  return resplen;
> @@ -1368,7 +1369,6 @@ send_dg(res_state statp,
>  			    goto wait;
>  			  }
>  
> -		next_ns:
>  			__res_iclose(statp, false);
>  			/* don't retry if called from dig */
>  			if (!statp->pfcode)
> -- 
> 1.8.3.1
>
  
Siddhesh Poyarekar April 28, 2014, 2:49 p.m. UTC | #6
Ping!

On Tue, Apr 22, 2014 at 03:16:00PM +0530, Siddhesh Poyarekar wrote:
> Ping!
> 
> On Tue, Apr 15, 2014 at 09:59:02PM +0530, Siddhesh Poyarekar wrote:
> > AF_UNSPEC results in sending two queries in parallel, one for the A
> > record and the other for the AAAA record.  If one of these is a
> > referral, then the query fails, which is wrong.  It should return at
> > least the one successful response.
> > 
> > The fix has two parts.  The first part makes the referral fall back to
> > the SERVFAIL path, which results in using the successful response.
> > There is a bug in that path however, due to which the second part is
> > necessary.  The bug here is that if the first response is a failure
> > and the second succeeds, __libc_res_nsearch does not detect that and
> > assumes a failure.  The case where the first response is a success and
> > the second fails, works correctly.
> > 
> > This condition is produced by buggy routers, so here's a crude
> > interposable library that can simulate such a condition.  The library
> > overrides the recvfrom syscall and modifies the header of the packet
> > received to reproduce this scenario.  It has two key variables:
> > mod_packet and first_error.
> > 
> > The mod_packet variable when set to 0, results in odd packets being
> > modified to be a referral.  When set to 1, even packets are modified
> > to be a referral.
> > 
> > The first_error causes the first response to be a failure so that a
> > domain-appended search is performed to test the second part of the
> > __libc_nsearch fix.
> > 
> > The driver for this fix is a simple getaddrinfo program that does an
> > AF_UNSPEC query.  I have omitted this since it should be easy to
> > implement.
> > 
> > I have tested this on x86_64.
> > 
> > The interceptor library source:
> > 
> > /* Override recvfrom and modify the header of the first DNS response to make it
> >    a referral and reproduce bz #845218.  We have to resort to this ugly hack
> >    because we cannot make bind return the buggy response of a referral for the
> >    AAAA record and an authoritative response for the A record.  */
> >  #define _GNU_SOURCE
> >  #include <sys/types.h>
> >  #include <sys/socket.h>
> >  #include <netinet/in.h>
> >  #include <arpa/inet.h>
> >  #include <stdio.h>
> >  #include <stdbool.h>
> >  #include <endian.h>
> >  #include <dlfcn.h>
> >  #include <stdlib.h>
> > 
> > /* Lifted from resolv/arpa/nameser_compat.h.  */
> > typedef struct {
> >     unsigned        id :16;         /*%< query identification number */
> >  #if BYTE_ORDER == BIG_ENDIAN
> >     /* fields in third byte */
> >     unsigned        qr: 1;          /*%< response flag */
> >     unsigned        opcode: 4;      /*%< purpose of message */
> >     unsigned        aa: 1;          /*%< authoritive answer */
> >     unsigned        tc: 1;          /*%< truncated message */
> >     unsigned        rd: 1;          /*%< recursion desired */
> >     /* fields
> >      * in
> >      * fourth
> >      * byte
> >      * */
> >     unsigned        ra: 1;          /*%< recursion available */
> >     unsigned        unused :1;      /*%< unused bits (MBZ as of 4.9.3a3) */
> >     unsigned        ad: 1;          /*%< authentic data from named */
> >     unsigned        cd: 1;          /*%< checking disabled by resolver */
> >     unsigned        rcode :4;       /*%< response code */
> >  #endif
> >  #if BYTE_ORDER == LITTLE_ENDIAN || BYTE_ORDER == PDP_ENDIAN
> >     /* fields
> >      * in
> >      * third
> >      * byte
> >      * */
> >     unsigned        rd :1;          /*%< recursion desired */
> >     unsigned        tc :1;          /*%< truncated message */
> >     unsigned        aa :1;          /*%< authoritive answer */
> >     unsigned        opcode :4;      /*%< purpose of message */
> >     unsigned        qr :1;          /*%< response flag */
> >     /* fields
> >      * in
> >      * fourth
> >      * byte
> >      * */
> >     unsigned        rcode :4;       /*%< response code */
> >     unsigned        cd: 1;          /*%< checking disabled by resolver */
> >     unsigned        ad: 1;          /*%< authentic data from named */
> >     unsigned        unused :1;      /*%< unused bits (MBZ as of 4.9.3a3) */
> >     unsigned        ra :1;          /*%< recursion available */
> >  #endif
> >     /* remaining
> >      * bytes
> >      * */
> >     unsigned        qdcount :16;    /*%< number of question entries */
> >     unsigned        ancount :16;    /*%< number of answer entries */
> >     unsigned        nscount :16;    /*%< number of authority entries */
> >     unsigned        arcount :16;    /*%< number of resource entries */
> > } HEADER;
> > 
> > static int done = 0;
> > 
> > /* Packets to modify.  0 for the odd packets and 1 for even packets.  */
> > static const int mod_packet = 0;
> > 
> > /* Set to true if the first request should result in an error, resulting in a
> >    search query.  */
> > static bool first_error = true;
> > 
> > static ssize_t (*real_recvfrom) (int sockfd, void *buf, size_t len, int flags,
> > 			  struct sockaddr *src_addr, socklen_t *addrlen);
> > 
> > void
> > __attribute__ ((constructor))
> > init (void)
> > {
> >   real_recvfrom = dlsym (RTLD_NEXT, "recvfrom");
> > 
> >   if (real_recvfrom == NULL)
> >     {
> >       printf ("Failed to get reference to recvfrom: %s\n", dlerror ());
> >       printf ("Cannot simulate test\n");
> >       abort ();
> >     }
> > }
> > 
> > /* Modify the second packet that we receive to set the header in a manner as to
> >    reproduce BZ #845218.  */
> > static void
> > mod_buf (HEADER *h, int port)
> > {
> >   if (done % 2 == mod_packet || (first_error && done == 1))
> >     {
> >       printf ("(Modifying header)");
> > 
> >       if (first_error && done == 1)
> > 	h->rcode = 3;
> >       else
> > 	h->rcode = 0;	/* NOERROR == 0.  */
> >       h->ancount = 0;
> >       h->aa = 0;
> >       h->ra = 0;
> >       h->arcount = 0;
> >     }
> >   done++;
> > }
> > 
> > ssize_t
> > recvfrom (int sockfd, void *buf, size_t len, int flags,
> > 	  struct sockaddr *src_addr, socklen_t *addrlen)
> > {
> >   ssize_t ret = real_recvfrom (sockfd, buf, len, flags, src_addr, addrlen);
> >   int port = htons (((struct sockaddr_in *) src_addr)->sin_port);
> >   struct in_addr addr = ((struct sockaddr_in *) src_addr)->sin_addr;
> >   const char *host = inet_ntoa (addr);
> >   printf ("\n*** From %s:%d: ", host, port);
> > 
> >   mod_buf (buf, port);
> > 
> >   printf ("returned %zd\n", ret);
> >   return ret;
> > }
> > 
> > 
> > 	[BZ #14308]
> > 	* resolv/res_query.c (__libc_res_nsearch): Return if at least
> > 	one response is valid.
> > 	* resolv/res_send.c (send_dg): Check for validity of other
> > 	response if the current response is a referral.
> > 
> > ---
> >  resolv/res_query.c | 7 +++++--
> >  resolv/res_send.c  | 2 +-
> >  2 files changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/resolv/res_query.c b/resolv/res_query.c
> > index a9db837..4e6612c 100644
> > --- a/resolv/res_query.c
> > +++ b/resolv/res_query.c
> > @@ -382,7 +382,9 @@ __libc_res_nsearch(res_state statp,
> >  					      answer, anslen, answerp,
> >  					      answerp2, nanswerp2, resplen2,
> >  					      answerp2_malloced);
> > -		if (ret > 0 || trailing_dot)
> > +		if (ret > 0 || trailing_dot
> > +		    /* If the second response is valid then we use that.  */
> > +		    || (ret == 0 && answerp2 != NULL && resplen2 > 0))
> >  			return (ret);
> >  		saved_herrno = h_errno;
> >  		tried_as_is++;
> > @@ -422,7 +424,8 @@ __libc_res_nsearch(res_state statp,
> >  						      answer, anslen, answerp,
> >  						      answerp2, nanswerp2,
> >  						      resplen2, answerp2_malloced);
> > -			if (ret > 0)
> > +			if (ret > 0 || (ret == 0 && answerp2 != NULL
> > +					&& resplen2 > 0))
> >  				return (ret);
> >  
> >  			if (answerp && *answerp != answer) {
> > diff --git a/resolv/res_send.c b/resolv/res_send.c
> > index 60743df..3273d55 100644
> > --- a/resolv/res_send.c
> > +++ b/resolv/res_send.c
> > @@ -1351,6 +1351,7 @@ send_dg(res_state statp,
> >  				(*thisresplenp > *thisanssizp)
> >  				? *thisanssizp : *thisresplenp);
> >  
> > +		next_ns:
> >  			if (recvresp1 || (buf2 != NULL && recvresp2)) {
> >  			  *resplen2 = 0;
> >  			  return resplen;
> > @@ -1368,7 +1369,6 @@ send_dg(res_state statp,
> >  			    goto wait;
> >  			  }
> >  
> > -		next_ns:
> >  			__res_iclose(statp, false);
> >  			/* don't retry if called from dig */
> >  			if (!statp->pfcode)
> > -- 
> > 1.8.3.1
> > 
> 
>
  
Carlos O'Donell April 29, 2014, 9:35 a.m. UTC | #7
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Siddhesh,

Review below. Summary: Looks good to me and makes sense. I like the
clever trick of moving "next_ns" up to check the second response :-)

On 04/15/2014 12:29 PM, Siddhesh Poyarekar wrote:
> AF_UNSPEC results in sending two queries in parallel, one for the A
> record and the other for the AAAA record.  If one of these is a
> referral, then the query fails, which is wrong.  It should return at
> least the one successful response.

That's correct. From first principles a stub resolver can't handle the
referral response as it's not a recursive resolver.

> The fix has two parts.  The first part makes the referral fall back to
> the SERVFAIL path, which results in using the successful response.
> There is a bug in that path however, due to which the second part is
> necessary.  The bug here is that if the first response is a failure
> and the second succeeds, __libc_res_nsearch does not detect that and
> assumes a failure.  The case where the first response is a success and
> the second fails, works correctly.

OK.
 
> This condition is produced by buggy routers, so here's a crude
> interposable library that can simulate such a condition.  The library
> overrides the recvfrom syscall and modifies the header of the packet
> received to reproduce this scenario.  It has two key variables:
> mod_packet and first_error.

Thanks, that's very useful.

> The mod_packet variable when set to 0, results in odd packets being
> modified to be a referral.  When set to 1, even packets are modified
> to be a referral.
> 
> The first_error causes the first response to be a failure so that a
> domain-appended search is performed to test the second part of the
> __libc_nsearch fix.

OK.

> The driver for this fix is a simple getaddrinfo program that does an
> AF_UNSPEC query.  I have omitted this since it should be easy to
> implement.
> 
> I have tested this on x86_64.
> 
> The interceptor library source:
> 
> /* Override recvfrom and modify the header of the first DNS response to make it
>    a referral and reproduce bz #845218.  We have to resort to this ugly hack
>    because we cannot make bind return the buggy response of a referral for the
>    AAAA record and an authoritative response for the A record.  */
>  #define _GNU_SOURCE
>  #include <sys/types.h>
>  #include <sys/socket.h>
>  #include <netinet/in.h>
>  #include <arpa/inet.h>
>  #include <stdio.h>
>  #include <stdbool.h>
>  #include <endian.h>
>  #include <dlfcn.h>
>  #include <stdlib.h>
> 
> /* Lifted from resolv/arpa/nameser_compat.h.  */
> typedef struct {
>     unsigned        id :16;         /*%< query identification number */
>  #if BYTE_ORDER == BIG_ENDIAN
>     /* fields in third byte */
>     unsigned        qr: 1;          /*%< response flag */
>     unsigned        opcode: 4;      /*%< purpose of message */
>     unsigned        aa: 1;          /*%< authoritive answer */
>     unsigned        tc: 1;          /*%< truncated message */
>     unsigned        rd: 1;          /*%< recursion desired */
>     /* fields
>      * in
>      * fourth
>      * byte
>      * */
>     unsigned        ra: 1;          /*%< recursion available */
>     unsigned        unused :1;      /*%< unused bits (MBZ as of 4.9.3a3) */
>     unsigned        ad: 1;          /*%< authentic data from named */
>     unsigned        cd: 1;          /*%< checking disabled by resolver */
>     unsigned        rcode :4;       /*%< response code */
>  #endif
>  #if BYTE_ORDER == LITTLE_ENDIAN || BYTE_ORDER == PDP_ENDIAN
>     /* fields
>      * in
>      * third
>      * byte
>      * */
>     unsigned        rd :1;          /*%< recursion desired */
>     unsigned        tc :1;          /*%< truncated message */
>     unsigned        aa :1;          /*%< authoritive answer */
>     unsigned        opcode :4;      /*%< purpose of message */
>     unsigned        qr :1;          /*%< response flag */
>     /* fields
>      * in
>      * fourth
>      * byte
>      * */
>     unsigned        rcode :4;       /*%< response code */
>     unsigned        cd: 1;          /*%< checking disabled by resolver */
>     unsigned        ad: 1;          /*%< authentic data from named */
>     unsigned        unused :1;      /*%< unused bits (MBZ as of 4.9.3a3) */
>     unsigned        ra :1;          /*%< recursion available */
>  #endif
>     /* remaining
>      * bytes
>      * */
>     unsigned        qdcount :16;    /*%< number of question entries */
>     unsigned        ancount :16;    /*%< number of answer entries */
>     unsigned        nscount :16;    /*%< number of authority entries */
>     unsigned        arcount :16;    /*%< number of resource entries */
> } HEADER;
> 
> static int done = 0;
> 
> /* Packets to modify.  0 for the odd packets and 1 for even packets.  */
> static const int mod_packet = 0;
> 
> /* Set to true if the first request should result in an error, resulting in a
>    search query.  */
> static bool first_error = true;
> 
> static ssize_t (*real_recvfrom) (int sockfd, void *buf, size_t len, int flags,
> 			  struct sockaddr *src_addr, socklen_t *addrlen);
> 
> void
> __attribute__ ((constructor))
> init (void)
> {
>   real_recvfrom = dlsym (RTLD_NEXT, "recvfrom");
> 
>   if (real_recvfrom == NULL)
>     {
>       printf ("Failed to get reference to recvfrom: %s\n", dlerror ());
>       printf ("Cannot simulate test\n");
>       abort ();
>     }
> }
> 
> /* Modify the second packet that we receive to set the header in a manner as to
>    reproduce BZ #845218.  */
> static void
> mod_buf (HEADER *h, int port)
> {
>   if (done % 2 == mod_packet || (first_error && done == 1))
>     {
>       printf ("(Modifying header)");
> 
>       if (first_error && done == 1)
> 	h->rcode = 3;
>       else
> 	h->rcode = 0;	/* NOERROR == 0.  */
>       h->ancount = 0;
>       h->aa = 0;
>       h->ra = 0;
>       h->arcount = 0;
>     }
>   done++;
> }
> 
> ssize_t
> recvfrom (int sockfd, void *buf, size_t len, int flags,
> 	  struct sockaddr *src_addr, socklen_t *addrlen)
> {
>   ssize_t ret = real_recvfrom (sockfd, buf, len, flags, src_addr, addrlen);
>   int port = htons (((struct sockaddr_in *) src_addr)->sin_port);
>   struct in_addr addr = ((struct sockaddr_in *) src_addr)->sin_addr;
>   const char *host = inet_ntoa (addr);
>   printf ("\n*** From %s:%d: ", host, port);
> 
>   mod_buf (buf, port);
> 
>   printf ("returned %zd\n", ret);
>   return ret;
> }
> 
> 
> 	[BZ #14308]
> 	* resolv/res_query.c (__libc_res_nsearch): Return if at least
> 	one response is valid.
> 	* resolv/res_send.c (send_dg): Check for validity of other
> 	response if the current response is a referral.
> 
> ---
>  resolv/res_query.c | 7 +++++--
>  resolv/res_send.c  | 2 +-
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/resolv/res_query.c b/resolv/res_query.c
> index a9db837..4e6612c 100644
> --- a/resolv/res_query.c
> +++ b/resolv/res_query.c
> @@ -382,7 +382,9 @@ __libc_res_nsearch(res_state statp,
>  					      answer, anslen, answerp,
>  					      answerp2, nanswerp2, resplen2,
>  					      answerp2_malloced);
> -		if (ret > 0 || trailing_dot)
> +		if (ret > 0 || trailing_dot
> +		    /* If the second response is valid then we use that.  */
> +		    || (ret == 0 && answerp2 != NULL && resplen2 > 0))

OK. Verified __libc_res_nquerydomain results in a send_dg or equivalent
such that we know `answerp2 != NULL && resplen2 > 0` is going to indicate
that we have a second T_AAAA type response back and should check it also.

>  			return (ret);
>  		saved_herrno = h_errno;
>  		tried_as_is++;
> @@ -422,7 +424,8 @@ __libc_res_nsearch(res_state statp,
>  						      answer, anslen, answerp,
>  						      answerp2, nanswerp2,
>  						      resplen2, answerp2_malloced);
> -			if (ret > 0)
> +			if (ret > 0 || (ret == 0 && answerp2 != NULL
> +					&& resplen2 > 0))

OK.

>  				return (ret);
>  
>  			if (answerp && *answerp != answer) {
> diff --git a/resolv/res_send.c b/resolv/res_send.c
> index 60743df..3273d55 100644
> --- a/resolv/res_send.c
> +++ b/resolv/res_send.c
> @@ -1351,6 +1351,7 @@ send_dg(res_state statp,
>  				(*thisresplenp > *thisanssizp)
>  				? *thisanssizp : *thisresplenp);
>  
> +		next_ns:

OK. Verified other paths should be OK with next_ns testing for the
second response on SERVFAIL, it increases the latency a little but
this is a buggy server and we're trying to salvage the response.

>  			if (recvresp1 || (buf2 != NULL && recvresp2)) {
>  			  *resplen2 = 0;
>  			  return resplen;
> @@ -1368,7 +1369,6 @@ send_dg(res_state statp,
>  			    goto wait;
>  			  }
>  
> -		next_ns:
>  			__res_iclose(statp, false);
>  			/* don't retry if called from dig */
>  			if (!statp->pfcode)
> 

Looks good to me.

Cheers,
Carlos.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTX3JGAAoJECXvCkNsKkr/xIYH/0XcR+TH4nT2eTiJCn4TXm0K
i75dnby1WIEPV6QlR8mmOv+LqZY50mPG4s1Eaz9z0ofY4VI0TV2Wd4dwz1NeJC7/
xQit94xTxL5eiGI8KQhR8zWLAjKsLSIbFr/C4Y/CZNB0voo0Kz1bBPo9Z/T1spsC
plJ70tkzQmaWqH4YnREO2gnt72wB6q83OjmdHVHuymFGtE7A8ltjE0FGCU1au5Vc
8LYzXvTkhbMY01iDlsO7o+4WtMmNbzmk6bcfLsagYtcdZgegy0iGoa5s+KBp3Cmk
hYAlN2YHzioBS1JKbbDfQKQbfzJgZ1a4srYYaiFysWpyI4Uj5kHWdJW46tCRytQ=
=TziA
-----END PGP SIGNATURE-----
  
Andreas Schwab July 8, 2014, 9:43 a.m. UTC | #8
Siddhesh Poyarekar <siddhesh@redhat.com> writes:

> diff --git a/resolv/res_query.c b/resolv/res_query.c
> index a9db837..4e6612c 100644
> --- a/resolv/res_query.c
> +++ b/resolv/res_query.c
> @@ -382,7 +382,9 @@ __libc_res_nsearch(res_state statp,
>  					      answer, anslen, answerp,
>  					      answerp2, nanswerp2, resplen2,
>  					      answerp2_malloced);
> -		if (ret > 0 || trailing_dot)
> +		if (ret > 0 || trailing_dot
> +		    /* If the second response is valid then we use that.  */
> +		    || (ret == 0 && answerp2 != NULL && resplen2 > 0))

That doesn't make sense, resplen2 is a pointer.  I'm surprised that the
compiler does not warn about that.

I think that answerp2 != NULL should be resplen2 != NULL.

> @@ -422,7 +424,8 @@ __libc_res_nsearch(res_state statp,
>  						      answer, anslen, answerp,
>  						      answerp2, nanswerp2,
>  						      resplen2, answerp2_malloced);
> -			if (ret > 0)
> +			if (ret > 0 || (ret == 0 && answerp2 != NULL
> +					&& resplen2 > 0))

Likewise.

Andreas.
  
Andreas Schwab July 8, 2014, 9:51 a.m. UTC | #9
There is also a third place in res_query.c that should probably get the
same change:

	/*
	 * If the query has not already been tried as is then try it
	 * unless RES_NOTLDQUERY is set and there were no dots.
	 */
	if ((dots || !searched || (statp->options & RES_NOTLDQUERY) == 0)
	    && !(tried_as_is || root_on_list)) {
		ret = __libc_res_nquerydomain(statp, name, NULL, class, type,
					      answer, anslen, answerp,
					      answerp2, nanswerp2, resplen2,
					      answerp2_malloced);
		if (ret > 0)
			return (ret);
	}


Andreas.
  

Patch

diff --git a/resolv/res_query.c b/resolv/res_query.c
index a9db837..4e6612c 100644
--- a/resolv/res_query.c
+++ b/resolv/res_query.c
@@ -382,7 +382,9 @@  __libc_res_nsearch(res_state statp,
 					      answer, anslen, answerp,
 					      answerp2, nanswerp2, resplen2,
 					      answerp2_malloced);
-		if (ret > 0 || trailing_dot)
+		if (ret > 0 || trailing_dot
+		    /* If the second response is valid then we use that.  */
+		    || (ret == 0 && answerp2 != NULL && resplen2 > 0))
 			return (ret);
 		saved_herrno = h_errno;
 		tried_as_is++;
@@ -422,7 +424,8 @@  __libc_res_nsearch(res_state statp,
 						      answer, anslen, answerp,
 						      answerp2, nanswerp2,
 						      resplen2, answerp2_malloced);
-			if (ret > 0)
+			if (ret > 0 || (ret == 0 && answerp2 != NULL
+					&& resplen2 > 0))
 				return (ret);
 
 			if (answerp && *answerp != answer) {
diff --git a/resolv/res_send.c b/resolv/res_send.c
index 60743df..3273d55 100644
--- a/resolv/res_send.c
+++ b/resolv/res_send.c
@@ -1351,6 +1351,7 @@  send_dg(res_state statp,
 				(*thisresplenp > *thisanssizp)
 				? *thisanssizp : *thisresplenp);
 
+		next_ns:
 			if (recvresp1 || (buf2 != NULL && recvresp2)) {
 			  *resplen2 = 0;
 			  return resplen;
@@ -1368,7 +1369,6 @@  send_dg(res_state statp,
 			    goto wait;
 			  }
 
-		next_ns:
 			__res_iclose(statp, false);
 			/* don't retry if called from dig */
 			if (!statp->pfcode)