This is the mail archive of the glibc-cvs@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

GNU C Library master sources branch release/2.22/master updated. glibc-2.22-50-g5a1a5f0


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, release/2.22/master has been updated
       via  5a1a5f0dd2744044801c91bf2588444c29cda533 (commit)
      from  de905d1487a6d1d1667ae1346e4f2629dce9485f (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
http://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=5a1a5f0dd2744044801c91bf2588444c29cda533

commit 5a1a5f0dd2744044801c91bf2588444c29cda533
Author: Florian Weimer <fweimer@redhat.com>
Date:   Fri Mar 25 11:49:51 2016 +0100

    resolv: Always set *resplen2 out parameter in send_dg [BZ #19791]
    
    Since commit 44d20bca52ace85850012b0ead37b360e3ecd96e (Implement
    second fallback mode for DNS requests), there is a code path which
    returns early, before *resplen2 is initialized.  This happens if the
    name server address is immediately recognized as invalid (because of
    lack of protocol support, or if it is a broadcast address such
    255.255.255.255, or another invalid address).
    
    If this happens and *resplen2 was non-zero (which is the case if a
    previous query resulted in a failure), __libc_res_nquery would reuse
    an existing second answer buffer.  This answer has been previously
    identified as unusable (for example, it could be an NXDOMAIN
    response).  Due to the presence of a second answer, no name server
    switching will occur.  The result is a name resolution failure,
    although a successful resolution would have been possible if name
    servers have been switched and queries had proceeded along the search
    path.
    
    The above paragraph still simplifies the situation.  Before glibc
    2.23, if the second answer needed malloc, the stub resolver would
    still attempt to reuse the second answer, but this is not possible
    because __libc_res_nsearch has freed it, after the unsuccessful call
    to __libc_res_nquerydomain, and set the buffer pointer to NULL.  This
    eventually leads to an assertion failure in __libc_res_nquery:
    
    	/* Make sure both hp and hp2 are defined */
    	assert((hp != NULL) && (hp2 != NULL));
    
    If assertions are disabled, the consequence is a NULL pointer
    dereference on the next line.
    
    Starting with glibc 2.23, as a result of commit
    e9db92d3acfe1822d56d11abcea5bfc4c41cf6ca (CVE-2015-7547: getaddrinfo()
    stack-based buffer overflow (Bug 18665)), the second answer is always
    allocated with malloc.  This means that the assertion failure happens
    with small responses as well because there is no buffer to reuse, as
    soon as there is a name resolution failure which triggers a search for
    an answer along the search path.
    
    This commit addresses the issue by ensuring that *resplen2 is
    initialized before the send_dg function returns.
    
    This commit also addresses a bug where an invalid second reply is
    incorrectly returned as a valid to the caller.
    
    (cherry picked from commit b66d837bb5398795c6b0f651bd5a5d66091d8577)

diff --git a/ChangeLog b/ChangeLog
index d97d7a5..3e0d69b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2016-03-25  Florian Weimer  <fweimer@redhat.com>
+
+	[BZ #19791]
+	* resolv/res_send.c (close_and_return_error): New function.
+	(send_dg): Initialize *resplen2 after reopen failure.  Call
+	close_and_return_error for error returns.  On error paths without
+	__res_iclose, initialze *resplen2 explicitly.  Update comment for
+	successful return.
+
 2016-03-21  Dylan Alex Simon  <dylan-sourceware@dylex.net>
 
 	[BZ #19822]
diff --git a/NEWS b/NEWS
index dbd647c..a3dd3c7 100644
--- a/NEWS
+++ b/NEWS
@@ -25,7 +25,7 @@ Version 2.22.1
 
   17905, 18420, 18421, 18480, 18589, 18743, 18778, 18781, 18787, 18796,
   18870, 18887, 18921, 18928, 18969, 18985, 19003, 19018, 19058, 19174,
-  19178, 19590, 19682, 19822.
+  19178, 19590, 19682, 19791, 19822.
 
 * The LD_POINTER_GUARD environment variable can no longer be used to
   disable the pointer guard feature.  It is always enabled.
diff --git a/resolv/res_send.c b/resolv/res_send.c
index 6511bb1..0add3d2 100644
--- a/resolv/res_send.c
+++ b/resolv/res_send.c
@@ -657,6 +657,18 @@ get_nsaddr (res_state statp, int n)
     return (struct sockaddr *) (void *) &statp->nsaddr_list[n];
 }
 
+/* Close the resolver structure, assign zero to *RESPLEN2 if RESPLEN2
+   is not NULL, and return zero.  */
+static int
+__attribute__ ((warn_unused_result))
+close_and_return_error (res_state statp, int *resplen2)
+{
+  __res_iclose(statp, false);
+  if (resplen2 != NULL)
+    *resplen2 = 0;
+  return 0;
+}
+
 /* The send_vc function is responsible for sending a DNS query over TCP
    to the nameserver numbered NS from the res_state STATP i.e.
    EXT(statp).nssocks[ns].  The function supports sending both IPv4 and
@@ -1159,7 +1171,11 @@ send_dg(res_state statp,
  retry_reopen:
 	retval = reopen (statp, terrno, ns);
 	if (retval <= 0)
-		return retval;
+	  {
+	    if (resplen2 != NULL)
+	      *resplen2 = 0;
+	    return retval;
+	  }
  retry:
 	evNowTime(&now);
 	evConsTime(&timeout, seconds, 0);
@@ -1172,8 +1188,6 @@ send_dg(res_state statp,
 	int recvresp2 = buf2 == NULL;
 	pfd[0].fd = EXT(statp).nssocks[ns];
 	pfd[0].events = POLLOUT;
-	if (resplen2 != NULL)
-	  *resplen2 = 0;
  wait:
 	if (need_recompute) {
 	recompute_resend:
@@ -1181,9 +1195,7 @@ send_dg(res_state statp,
 		if (evCmpTime(finish, now) <= 0) {
 		poll_err_out:
 			Perror(statp, stderr, "poll", errno);
-		err_out:
-			__res_iclose(statp, false);
-			return (0);
+			return close_and_return_error (statp, resplen2);
 		}
 		evSubTime(&timeout, &finish, &now);
 		need_recompute = 0;
@@ -1230,7 +1242,9 @@ send_dg(res_state statp,
 		  }
 
 		*gotsomewhere = 1;
-		return (0);
+		if (resplen2 != NULL)
+		  *resplen2 = 0;
+		return 0;
 	}
 	if (n < 0) {
 		if (errno == EINTR)
@@ -1298,7 +1312,7 @@ send_dg(res_state statp,
 
 		      fail_sendmmsg:
 			Perror(statp, stderr, "sendmmsg", errno);
-			goto err_out;
+			return close_and_return_error (statp, resplen2);
 		      }
 		  }
 		else
@@ -1316,7 +1330,7 @@ send_dg(res_state statp,
 		      if (errno == EINTR || errno == EAGAIN)
 			goto recompute_resend;
 		      Perror(statp, stderr, "send", errno);
-		      goto err_out;
+		      return close_and_return_error (statp, resplen2);
 		    }
 		  just_one:
 		    if (nwritten != 0 || buf2 == NULL || single_request)
@@ -1394,7 +1408,7 @@ send_dg(res_state statp,
 				goto wait;
 			}
 			Perror(statp, stderr, "recvfrom", errno);
-			goto err_out;
+			return close_and_return_error (statp, resplen2);
 		}
 		*gotsomewhere = 1;
 		if (__glibc_unlikely (*thisresplenp < HFIXEDSZ))       {
@@ -1405,7 +1419,7 @@ send_dg(res_state statp,
 			       (stdout, ";; undersized: %d\n",
 				*thisresplenp));
 			*terrno = EMSGSIZE;
-			goto err_out;
+			return close_and_return_error (statp, resplen2);
 		}
 		if ((recvresp1 || hp->id != anhp->id)
 		    && (recvresp2 || hp2->id != anhp->id)) {
@@ -1454,7 +1468,7 @@ send_dg(res_state statp,
 				? *thisanssizp : *thisresplenp);
 			/* record the error */
 			statp->_flags |= RES_F_EDNS0ERR;
-			goto err_out;
+			return close_and_return_error (statp, resplen2);
 	}
 #endif
 		if (!(statp->options & RES_INSECURE2)
@@ -1506,10 +1520,10 @@ send_dg(res_state statp,
 			    goto wait;
 			  }
 
-			__res_iclose(statp, false);
 			/* don't retry if called from dig */
 			if (!statp->pfcode)
-				return (0);
+			  return close_and_return_error (statp, resplen2);
+			__res_iclose(statp, false);
 		}
 		if (anhp->rcode == NOERROR && anhp->ancount == 0
 		    && anhp->aa == 0 && anhp->ra == 0 && anhp->arcount == 0) {
@@ -1531,6 +1545,8 @@ send_dg(res_state statp,
 			__res_iclose(statp, false);
 			// XXX if we have received one reply we could
 			// XXX use it and not repeat it over TCP...
+			if (resplen2 != NULL)
+			  *resplen2 = 0;
 			return (1);
 		}
 		/* Mark which reply we received.  */
@@ -1546,21 +1562,22 @@ send_dg(res_state statp,
 					__res_iclose (statp, false);
 					retval = reopen (statp, terrno, ns);
 					if (retval <= 0)
-						return retval;
+					  {
+					    if (resplen2 != NULL)
+					      *resplen2 = 0;
+					    return retval;
+					  }
 					pfd[0].fd = EXT(statp).nssocks[ns];
 				}
 			}
 			goto wait;
 		}
-		/*
-		 * All is well, or the error is fatal.  Signal that the
-		 * next nameserver ought not be tried.
-		 */
+		/* All is well.  We have received both responses (if
+		   two responses were requested).  */
 		return (resplen);
-	} else if (pfd[0].revents & (POLLERR | POLLHUP | POLLNVAL)) {
-		/* Something went wrong.  We can stop trying.  */
-		goto err_out;
-	}
+	} else if (pfd[0].revents & (POLLERR | POLLHUP | POLLNVAL))
+	  /* Something went wrong.  We can stop trying.  */
+	  return close_and_return_error (statp, resplen2);
 	else {
 		/* poll should not have returned > 0 in this case.  */
 		abort ();

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog         |    9 +++++++
 NEWS              |    2 +-
 resolv/res_send.c |   63 +++++++++++++++++++++++++++++++++-------------------
 3 files changed, 50 insertions(+), 24 deletions(-)


hooks/post-receive
-- 
GNU C Library master sources


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]