From c4b21cbe4a686587f01e66490bb9a658a5d4c5cf Mon Sep 17 00:00:00 2001 From: Julian Elischer Date: Fri, 21 Aug 2009 11:20:10 +0000 Subject: [PATCH] Fix ipfw's initialization functions to get the correct order of evaluation to allow vnet and non vnet operation. Move some functions from ip_fw_pfil.c to ip_fw2.c and mode to mostly using the SYSINIT and VNET_SYSINIT handlers instead of the modevent handler. Correct some spelling errors in comments in the affected code. Note this bug fixes a crash in NON VIMAGE kernels when ipfw is unloaded. This patch is a minimal patch for 8.0 I have a much larger patch that actually fixes the underlying problems that will be applied after 8.0 Reviewed by: zec@, rwatson@, bz@(earlier version) Approved by: re (rwatson) MFC after: Immediatly --- sys/netinet/ip_fw.h | 6 +- sys/netinet/ipfw/ip_fw2.c | 126 +++++++++++++++++++++++++++++----- sys/netinet/ipfw/ip_fw_pfil.c | 60 +++------------- 3 files changed, 123 insertions(+), 69 deletions(-) diff --git a/sys/netinet/ip_fw.h b/sys/netinet/ip_fw.h index 18920a0242cb..9967a29607b0 100644 --- a/sys/netinet/ip_fw.h +++ b/sys/netinet/ip_fw.h @@ -645,8 +645,10 @@ int ipfw_check_out(void *, struct mbuf **, struct ifnet *, int, struct inpcb *in int ipfw_chk(struct ip_fw_args *); -int ipfw_init(void); -void ipfw_destroy(void); +int ipfw_hook(void); +int ipfw6_hook(void); +int ipfw_unhook(void); +int ipfw6_unhook(void); #ifdef NOTYET void ipfw_nat_destroy(void); #endif diff --git a/sys/netinet/ipfw/ip_fw2.c b/sys/netinet/ipfw/ip_fw2.c index 31065daa3082..a17a64792e83 100644 --- a/sys/netinet/ipfw/ip_fw2.c +++ b/sys/netinet/ipfw/ip_fw2.c @@ -102,6 +102,8 @@ __FBSDID("$FreeBSD$"); #include #endif +static VNET_DEFINE(int, ipfw_vnet_ready) = 0; +#define V_ipfw_vnet_ready VNET(ipfw_vnet_ready) /* * set_disable contains one bit per set value (0..31). * If the bit is set, all rules with the corresponding set @@ -2237,7 +2239,7 @@ ipfw_chk(struct ip_fw_args *args) /* end of ipv6 variables */ int is_ipv4 = 0; - if (m->m_flags & M_SKIP_FIREWALL) + if (m->m_flags & M_SKIP_FIREWALL || (! V_ipfw_vnet_ready)) return (IP_FW_PASS); /* accept */ dst_ip.s_addr = 0; /* make sure it is initialized */ @@ -4579,12 +4581,10 @@ ipfw_tick(void * vnetx) CURVNET_RESTORE(); } - - /**************** * Stuff that must be initialised only on boot or module load */ -int +static int ipfw_init(void) { int error = 0; @@ -4623,9 +4623,11 @@ ipfw_init(void) default_to_accept ? "accept" : "deny"); /* - * Note: V_xxx variables can be accessed here but the iattach() - * may not have been called yet for the VIMGE case. - * Tuneables will have been processed. + * Note: V_xxx variables can be accessed here but the vnet specific + * initializer may not have been called yet for the VIMAGE case. + * Tuneables will have been processed. We will print out values for + * the default vnet. + * XXX This should all be rationalized AFTER 8.0 */ if (V_fw_verbose == 0) printf("disabled\n"); @@ -4635,6 +4637,20 @@ ipfw_init(void) printf("limited to %d packets/entry by default\n", V_verbose_limit); + /* + * Hook us up to pfil. + * Eventually pfil will be per vnet. + */ + if ((error = ipfw_hook()) != 0) { + printf("ipfw_hook() error\n"); + return (error); + } +#ifdef INET6 + if ((error = ipfw6_hook()) != 0) { + printf("ipfw6_hook() error\n"); + return (error); + } +#endif /* * Other things that are only done the first time. * (now that we a re cuaranteed of success). @@ -4645,8 +4661,8 @@ ipfw_init(void) } /**************** - * Stuff that must be initialised for every instance - * (including the forst of course). + * Stuff that must be initialized for every instance + * (including the first of course). */ static int vnet_ipfw_init(const void *unused) @@ -4726,17 +4742,17 @@ vnet_ipfw_init(const void *unused) #endif /* First set up some values that are compile time options */ + V_ipfw_vnet_ready = 1; /* Open for business */ return (0); } /********************** * Called for the removal of the last instance only on module unload. */ -void +static void ipfw_destroy(void) { - ip_fw_chk_ptr = NULL; - ip_fw_ctl_ptr = NULL; + uma_zdestroy(ipfw_dyn_rule_zone); IPFW_DYN_LOCK_DESTROY(); printf("IP firewall unloaded\n"); @@ -4750,7 +4766,9 @@ vnet_ipfw_uninit(const void *unused) { struct ip_fw *reap; + V_ipfw_vnet_ready = 0; /* tell new callers to go away */ callout_drain(&V_ipfw_timeout); + /* We wait on the wlock here until the last user leaves */ IPFW_WLOCK(&V_layer3_chain); flush_tables(&V_layer3_chain); V_layer3_chain.reap = NULL; @@ -4766,10 +4784,86 @@ vnet_ipfw_uninit(const void *unused) return 0; } -VNET_SYSINIT(vnet_ipfw_init, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_ANY - 255, - vnet_ipfw_init, NULL); +/* + * Module event handler. + * In general we have the choice of handling most of these events by the + * event handler or by the (VNET_)SYS(UN)INIT handlers. I have chosen to + * use the SYSINIT handlers as they are more capable of expressing the + * flow of control during module and vnet operations, so this is just + * a skeleton. Note there is no SYSINIT equivalent of the module + * SHUTDOWN handler, but we don't have anything to do in that case anyhow. + */ +static int +ipfw_modevent(module_t mod, int type, void *unused) +{ + int err = 0; -VNET_SYSUNINIT(vnet_ipfw_uninit, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_ANY - 255, - vnet_ipfw_uninit, NULL); + switch (type) { + case MOD_LOAD: + /* Called once at module load or + * system boot if compiled in. */ + break; + case MOD_UNLOAD: + break; + case MOD_QUIESCE: + /* Yes, the unhooks can return errors, we can safely ignore + * them. Eventually these will be done per jail as they + * shut down. We will wait on each vnet's l3 lock as existing + * callers go away. + */ + ipfw_unhook(); +#ifdef INET6 + ipfw6_unhook(); +#endif + /* layer2 and other entrypoints still come in this way. */ + ip_fw_chk_ptr = NULL; + ip_fw_ctl_ptr = NULL; + /* Called during unload. */ + break; + case MOD_SHUTDOWN: + /* Called during system shutdown. */ + break; + default: + err = EOPNOTSUPP; + break; + } + return err; +} + +static moduledata_t ipfwmod = { + "ipfw", + ipfw_modevent, + 0 +}; + +/* Define startup order. */ +#define IPFW_SI_SUB_FIREWALL SI_SUB_PROTO_IFATTACHDOMAIN +#define IPFW_MODEVENT_ORDER (SI_ORDER_ANY - 255) /* On boot slot in here. */ +#define IPFW_MODULE_ORDER (IPFW_MODEVENT_ORDER + 1) /* A little later. */ +#define IPFW_VNET_ORDER (IPFW_MODEVENT_ORDER + 2) /* Later still. */ + +DECLARE_MODULE(ipfw, ipfwmod, IPFW_SI_SUB_FIREWALL, IPFW_MODEVENT_ORDER); +MODULE_VERSION(ipfw, 2); +/* should declare some dependencies here */ + +/* + * Starting up. Done in order after ipfwmod() has been called. + * VNET_SYSINIT is also called for each existing vnet and each new vnet. + */ +SYSINIT(ipfw_init, IPFW_SI_SUB_FIREWALL, IPFW_MODULE_ORDER, + ipfw_init, NULL); +VNET_SYSINIT(vnet_ipfw_init, IPFW_SI_SUB_FIREWALL, IPFW_VNET_ORDER, + vnet_ipfw_init, NULL); + +/* + * Closing up shop. These are done in REVERSE ORDER, but still + * after ipfwmod() has been called. Not called on reboot. + * VNET_SYSUNINIT is also called for each exiting vnet as it exits. + * or when the module is unloaded. + */ +SYSUNINIT(ipfw_destroy, IPFW_SI_SUB_FIREWALL, IPFW_MODULE_ORDER, + ipfw_destroy, NULL); +VNET_SYSUNINIT(vnet_ipfw_uninit, IPFW_SI_SUB_FIREWALL, IPFW_VNET_ORDER, + vnet_ipfw_uninit, NULL); diff --git a/sys/netinet/ipfw/ip_fw_pfil.c b/sys/netinet/ipfw/ip_fw_pfil.c index e28d5ca64030..ffffb594546b 100644 --- a/sys/netinet/ipfw/ip_fw_pfil.c +++ b/sys/netinet/ipfw/ip_fw_pfil.c @@ -53,6 +53,7 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include @@ -441,7 +442,7 @@ ipfw_divert(struct mbuf **m, int incoming, int tee) return 1; } -static int +int ipfw_hook(void) { struct pfil_head *pfh_inet; @@ -458,7 +459,7 @@ ipfw_hook(void) return 0; } -static int +int ipfw_unhook(void) { struct pfil_head *pfh_inet; @@ -476,7 +477,7 @@ ipfw_unhook(void) } #ifdef INET6 -static int +int ipfw6_hook(void) { struct pfil_head *pfh_inet6; @@ -493,7 +494,7 @@ ipfw6_hook(void) return 0; } -static int +int ipfw6_unhook(void) { struct pfil_head *pfh_inet6; @@ -517,6 +518,10 @@ ipfw_chg_hook(SYSCTL_HANDLER_ARGS) int enable = *(int *)arg1; int error; +#ifdef VIMAGE /* Since enabling is global, only let base do it. */ + if (! IS_DEFAULT_VNET(curvnet)) + return (EPERM); +#endif error = sysctl_handle_int(oidp, &enable, 0, req); if (error) return (error); @@ -549,50 +554,3 @@ ipfw_chg_hook(SYSCTL_HANDLER_ARGS) return (0); } -static int -ipfw_modevent(module_t mod, int type, void *unused) -{ - int err = 0; - - switch (type) { - case MOD_LOAD: - if ((err = ipfw_init()) != 0) { - printf("ipfw_init() error\n"); - break; - } - if ((err = ipfw_hook()) != 0) { - printf("ipfw_hook() error\n"); - break; - } -#ifdef INET6 - if ((err = ipfw6_hook()) != 0) { - printf("ipfw_hook() error\n"); - break; - } -#endif - break; - - case MOD_UNLOAD: - if ((err = ipfw_unhook()) > 0) - break; -#ifdef INET6 - if ((err = ipfw6_unhook()) > 0) - break; -#endif - ipfw_destroy(); - break; - - default: - return EOPNOTSUPP; - break; - } - return err; -} - -static moduledata_t ipfwmod = { - "ipfw", - ipfw_modevent, - 0 -}; -DECLARE_MODULE(ipfw, ipfwmod, SI_SUB_PROTO_IFATTACHDOMAIN, SI_ORDER_ANY - 256); -MODULE_VERSION(ipfw, 2);