Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 27 Jun 2016 15:47:25 +0000 (UTC)
From:      "Bjoern A. Zeeb" <bz@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-projects@freebsd.org
Subject:   svn commit: r302223 - projects/vnet/sys/contrib/ipfilter/netinet
Message-ID:  <201606271547.u5RFlPUY087302@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: bz
Date: Mon Jun 27 15:47:25 2016
New Revision: 302223
URL: https://svnweb.freebsd.org/changeset/base/302223

Log:
  DIsable the "cloner" event handler;  we are getting interface
  events before the firewall is fully initiallized and also no vnet
  information thus leading to uninitialised memory accesses.
  In addition it is unclear why we need it in first place.
  If it turns out to be needed, well need a dedicated event handler
  for it.
  
  We make sure ipf_running is initialized statically to something
  that indicates really not running;  though the problem trying to
  address with that was solved differently.
  
  Very specially handle the dynamic sysctls added.  The problem is
  that "ipmain" is the virtualized struct, but the fields used for
  the sysctls are hanging off memory allocated and attached to the
  virtualized "ipmain" thus standard VNET macros and sysctl handling
  does not work.  We still say it is VNET sysctls to get the proper
  protection checks in the VIMAGE case;  to solve the problem of
  accessing the right bit of memory haning of each per-VNET ipmain,
  we use a dedicated handler function wrapping around sysctl_ipf_int()
  undoing the base calculation from kern_sysctl.c and then adding the
  passed-in offset into the right struct depending on handler.
  A bit of a mess exposing VNET-internals this way but the only way
  to keep the code without having to massively restructure ipf internals.
  
  Sponsored by:	The FreeBSD Foundation

Modified:
  projects/vnet/sys/contrib/ipfilter/netinet/ip_fil_freebsd.c
  projects/vnet/sys/contrib/ipfilter/netinet/mlfk_ipl.c

Modified: projects/vnet/sys/contrib/ipfilter/netinet/ip_fil_freebsd.c
==============================================================================
--- projects/vnet/sys/contrib/ipfilter/netinet/ip_fil_freebsd.c	Mon Jun 27 06:41:11 2016	(r302222)
+++ projects/vnet/sys/contrib/ipfilter/netinet/ip_fil_freebsd.c	Mon Jun 27 15:47:25 2016	(r302223)
@@ -105,7 +105,9 @@ MALLOC_DEFINE(M_IPFILTER, "ipfilter", "I
 static	int	ipf_send_ip __P((fr_info_t *, mb_t *));
 static void	ipf_timer_func __P((void *arg));
 
-VNET_DEFINE(ipf_main_softc_t, ipfmain);
+VNET_DEFINE(ipf_main_softc_t, ipfmain) = {
+	.ipf_running		= -2,
+};
 #define	V_ipfmain		VNET(ipfmain)
 
 # include <sys/conf.h>
@@ -113,7 +115,10 @@ VNET_DEFINE(ipf_main_softc_t, ipfmain);
 #  include <net/pfil.h>
 # endif /* NETBSD_PF */
 
-static eventhandler_tag ipf_arrivetag, ipf_departtag, ipf_clonetag;
+static eventhandler_tag ipf_arrivetag, ipf_departtag;
+#if 0
+static eventhandler_tag ipf_clonetag;
+#endif
 
 static void ipf_ifevent(void *arg, struct ifnet *ifp);
 
@@ -123,7 +128,8 @@ static void ipf_ifevent(arg, ifp)
 {
 
 	CURVNET_SET(ifp->if_vnet);
-        ipf_sync(&V_ipfmain, NULL);
+	if (V_ipfmain.ipf_running > 0)
+		ipf_sync(&V_ipfmain, NULL);
 	CURVNET_RESTORE();
 }
 
@@ -1411,8 +1417,10 @@ ipf_event_reg(void)
 	ipf_departtag = EVENTHANDLER_REGISTER(ifnet_departure_event, \
 					       ipf_ifevent, NULL, \
 					       EVENTHANDLER_PRI_ANY);
+#if 0
 	ipf_clonetag  = EVENTHANDLER_REGISTER(if_clone_event, ipf_ifevent, \
 					       NULL, EVENTHANDLER_PRI_ANY);
+#endif
 }
 
 void
@@ -1424,9 +1432,11 @@ ipf_event_dereg(void)
 	if (ipf_departtag != NULL) {
 		EVENTHANDLER_DEREGISTER(ifnet_departure_event, ipf_departtag);
 	}
+#if 0
 	if (ipf_clonetag != NULL) {
 		EVENTHANDLER_DEREGISTER(if_clone_event, ipf_clonetag);
 	}
+#endif
 }
 
 

Modified: projects/vnet/sys/contrib/ipfilter/netinet/mlfk_ipl.c
==============================================================================
--- projects/vnet/sys/contrib/ipfilter/netinet/mlfk_ipl.c	Mon Jun 27 06:41:11 2016	(r302222)
+++ projects/vnet/sys/contrib/ipfilter/netinet/mlfk_ipl.c	Mon Jun 27 15:47:25 2016	(r302223)
@@ -59,10 +59,14 @@ static dev_t ipf_devs[IPL_LOGSIZE];
 #endif
 
 static int sysctl_ipf_int ( SYSCTL_HANDLER_ARGS );
+static int sysctl_ipf_int_nat ( SYSCTL_HANDLER_ARGS );
+static int sysctl_ipf_int_state ( SYSCTL_HANDLER_ARGS );
+static int sysctl_ipf_int_auth ( SYSCTL_HANDLER_ARGS );
+static int sysctl_ipf_int_frag ( SYSCTL_HANDLER_ARGS );
 static int ipf_modload(void);
 static int ipf_modunload(void);
-static int ipf_fbsd_sysctl_create(ipf_main_softc_t*);
-static int ipf_fbsd_sysctl_destroy(ipf_main_softc_t*);
+static int ipf_fbsd_sysctl_create(void);
+static int ipf_fbsd_sysctl_destroy(void);
 
 #if (__FreeBSD_version >= 500024)
 # if (__FreeBSD_version >= 502116)
@@ -89,11 +93,19 @@ SYSCTL_DECL(_net_inet);
 #define SYSCTL_IPF(parent, nbr, name, access, ptr, val, descr) \
 	SYSCTL_OID(parent, nbr, name, CTLTYPE_INT|CTLFLAG_VNET|access, \
 		   ptr, val, sysctl_ipf_int, "I", descr)
-#define SYSCTL_DYN_IPF(parent, nbr, name, access,ptr, val, descr) \
-	SYSCTL_ADD_OID(&V_ipf_clist, SYSCTL_STATIC_CHILDREN(parent), nbr, name, \
-	CTLFLAG_DYN|CTLTYPE_INT|CTLFLAG_VNET|access, ptr, val, sysctl_ipf_int, "I", descr)
-static VNET_DEFINE(struct sysctl_ctx_list, ipf_clist);
-#define	V_ipf_clist		VNET(ipf_clist)
+#define SYSCTL_DYN_IPF_NAT(parent, nbr, name, access,ptr, val, descr) \
+	SYSCTL_ADD_OID(&ipf_clist, SYSCTL_STATIC_CHILDREN(parent), nbr, name, \
+	CTLFLAG_DYN|CTLTYPE_INT|CTLFLAG_VNET|access, ptr, val, sysctl_ipf_int_nat, "I", descr)
+#define SYSCTL_DYN_IPF_STATE(parent, nbr, name, access,ptr, val, descr) \
+	SYSCTL_ADD_OID(&ipf_clist, SYSCTL_STATIC_CHILDREN(parent), nbr, name, \
+	CTLFLAG_DYN|CTLTYPE_INT|CTLFLAG_VNET|access, ptr, val, sysctl_ipf_int_state, "I", descr)
+#define SYSCTL_DYN_IPF_FRAG(parent, nbr, name, access,ptr, val, descr) \
+	SYSCTL_ADD_OID(&ipf_clist, SYSCTL_STATIC_CHILDREN(parent), nbr, name, \
+	CTLFLAG_DYN|CTLTYPE_INT|CTLFLAG_VNET|access, ptr, val, sysctl_ipf_int_frag, "I", descr)
+#define SYSCTL_DYN_IPF_AUTH(parent, nbr, name, access,ptr, val, descr) \
+	SYSCTL_ADD_OID(&ipf_clist, SYSCTL_STATIC_CHILDREN(parent), nbr, name, \
+	CTLFLAG_DYN|CTLTYPE_INT|CTLFLAG_VNET|access, ptr, val, sysctl_ipf_int_auth, "I", descr)
+static struct sysctl_ctx_list ipf_clist;
 #define	CTLFLAG_OFF	0x00800000	/* IPFilter must be disabled */
 #define	CTLFLAG_RWO	(CTLFLAG_RW|CTLFLAG_OFF)
 SYSCTL_NODE(_net_inet, OID_AUTO, ipf, CTLFLAG_RW, 0, "IPF");
@@ -205,14 +217,8 @@ vnet_ipf_init(void)
 	if (ipf_create_all(&V_ipfmain) == NULL)
 		return;
 
-	if (ipf_fbsd_sysctl_create(&V_ipfmain) != 0) {
-		ipf_destroy_all(&V_ipfmain);
-		return;
-	}
-
 	error = ipfattach(&V_ipfmain);
 	if (error) {
-		(void)ipf_fbsd_sysctl_destroy(&V_ipfmain);
 		ipf_destroy_all(&V_ipfmain);
 		return;
 	}
@@ -251,6 +257,10 @@ ipf_modload()
 	if (ipf_load_all() != 0)
 		return EIO;
 
+	if (ipf_fbsd_sysctl_create() != 0) {
+		return EIO;
+	}
+
 	for (i = 0; i < IPL_LOGSIZE; i++)
 		ipf_devs[i] = NULL;
 	for (i = 0; (str = ipf_devfiles[i]); i++) {
@@ -280,14 +290,10 @@ vnet_ipf_uninit(void)
 	if (V_ipfmain.ipf_refcnt)
 		return;
 
-	if (ipf_fbsd_sysctl_destroy(&V_ipfmain) != 0)
-		return;
-
 	if (V_ipfmain.ipf_running >= 0) {
 		if (ipfdetach(&V_ipfmain) != 0)
 			return;
 
-		ipf_fbsd_sysctl_destroy(&V_ipfmain);
 		ipf_destroy_all(&V_ipfmain);
 	}
 
@@ -303,6 +309,8 @@ ipf_modunload()
 
 	ipf_event_dereg();
 
+	ipf_fbsd_sysctl_destroy();
+
 	error = ipf_pfil_unhook();
 	if (error != 0)
 		return error;
@@ -357,6 +365,92 @@ sysctl_ipf_int ( SYSCTL_HANDLER_ARGS )
 	}
 	return (error);
 }
+
+/*
+ * In the VIMAGE case kern_sysctl.c already adds the vnet base address given
+ * we set CTLFLAG_VNET to get proper access checks.  Have to undo this.
+ * Then we add the given offset to the specific malloced struct hanging off
+ * virtualized ipmain struct.
+ */
+static int
+sysctl_ipf_int_nat ( SYSCTL_HANDLER_ARGS )
+{
+
+	if (arg1) {
+		ipf_nat_softc_t *nat_softc;
+
+		nat_softc = V_ipfmain.ipf_nat_soft;
+#ifdef VIMAGE
+		arg1 = (void *)((uintptr_t)arg1 - curvnet->vnet_data_base);
+#endif
+		arg1 = (void *)((uintptr_t)nat_softc + (uintptr_t)arg1);
+	}
+
+	return (sysctl_ipf_int(oidp, arg1, arg2, req));
+}
+
+static int
+sysctl_ipf_int_state ( SYSCTL_HANDLER_ARGS )
+{
+
+	if (arg1) {
+		ipf_state_softc_t *state_softc;
+
+		state_softc = V_ipfmain.ipf_state_soft;
+#ifdef VIMAGE
+		arg1 = (void *)((uintptr_t)arg1 - curvnet->vnet_data_base);
+#endif
+		arg1 = (void *)((uintptr_t)state_softc + (uintptr_t)arg1);
+	}
+
+	return (sysctl_ipf_int(oidp, arg1, arg2, req));
+}
+
+static int
+sysctl_ipf_int_auth ( SYSCTL_HANDLER_ARGS )
+{
+
+	if (arg1) {
+		ipf_auth_softc_t *auth_softc;
+
+		auth_softc = V_ipfmain.ipf_auth_soft;
+#ifdef VIMAGE
+		arg1 = (void *)((uintptr_t)arg1 - curvnet->vnet_data_base);
+#endif
+		arg1 = (void *)((uintptr_t)auth_softc + (uintptr_t)arg1);
+	}
+
+	return (sysctl_ipf_int(oidp, arg1, arg2, req));
+}
+
+static int
+sysctl_ipf_int_frag ( SYSCTL_HANDLER_ARGS )
+{
+
+	if (arg1) {
+		ipf_frag_softc_t *frag_softc;
+
+		frag_softc = V_ipfmain.ipf_frag_soft;
+#ifdef VIMAGE
+		arg1 = (void *)((uintptr_t)arg1 - curvnet->vnet_data_base);
+#endif
+		arg1 = (void *)((uintptr_t)frag_softc + (uintptr_t)arg1);
+	}
+
+	return (sysctl_ipf_int(oidp, arg1, arg2, req));
+}
+
+
+#if 0
+-       ipf_state_softc_t *state_softc;
+-       ipf_auth_softc_t *auth_softc;
+-       ipf_frag_softc_t *frag_softc;
+ 
+-       state_softc = main_softc->ipf_state_soft;
+-       auth_softc = main_softc->ipf_auth_soft;
+-       frag_softc = main_softc->ipf_frag_soft;
+#endif
+
 #endif
 
 
@@ -571,53 +665,42 @@ static int ipfwrite(dev, uio)
 }
 
 static int
-ipf_fbsd_sysctl_create(main_softc)
-	ipf_main_softc_t *main_softc;
+ipf_fbsd_sysctl_create(void)
 {
-	ipf_nat_softc_t	*nat_softc;
-	ipf_state_softc_t *state_softc;
-	ipf_auth_softc_t *auth_softc;
-	ipf_frag_softc_t *frag_softc;
-
-	nat_softc = main_softc->ipf_nat_soft;
-	state_softc = main_softc->ipf_state_soft;
-	auth_softc = main_softc->ipf_auth_soft;
-	frag_softc = main_softc->ipf_frag_soft;
-
-	sysctl_ctx_init(&V_ipf_clist);
-
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "fr_defnatage", CTLFLAG_RWO,
-	    &nat_softc->ipf_nat_defage, 0, "");
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "fr_statesize", CTLFLAG_RWO,
-	    &state_softc->ipf_state_size, 0, "");
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "fr_statemax", CTLFLAG_RWO,
-	    &state_softc->ipf_state_max, 0, "");
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "ipf_nattable_max", CTLFLAG_RWO,
-	    &nat_softc->ipf_nat_table_max, 0, "");
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "ipf_nattable_sz", CTLFLAG_RWO,
-	    &nat_softc->ipf_nat_table_sz, 0, "");
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "ipf_natrules_sz", CTLFLAG_RWO,
-	    &nat_softc->ipf_nat_maprules_sz, 0, "");
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "ipf_rdrrules_sz", CTLFLAG_RWO,
-	    &nat_softc->ipf_nat_rdrrules_sz, 0, "");
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "ipf_hostmap_sz", CTLFLAG_RWO,
-	    &nat_softc->ipf_nat_hostmap_sz, 0, "");
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "fr_authsize", CTLFLAG_RWO,
-	   &auth_softc->ipf_auth_size, 0, "");
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "fr_authused", CTLFLAG_RD,
-	   &auth_softc->ipf_auth_used, 0, "");
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "fr_defaultauthage", CTLFLAG_RW,
-	   &auth_softc->ipf_auth_defaultage, 0, "");
-	SYSCTL_DYN_IPF(_net_inet_ipf, OID_AUTO, "fr_ipfrttl", CTLFLAG_RW,
-	   &frag_softc->ipfr_ttl, 0, "");
+
+	sysctl_ctx_init(&ipf_clist);
+
+	SYSCTL_DYN_IPF_NAT(_net_inet_ipf, OID_AUTO, "fr_defnatage", CTLFLAG_RWO,
+	    (void *)offsetof(ipf_nat_softc_t, ipf_nat_defage), 0, "");
+	SYSCTL_DYN_IPF_STATE(_net_inet_ipf, OID_AUTO, "fr_statesize", CTLFLAG_RWO,
+	    (void *)offsetof(ipf_state_softc_t, ipf_state_size), 0, "");
+	SYSCTL_DYN_IPF_STATE(_net_inet_ipf, OID_AUTO, "fr_statemax", CTLFLAG_RWO,
+	    (void *)offsetof(ipf_state_softc_t, ipf_state_max), 0, "");
+	SYSCTL_DYN_IPF_NAT(_net_inet_ipf, OID_AUTO, "ipf_nattable_max", CTLFLAG_RWO,
+	    (void *)offsetof(ipf_nat_softc_t, ipf_nat_table_max), 0, "");
+	SYSCTL_DYN_IPF_NAT(_net_inet_ipf, OID_AUTO, "ipf_nattable_sz", CTLFLAG_RWO,
+	    (void *)offsetof(ipf_nat_softc_t, ipf_nat_table_sz), 0, "");
+	SYSCTL_DYN_IPF_NAT(_net_inet_ipf, OID_AUTO, "ipf_natrules_sz", CTLFLAG_RWO,
+	    (void *)offsetof(ipf_nat_softc_t, ipf_nat_maprules_sz), 0, "");
+	SYSCTL_DYN_IPF_NAT(_net_inet_ipf, OID_AUTO, "ipf_rdrrules_sz", CTLFLAG_RWO,
+	    (void *)offsetof(ipf_nat_softc_t, ipf_nat_rdrrules_sz), 0, "");
+	SYSCTL_DYN_IPF_NAT(_net_inet_ipf, OID_AUTO, "ipf_hostmap_sz", CTLFLAG_RWO,
+	    (void *)offsetof(ipf_nat_softc_t, ipf_nat_hostmap_sz), 0, "");
+	SYSCTL_DYN_IPF_AUTH(_net_inet_ipf, OID_AUTO, "fr_authsize", CTLFLAG_RWO,
+	    (void *)offsetof(ipf_auth_softc_t, ipf_auth_size), 0, "");
+	SYSCTL_DYN_IPF_AUTH(_net_inet_ipf, OID_AUTO, "fr_authused", CTLFLAG_RD,
+	    (void *)offsetof(ipf_auth_softc_t, ipf_auth_used), 0, "");
+	SYSCTL_DYN_IPF_AUTH(_net_inet_ipf, OID_AUTO, "fr_defaultauthage", CTLFLAG_RW,
+	    (void *)offsetof(ipf_auth_softc_t, ipf_auth_defaultage), 0, "");
+	SYSCTL_DYN_IPF_FRAG(_net_inet_ipf, OID_AUTO, "fr_ipfrttl", CTLFLAG_RW,
+	    (void *)offsetof(ipf_frag_softc_t, ipfr_ttl), 0, "");
 	return 0;
 }
 
 static int
-ipf_fbsd_sysctl_destroy(main_softc)
-	ipf_main_softc_t *main_softc;
+ipf_fbsd_sysctl_destroy(void)
 {
-	if (sysctl_ctx_free(&V_ipf_clist)) {
+	if (sysctl_ctx_free(&ipf_clist)) {
 		printf("sysctl_ctx_free failed");
 		return(ENOTEMPTY);
 	}



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