Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 8 Jun 2016 16:59:09 +0000 (UTC)
From:      Garrett Cooper <ngie@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org
Subject:   svn commit: r301642 - stable/9/usr.sbin/rpcbind
Message-ID:  <201606081659.u58Gx9In015961@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ngie
Date: Wed Jun  8 16:59:09 2016
New Revision: 301642
URL: https://svnweb.freebsd.org/changeset/base/301642

Log:
  MFstable/10 r296994:
  r296994 (by asomers):
  
  MFC r293229, r293833 to usr.sbin/rpcbind
  
  r293833 | asomers | 2016-01-13 10:33:50 -0700 (Wed, 13 Jan 2016) | 16 lines
  
  Fix Coverity warnings regarding r293229
  
  rpcbind/check_bound.c
          Fix CID1347798, a memory leak in mergeaddr.
  
  rpcbind/tests/addrmerge_test.c
          Fix CID1347800 through CID1347803, memory leaks in ATF tests.  They
          are harmless because each ATF test case runs in its own process, but
          they are trivial to fix.  Fix a few other leaks that Coverity didn't
          detect, too.
  
  r293229 | asomers | 2016-01-05 17:00:11 -0700 (Tue, 05 Jan 2016) | 36 lines
  
  "source routing" in rpcbind
  
  Fix a bug in rpcbind for multihomed hosts. If the server had interfaces on
  two separate subnets, and a client on the first subnet contacted rpcbind at
  the address on the second subnet, rpcbind would advertise addresses on the
  first subnet. This is a bug, because it should prefer to advertise the
  address where it was contacted. The requested service might be firewalled
  off from the address on the first subnet, for example.
  
  usr.sbin/rpcbind/check_bound.c
          If the address on which a request was received is known, pass that
          to addrmerge as the clnt_uaddr parameter. That is what addrmerge's
          comment indicates the parameter is supposed to mean. The previous
          behavior is that clnt_uaddr would contain the address from which the
          client sent the request.
  
  usr.sbin/rpcbind/util.c
          Modify addrmerge to prefer to use an IP that is equal to clnt_uaddr,
          if one is found. Refactor the relevant portion of the function for
          clarity, and to reduce the number of ifdefs.
  
  etc/mtree/BSD.tests.dist
  usr.sbin/rpcbind/tests/Makefile
  usr.sbin/rpcbind/tests/addrmerge_test.c
          Add unit tests for usr.sbin/rpcbind/util.c:addrmerge.
  
  usr.sbin/rpcbind/check_bound.c
  usr.sbin/rpcbind/rpcbind.h
  usr.sbin/rpcbind/util.c
          Constify some function arguments

Modified:
  stable/9/usr.sbin/rpcbind/check_bound.c
  stable/9/usr.sbin/rpcbind/rpcbind.h
  stable/9/usr.sbin/rpcbind/util.c
Directory Properties:
  stable/9/   (props changed)
  stable/9/usr.sbin/   (props changed)

Modified: stable/9/usr.sbin/rpcbind/check_bound.c
==============================================================================
--- stable/9/usr.sbin/rpcbind/check_bound.c	Wed Jun  8 16:26:44 2016	(r301641)
+++ stable/9/usr.sbin/rpcbind/check_bound.c	Wed Jun  8 16:59:09 2016	(r301642)
@@ -51,6 +51,7 @@ static	char sccsid[] = "@(#)check_bound.
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <rpc/rpc.h>
+#include <rpc/svc_dg.h>
 #include <stdio.h>
 #include <netconfig.h>
 #include <syslog.h>
@@ -160,6 +161,7 @@ char *
 mergeaddr(SVCXPRT *xprt, char *netid, char *uaddr, char *saddr)
 {
 	struct fdlist *fdl;
+	struct svc_dg_data *dg_data;
 	char *c_uaddr, *s_uaddr, *m_uaddr, *allocated_uaddr = NULL;
 
 	for (fdl = fdhead; fdl; fdl = fdl->next)
@@ -171,21 +173,31 @@ mergeaddr(SVCXPRT *xprt, char *netid, ch
 		/* that server died */
 		return (nullstring);
 	/*
+	 * Try to determine the local address on which the client contacted us,
+	 * so we can send a reply from the same address.  If it's unknown, then
+	 * try to determine which address the client used, and pick a nearby
+	 * local address.
+	 *
 	 * If saddr is not NULL, the remote client may have included the
 	 * address by which it contacted us.  Use that for the "client" uaddr,
 	 * otherwise use the info from the SVCXPRT.
 	 */
-	if (saddr != NULL) {
+	dg_data = (struct svc_dg_data*)xprt->xp_p2;
+	if (dg_data != NULL && dg_data->su_srcaddr.buf != NULL) {
+		c_uaddr = taddr2uaddr(fdl->nconf, &dg_data->su_srcaddr);
+		allocated_uaddr = c_uaddr;
+	}
+	else if (saddr != NULL) {
 		c_uaddr = saddr;
 	} else {
 		c_uaddr = taddr2uaddr(fdl->nconf, svc_getrpccaller(xprt));
-		if (c_uaddr == NULL) {
-			syslog(LOG_ERR, "taddr2uaddr failed for %s",
-				fdl->nconf->nc_netid);
-			return (NULL);
-		}
 		allocated_uaddr = c_uaddr;
 	}
+	if (c_uaddr == NULL) {
+		syslog(LOG_ERR, "taddr2uaddr failed for %s",
+			fdl->nconf->nc_netid);
+		return (NULL);
+	}
 
 #ifdef ND_DEBUG
 	if (debugging) {
@@ -218,7 +230,7 @@ mergeaddr(SVCXPRT *xprt, char *netid, ch
  * structure should not be freed.
  */
 struct netconfig *
-rpcbind_get_conf(char *netid)
+rpcbind_get_conf(const char *netid)
 {
 	struct fdlist *fdl;
 

Modified: stable/9/usr.sbin/rpcbind/rpcbind.h
==============================================================================
--- stable/9/usr.sbin/rpcbind/rpcbind.h	Wed Jun  8 16:26:44 2016	(r301641)
+++ stable/9/usr.sbin/rpcbind/rpcbind.h	Wed Jun  8 16:59:09 2016	(r301642)
@@ -83,7 +83,7 @@ extern char *tcp_uaddr;		/* Universal TC
 int add_bndlist(struct netconfig *, struct netbuf *);
 bool_t is_bound(char *, char *);
 char *mergeaddr(SVCXPRT *, char *, char *, char *);
-struct netconfig *rpcbind_get_conf(char *);
+struct netconfig *rpcbind_get_conf(const char *);
 
 void rpcbs_init(void); 
 void rpcbs_procinfo(rpcvers_t, rpcproc_t);
@@ -132,8 +132,8 @@ extern void pmap_service(struct svc_req 
 void write_warmstart(void);
 void read_warmstart(void);
 
-char *addrmerge(struct netbuf *caller, char *serv_uaddr, char *clnt_uaddr,
-		     char *netid);
+char *addrmerge(struct netbuf *caller, const char *serv_uaddr,
+		const char *clnt_uaddr, char const *netid);
 int listen_addr(const struct sockaddr *sa);
 void network_init(void);
 struct sockaddr *local_sa(int);

Modified: stable/9/usr.sbin/rpcbind/util.c
==============================================================================
--- stable/9/usr.sbin/rpcbind/util.c	Wed Jun  8 16:26:44 2016	(r301641)
+++ stable/9/usr.sbin/rpcbind/util.c	Wed Jun  8 16:59:09 2016	(r301642)
@@ -56,7 +56,7 @@ static struct sockaddr_in *local_in4;
 static struct sockaddr_in6 *local_in6;
 #endif
 
-static int bitmaskcmp(void *, void *, void *, int);
+static int bitmaskcmp(struct sockaddr *, struct sockaddr *, struct sockaddr *);
 #ifdef INET6
 static void in6_fillscopeid(struct sockaddr_in6 *);
 #endif
@@ -67,10 +67,34 @@ static void in6_fillscopeid(struct socka
  * match.
  */
 static int
-bitmaskcmp(void *dst, void *src, void *mask, int bytelen)
+bitmaskcmp(struct sockaddr *dst, struct sockaddr *src, struct sockaddr *mask)
 {
 	int i;
-	u_int8_t *p1 = dst, *p2 = src, *netmask = mask;
+	u_int8_t *p1, *p2, *netmask;
+	int bytelen;
+
+	if (dst->sa_family != src->sa_family ||
+	    dst->sa_family != mask->sa_family)
+		return (1);
+
+	switch (dst->sa_family) {
+	case AF_INET:
+		p1 = (uint8_t*) &SA2SINADDR(dst);
+		p2 = (uint8_t*) &SA2SINADDR(src);
+		netmask = (uint8_t*) &SA2SINADDR(mask);
+		bytelen = sizeof(struct in_addr);
+		break;
+#ifdef INET6
+	case AF_INET6:
+		p1 = (uint8_t*) &SA2SIN6ADDR(dst);
+		p2 = (uint8_t*) &SA2SIN6ADDR(src);
+		netmask = (uint8_t*) &SA2SIN6ADDR(mask);
+		bytelen = sizeof(struct in6_addr);
+		break;
+#endif
+	default:
+		return (1);
+	}
 
 	for (i = 0; i < bytelen; i++)
 		if ((p1[i] & netmask[i]) != (p2[i] & netmask[i]))
@@ -109,16 +133,18 @@ in6_fillscopeid(struct sockaddr_in6 *sin
  * string which should be freed by the caller. On error, returns NULL.
  */
 char *
-addrmerge(struct netbuf *caller, char *serv_uaddr, char *clnt_uaddr,
-	  char *netid)
+addrmerge(struct netbuf *caller, const char *serv_uaddr, const char *clnt_uaddr,
+	  const char *netid)
 {
 	struct ifaddrs *ifap, *ifp = NULL, *bestif;
 	struct netbuf *serv_nbp = NULL, *hint_nbp = NULL, tbuf;
 	struct sockaddr *caller_sa, *hint_sa, *ifsa, *ifmasksa, *serv_sa;
 	struct sockaddr_storage ss;
 	struct netconfig *nconf;
-	char *caller_uaddr = NULL, *hint_uaddr = NULL;
+	char *caller_uaddr = NULL;
+	const char *hint_uaddr = NULL;
 	char *ret = NULL;
+	int bestif_goodness;
 
 #ifdef ND_DEBUG
 	if (debugging)
@@ -162,19 +188,29 @@ addrmerge(struct netbuf *caller, char *s
 		goto freeit;
 
 	/*
-	 * Loop through all interfaces. For each interface, see if it
-	 * is either the loopback interface (which we always listen
-	 * on) or is one of the addresses the program bound to (the
-	 * wildcard by default, or a subset if -h is specified) and
-	 * the network portion of its address is equal to that of the
-	 * client.  If so, we have found the interface that we want to
-	 * use.
+	 * Loop through all interface addresses.  We are listening to an address
+	 * if any of the following are true:
+	 * a) It's a loopback address
+	 * b) It was specified with the -h command line option
+	 * c) There were no -h command line options.
+	 *
+	 * Among addresses on which we are listening, choose in order of
+	 * preference an address that is:
+	 *
+	 * a) Equal to the hint
+	 * b) A link local address with the same scope ID as the client's
+	 *    address, if the client's address is also link local
+	 * c) An address on the same subnet as the client's address
+	 * d) A non-localhost, non-p2p address
+	 * e) Any usable address
 	 */
 	bestif = NULL;
+	bestif_goodness = 0;
 	for (ifap = ifp; ifap != NULL; ifap = ifap->ifa_next) {
 		ifsa = ifap->ifa_addr;
 		ifmasksa = ifap->ifa_netmask;
 
+		/* Skip addresses where we don't listen */
 		if (ifsa == NULL || ifsa->sa_family != hint_sa->sa_family ||
 		    !(ifap->ifa_flags & IFF_UP))
 			continue;
@@ -182,21 +218,29 @@ addrmerge(struct netbuf *caller, char *s
 		if (!(ifap->ifa_flags & IFF_LOOPBACK) && !listen_addr(ifsa))
 			continue;
 
-		switch (hint_sa->sa_family) {
-		case AF_INET:
-			/*
-			 * If the hint address matches this interface
-			 * address/netmask, then we're done.
-			 */
-			if (!bitmaskcmp(&SA2SINADDR(ifsa),
-			    &SA2SINADDR(hint_sa), &SA2SINADDR(ifmasksa),
-			    sizeof(struct in_addr))) {
-				bestif = ifap;
-				goto found;
-			}
-			break;
+		if ((hint_sa->sa_family == AF_INET) &&
+		    ((((struct sockaddr_in*)hint_sa)->sin_addr.s_addr == 
+		      ((struct sockaddr_in*)ifsa)->sin_addr.s_addr))) {
+			const int goodness = 4;
+
+			bestif_goodness = goodness;
+			bestif = ifap;
+			goto found;
+		}
 #ifdef INET6
-		case AF_INET6:
+		if ((hint_sa->sa_family == AF_INET6) &&
+		    (0 == memcmp(&((struct sockaddr_in6*)hint_sa)->sin6_addr,
+				 &((struct sockaddr_in6*)ifsa)->sin6_addr,
+				 sizeof(struct in6_addr))) &&
+		    (((struct sockaddr_in6*)hint_sa)->sin6_scope_id ==
+		    (((struct sockaddr_in6*)ifsa)->sin6_scope_id))) {
+			const int goodness = 4;
+
+			bestif_goodness = goodness;
+			bestif = ifap;
+			goto found;
+		}
+		if (hint_sa->sa_family == AF_INET6) {
 			/*
 			 * For v6 link local addresses, if the caller is on
 			 * a link-local address then use the scope id to see
@@ -208,28 +252,33 @@ addrmerge(struct netbuf *caller, char *s
 			    IN6_IS_ADDR_LINKLOCAL(&SA2SIN6ADDR(hint_sa))) {
 				if (SA2SIN6(ifsa)->sin6_scope_id ==
 				    SA2SIN6(caller_sa)->sin6_scope_id) {
-					bestif = ifap;
-					goto found;
+					const int goodness = 3;
+
+					if (bestif_goodness < goodness) {
+						bestif = ifap;
+						bestif_goodness = goodness;
+					}
 				}
-			} else if (!bitmaskcmp(&SA2SIN6ADDR(ifsa),
-			    &SA2SIN6ADDR(hint_sa), &SA2SIN6ADDR(ifmasksa),
-			    sizeof(struct in6_addr))) {
+			}
+		}
+#endif /* INET6 */
+		if (0 == bitmaskcmp(hint_sa, ifsa, ifmasksa)) {
+			const int goodness = 2;
+
+			if (bestif_goodness < goodness) {
 				bestif = ifap;
-				goto found;
+				bestif_goodness = goodness;
 			}
-			break;
-#endif
-		default:
-			continue;
 		}
+		if (!(ifap->ifa_flags & (IFF_LOOPBACK | IFF_POINTOPOINT))) {
+			const int goodness = 1;
 
-		/*
-		 * Remember the first possibly useful interface, preferring
-		 * "normal" to point-to-point and loopback ones.
-		 */
-		if (bestif == NULL ||
-		    (!(ifap->ifa_flags & (IFF_LOOPBACK | IFF_POINTOPOINT)) &&
-		    (bestif->ifa_flags & (IFF_LOOPBACK | IFF_POINTOPOINT))))
+			if (bestif_goodness < goodness) {
+				bestif = ifap;
+				bestif_goodness = goodness;
+			}
+		}
+		if (bestif == NULL)
 			bestif = ifap;
 	}
 	if (bestif == NULL)



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201606081659.u58Gx9In015961>