Author: asomers
Date: Thu Mar 17 20:00:49 2016
New Revision: 296994
URL: https://svnweb.freebsd.org/changeset/base/296994
Log:
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
Added:
stable/10/usr.sbin/rpcbind/tests/
- copied from r293229, head/usr.sbin/rpcbind/tests/
Modified:
stable/10/etc/mtree/BSD.tests.dist
stable/10/usr.sbin/rpcbind/Makefile
stable/10/usr.sbin/rpcbind/check_bound.c
stable/10/usr.sbin/rpcbind/rpcbind.h
stable/10/usr.sbin/rpcbind/tests/Makefile
stable/10/usr.sbin/rpcbind/tests/addrmerge_test.c
stable/10/usr.sbin/rpcbind/util.c
Directory Properties:
stable/10/ (props changed)
Modified: stable/10/etc/mtree/BSD.tests.dist
==============================================================================
--- stable/10/etc/mtree/BSD.tests.dist Thu Mar 17 19:35:08 2016
(r296993)
+++ stable/10/etc/mtree/BSD.tests.dist Thu Mar 17 20:00:49 2016
(r296994)
@@ -464,6 +464,8 @@
..
pw
..
+ rpcbind
+ ..
sa
..
..
Modified: stable/10/usr.sbin/rpcbind/Makefile
==============================================================================
--- stable/10/usr.sbin/rpcbind/Makefile Thu Mar 17 19:35:08 2016
(r296993)
+++ stable/10/usr.sbin/rpcbind/Makefile Thu Mar 17 20:00:49 2016
(r296994)
@@ -14,6 +14,10 @@ CFLAGS+= -DPORTMAP -DLIBWRAP
CFLAGS+= -DINET6
.endif
+.if ${MK_TESTS} != "no"
+SUBDIR+= tests
+.endif
+
WARNS?= 1
DPADD= ${LIBWRAP} ${LIBUTIL}
Modified: stable/10/usr.sbin/rpcbind/check_bound.c
==============================================================================
--- stable/10/usr.sbin/rpcbind/check_bound.c Thu Mar 17 19:35:08 2016
(r296993)
+++ stable/10/usr.sbin/rpcbind/check_bound.c Thu Mar 17 20:00:49 2016
(r296994)
@@ -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/10/usr.sbin/rpcbind/rpcbind.h
==============================================================================
--- stable/10/usr.sbin/rpcbind/rpcbind.h Thu Mar 17 19:35:08 2016
(r296993)
+++ stable/10/usr.sbin/rpcbind/rpcbind.h Thu Mar 17 20:00:49 2016
(r296994)
@@ -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/10/usr.sbin/rpcbind/tests/Makefile
==============================================================================
--- head/usr.sbin/rpcbind/tests/Makefile Wed Jan 6 00:00:11 2016
(r293229)
+++ stable/10/usr.sbin/rpcbind/tests/Makefile Thu Mar 17 20:00:49 2016
(r296994)
@@ -1,6 +1,6 @@
# $FreeBSD$
-.include <src.opts.mk>
+.include <bsd.own.mk>
.PATH: ${.CURDIR}/..
Modified: stable/10/usr.sbin/rpcbind/tests/addrmerge_test.c
==============================================================================
--- head/usr.sbin/rpcbind/tests/addrmerge_test.c Wed Jan 6 00:00:11
2016 (r293229)
+++ stable/10/usr.sbin/rpcbind/tests/addrmerge_test.c Thu Mar 17 20:00:49
2016 (r296994)
@@ -435,6 +435,7 @@ ATF_TC_BODY(addrmerge_localhost_only, tc
/* We must return localhost if there is nothing better */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("127.0.0.1.3.46", maddr);
+ free(maddr);
}
ATF_TC_WITHOUT_HEAD(addrmerge_singlehomed);
@@ -450,6 +451,7 @@ ATF_TC_BODY(addrmerge_singlehomed, tc)
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("192.0.2.2.3.46", maddr);
+ free(maddr);
}
ATF_TC_WITHOUT_HEAD(addrmerge_one_addr_on_each_subnet);
@@ -466,6 +468,7 @@ ATF_TC_BODY(addrmerge_one_addr_on_each_s
/* We must return the address on the caller's subnet */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("192.0.2.2.3.46", maddr);
+ free(maddr);
}
@@ -488,6 +491,7 @@ ATF_TC_BODY(addrmerge_one_addr_on_each_s
/* We must return the address on the caller's subnet */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("192.0.2.2.3.46", maddr);
+ free(maddr);
}
ATF_TC_WITHOUT_HEAD(addrmerge_point2point);
@@ -505,6 +509,7 @@ ATF_TC_BODY(addrmerge_point2point, tc)
/* addrmerge should disprefer P2P interfaces */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("192.0.2.130.3.46", maddr);
+ free(maddr);
}
/* Like addrerge_point2point, but getifaddrs returns a different order */
@@ -523,6 +528,7 @@ ATF_TC_BODY(addrmerge_point2point_rev, t
/* addrmerge should disprefer P2P interfaces */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("192.0.2.130.3.46", maddr);
+ free(maddr);
}
/*
@@ -544,6 +550,7 @@ ATF_TC_BODY(addrmerge_bindip, tc)
/* We must return the address to which we are bound */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("192.0.2.3.3.46", maddr);
+ free(maddr);
}
/* Like addrmerge_bindip, but getifaddrs returns a different order */
@@ -562,6 +569,7 @@ ATF_TC_BODY(addrmerge_bindip_rev, tc)
/* We must return the address to which we are bound */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("192.0.2.3.3.46", maddr);
+ free(maddr);
}
/*
@@ -582,6 +590,7 @@ ATF_TC_BODY(addrmerge_recvdstaddr, tc)
/* We must return the address on which the request was received */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("192.0.2.2.3.46", maddr);
+ free(maddr);
}
ATF_TC_WITHOUT_HEAD(addrmerge_recvdstaddr_rev);
@@ -598,6 +607,7 @@ ATF_TC_BODY(addrmerge_recvdstaddr_rev, t
/* We must return the address on which the request was received */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("192.0.2.2.3.46", maddr);
+ free(maddr);
}
#ifdef INET6
@@ -614,6 +624,7 @@ ATF_TC_BODY(addrmerge_localhost_only6, t
/* We must return localhost if there is nothing better */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("::1.3.46", maddr);
+ free(maddr);
}
ATF_TC_WITHOUT_HEAD(addrmerge_singlehomed6);
@@ -629,6 +640,7 @@ ATF_TC_BODY(addrmerge_singlehomed6, tc)
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("2001:db8::2.3.46", maddr);
+ free(maddr);
}
ATF_TC_WITHOUT_HEAD(addrmerge_one_addr_on_each_subnet6);
@@ -645,6 +657,7 @@ ATF_TC_BODY(addrmerge_one_addr_on_each_s
/* We must return the address on the caller's subnet */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("2001:db8::2.3.46", maddr);
+ free(maddr);
}
@@ -667,6 +680,7 @@ ATF_TC_BODY(addrmerge_one_addr_on_each_s
/* We must return the address on the caller's subnet */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("2001:db8::2.3.46", maddr);
+ free(maddr);
}
ATF_TC_WITHOUT_HEAD(addrmerge_point2point6);
@@ -684,6 +698,7 @@ ATF_TC_BODY(addrmerge_point2point6, tc)
/* addrmerge should disprefer P2P interfaces */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("2001:db8:1::2.3.46", maddr);
+ free(maddr);
}
/* Like addrerge_point2point, but getifaddrs returns a different order */
@@ -702,6 +717,7 @@ ATF_TC_BODY(addrmerge_point2point6_rev,
/* addrmerge should disprefer P2P interfaces */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("2001:db8:1::2.3.46", maddr);
+ free(maddr);
}
ATF_TC_WITHOUT_HEAD(addrmerge_bindip6);
@@ -719,6 +735,7 @@ ATF_TC_BODY(addrmerge_bindip6, tc)
/* We must return the address to which we are bound */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("2001:db8::3.3.46", maddr);
+ free(maddr);
}
/* Like addrerge_bindip, but getifaddrs returns a different order */
@@ -737,6 +754,7 @@ ATF_TC_BODY(addrmerge_bindip6_rev, tc)
/* We must return the address to which we are bound */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("2001:db8::3.3.46", maddr);
+ free(maddr);
}
/*
@@ -761,6 +779,7 @@ ATF_TC_BODY(addrmerge_ipv6_linklocal, tc
/* We must return the address to which we are bound */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("fe80::2.3.46", maddr);
+ free(maddr);
}
ATF_TC_WITHOUT_HEAD(addrmerge_ipv6_linklocal_rev);
@@ -781,6 +800,7 @@ ATF_TC_BODY(addrmerge_ipv6_linklocal_rev
/* We must return the address to which we are bound */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("fe80::2.3.46", maddr);
+ free(maddr);
}
ATF_TC_WITHOUT_HEAD(addrmerge_recvdstaddr6);
@@ -797,6 +817,7 @@ ATF_TC_BODY(addrmerge_recvdstaddr6, tc)
/* We must return the address on which the request was received */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("2001:db8::2.3.46", maddr);
+ free(maddr);
}
ATF_TC_WITHOUT_HEAD(addrmerge_recvdstaddr6_rev);
@@ -813,6 +834,7 @@ ATF_TC_BODY(addrmerge_recvdstaddr6_rev,
/* We must return the address on which the request was received */
ATF_REQUIRE(maddr != NULL);
ATF_CHECK_STREQ("2001:db8::2.3.46", maddr);
+ free(maddr);
}
#endif /* INET6 */
Modified: stable/10/usr.sbin/rpcbind/util.c
==============================================================================
--- stable/10/usr.sbin/rpcbind/util.c Thu Mar 17 19:35:08 2016
(r296993)
+++ stable/10/usr.sbin/rpcbind/util.c Thu Mar 17 20:00:49 2016
(r296994)
@@ -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 *);
/*
* For all bits set in "mask", compare the corresponding bits in
@@ -64,10 +64,34 @@ static int bitmaskcmp(void *, void *, vo
* 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]))
@@ -86,16 +110,18 @@ bitmaskcmp(void *dst, void *src, void *m
* 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)
@@ -139,19 +165,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;
@@ -159,21 +195,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
@@ -184,28 +228,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)
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
