Date: Sat, 28 Apr 2012 23:18:00 +0900 (JST) From: Hiroki Sato <hrs@FreeBSD.org> To: melifaro@FreeBSD.org, kudzu@tenebras.com Cc: freebsd-ipfw@FreeBSD.org Subject: Re: CFR: ipfw0 pseudo-interface clonable Message-ID: <20120428.231800.306465812317617923.hrs@allbsd.org> In-Reply-To: <20120425.020518.406495893112283552.hrs@allbsd.org> <CAHu1Y70kbxu6dquUKvrLwQwjLBuCiv2wu7QitoUHro17=rjbGA@mail.gmail.com> References: <20120425.002600.1631867625819249738.hrs@allbsd.org> <4F96D11B.2060007@FreeBSD.org> <20120425.020518.406495893112283552.hrs@allbsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
----Security_Multipart0(Sat_Apr_28_23_18_00_2012_214)-- Content-Type: Multipart/Mixed; boundary="--Next_Part(Sat_Apr_28_23_18_00_2012_764)--" Content-Transfer-Encoding: 7bit ----Next_Part(Sat_Apr_28_23_18_00_2012_764)-- Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Hiroki Sato <hrs@freebsd.org> wrote in <20120425.020518.406495893112283552.hrs@allbsd.org>: hr> "Alexander V. Chernikov" <melifaro@FreeBSD.org> wrote hr> in <4F96D11B.2060007@FreeBSD.org>: hr> hr> me> On 24.04.2012 19:26, Hiroki Sato wrote: hr> me> > Hi, hr> me> > hr> me> > I created the attached patch to make the current ipfw0 hr> me> > pseudo-interface clonable. The functionality of ipfw0 logging hr> me> > interface is not changed by this patch, but the ipfw0 hr> me> > pseudo-interface is not created by default and can be created with hr> me> > the following command: hr> me> ipfw_log() log_if usage is not protected, so it is possible to trigger hr> me> use-after-free. hr> hr> Ah, right. I will revise lock handling and resubmit the patch. Michael Sierchio <kudzu@tenebras.com> wrote in <CAHu1Y70kbxu6dquUKvrLwQwjLBuCiv2wu7QitoUHro17=rjbGA@mail.gmail.com>: ku> A man page for the ipfw pseudo-interface would be welcome. A revised patch is attached. The lock around log_if should be fixed and ipfw(8) manual page is updated. Also, an rc.conf(5) variable $firewall_logif is added to create ipfw0 interface at boot time (NO by default). Any comments are welcome. Thank you. -- Hiroki ----Next_Part(Sat_Apr_28_23_18_00_2012_764)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="ipfw_clone_interface.20120429-1.diff" Index: sys/netinet/ipfw/ip_fw_log.c =================================================================== --- sys/netinet/ipfw/ip_fw_log.c (revision 234428) +++ sys/netinet/ipfw/ip_fw_log.c (working copy) @@ -44,8 +44,11 @@ #include <sys/socket.h> #include <sys/sysctl.h> #include <sys/syslog.h> +#include <sys/lock.h> +#include <sys/rwlock.h> #include <net/ethernet.h> /* for ETHERTYPE_IP */ #include <net/if.h> +#include <net/if_clone.h> #include <net/vnet.h> #include <net/if_types.h> /* for IFT_ETHER */ #include <net/bpf.h> /* for BPF */ @@ -90,7 +93,16 @@ } #else /* !WITHOUT_BPF */ static struct ifnet *log_if; /* hook to attach to bpf */ +static struct rwlock log_if_lock; +#define LOGIF_LOCK_INIT(x) rw_init(&log_if_lock, "ipfw log_if lock") +#define LOGIF_LOCK_DESTROY(x) rw_destroy(&log_if_lock) +#define LOGIF_RLOCK(x) rw_rlock(&log_if_lock) +#define LOGIF_RUNLOCK(x) rw_runlock(&log_if_lock) +#define LOGIF_WLOCK(x) rw_wlock(&log_if_lock) +#define LOGIF_WUNLOCK(x) rw_wunlock(&log_if_lock) +#define IPFWNAME "ipfw" + /* we use this dummy function for all ifnet callbacks */ static int log_dummy(struct ifnet *ifp, u_long cmd, caddr_t addr) @@ -116,37 +128,104 @@ static const u_char ipfwbroadcastaddr[6] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff }; +static int +ipfw_log_clone_match(struct if_clone *ifc, const char *name) +{ + + return (strncmp(name, IPFWNAME, sizeof(IPFWNAME) - 1) == 0); +} + +static int +ipfw_log_clone_create(struct if_clone *ifc, char *name, size_t len, caddr_t params) +{ + int error; + int unit; + struct ifnet *ifp; + + error = ifc_name2unit(name, &unit); + if (error) + return (error); + + error = ifc_alloc_unit(ifc, &unit); + if (error) + return (error); + + ifp = if_alloc(IFT_ETHER); + if (ifp == NULL) { + ifc_free_unit(ifc, unit); + return (ENOSPC); + } + ifp->if_dname = IPFWNAME; + ifp->if_dunit = unit; + snprintf(ifp->if_xname, IFNAMSIZ, "%s%d", IPFWNAME, unit); + strlcpy(name, ifp->if_xname, len); + ifp->if_mtu = 65536; + ifp->if_flags = IFF_UP | IFF_SIMPLEX | IFF_MULTICAST; + ifp->if_init = (void *)log_dummy; + ifp->if_ioctl = log_dummy; + ifp->if_start = ipfw_log_start; + ifp->if_output = ipfw_log_output; + ifp->if_addrlen = 6; + ifp->if_hdrlen = 14; + ifp->if_broadcastaddr = ipfwbroadcastaddr; + ifp->if_baudrate = IF_Mbps(10); + + LOGIF_WLOCK(); + if (log_if == NULL) + log_if = ifp; + else { + LOGIF_WUNLOCK(); + if_free(ifp); + ifc_free_unit(ifc, unit); + return (EEXIST); + } + LOGIF_WUNLOCK(); + if_attach(ifp); + bpfattach(ifp, DLT_EN10MB, 14); + + return (0); +} + +static int +ipfw_log_clone_destroy(struct if_clone *ifc, struct ifnet *ifp) +{ + int unit; + + if (ifp == NULL) + return (0); + + LOGIF_WLOCK(); + if (log_if != NULL && ifp == log_if) + log_if = NULL; + else { + LOGIF_WUNLOCK(); + return (EINVAL); + } + LOGIF_WUNLOCK(); + + unit = ifp->if_dunit; + bpfdetach(ifp); + if_detach(ifp); + if_free(ifp); + ifc_free_unit(ifc, unit); + + return (0); +} + +static struct if_clone ipfw_log_cloner = IFC_CLONE_INITIALIZER( + IPFWNAME, NULL, IF_MAXUNIT, + NULL, ipfw_log_clone_match, ipfw_log_clone_create, ipfw_log_clone_destroy); + void ipfw_log_bpf(int onoff) { - struct ifnet *ifp; if (onoff) { - if (log_if) - return; - ifp = if_alloc(IFT_ETHER); - if (ifp == NULL) - return; - if_initname(ifp, "ipfw", 0); - ifp->if_mtu = 65536; - ifp->if_flags = IFF_UP | IFF_SIMPLEX | IFF_MULTICAST; - ifp->if_init = (void *)log_dummy; - ifp->if_ioctl = log_dummy; - ifp->if_start = ipfw_log_start; - ifp->if_output = ipfw_log_output; - ifp->if_addrlen = 6; - ifp->if_hdrlen = 14; - if_attach(ifp); - ifp->if_broadcastaddr = ipfwbroadcastaddr; - ifp->if_baudrate = IF_Mbps(10); - bpfattach(ifp, DLT_EN10MB, 14); - log_if = ifp; + LOGIF_LOCK_INIT(); + if_clone_attach(&ipfw_log_cloner); } else { - if (log_if) { - ether_ifdetach(log_if); - if_free(log_if); - } - log_if = NULL; + if_clone_detach(&ipfw_log_cloner); + LOGIF_LOCK_DESTROY(); } } #endif /* !WITHOUT_BPF */ @@ -166,9 +245,11 @@ if (V_fw_verbose == 0) { #ifndef WITHOUT_BPF - - if (log_if == NULL || log_if->if_bpf == NULL) + LOGIF_RLOCK(); + if (log_if == NULL || log_if->if_bpf == NULL) { + LOGIF_RUNLOCK(); return; + } if (args->eh) /* layer2, use orig hdr */ BPF_MTAP2(log_if, args->eh, ETHER_HDR_LEN, m); @@ -177,6 +258,7 @@ * more info in the header. */ BPF_MTAP2(log_if, "DDDDDDSSSSSS\x08\x00", ETHER_HDR_LEN, m); + LOGIF_RUNLOCK(); #endif /* !WITHOUT_BPF */ return; } Index: sbin/ipfw/ipfw.8 =================================================================== --- sbin/ipfw/ipfw.8 (revision 234428) +++ sbin/ipfw/ipfw.8 (working copy) @@ -1,7 +1,7 @@ .\" .\" $FreeBSD$ .\" -.Dd March 9, 2012 +.Dd April 28, 2012 .Dt IPFW 8 .Os .Sh NAME @@ -560,7 +560,22 @@ .Xr bpf 4 attached to the .Li ipfw0 -pseudo interface. There is no overhead if no +pseudo interface. +This pseudo interface can be created after a boot +manually by using the following command: +.Bd -literal -offset indent +# ifconfig ipfw0 create +.Ed +.Pp +Or, automatically at boot time by adding the following +line to the +.Xr rc.conf 5 +file: +.Bd -literal -offset indent +firewall_logif="YES" +.Ed +.Pp +There is no overhead if no .Xr bpf 4 is attached to the pseudo interface. .Pp Index: etc/rc.d/ipfw =================================================================== --- etc/rc.d/ipfw (revision 234412) +++ etc/rc.d/ipfw (working copy) @@ -57,6 +57,10 @@ echo 'Firewall logging enabled.' sysctl net.inet.ip.fw.verbose=1 >/dev/null fi + if checkyesno firewall_logif; then + echo 'Firewall logging pseudo-interface (ipfw0) created.' + ifconfig ipfw0 create + fi } ipfw_poststart() Index: etc/defaults/rc.conf =================================================================== --- etc/defaults/rc.conf (revision 234428) +++ etc/defaults/rc.conf (working copy) @@ -123,6 +123,7 @@ firewall_type="UNKNOWN" # Firewall type (see /etc/rc.firewall) firewall_quiet="NO" # Set to YES to suppress rule display firewall_logging="NO" # Set to YES to enable events logging +firewall_logif="NO" # Set to YES to create logging-pseudo interface firewall_flags="" # Flags passed to ipfw when type is a file firewall_coscripts="" # List of executables/scripts to run after # firewall starts/stops ----Next_Part(Sat_Apr_28_23_18_00_2012_764)---- ----Security_Multipart0(Sat_Apr_28_23_18_00_2012_214)-- Content-Type: application/pgp-signature Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iEYEABECAAYFAk+b/BgACgkQTyzT2CeTzy2fdwCghazmRs6QeMA0631ZY3CGyTNC oDwAoIlL9GEFhmLy7Bw7epRAU9swB+EY =MiFt -----END PGP SIGNATURE----- ----Security_Multipart0(Sat_Apr_28_23_18_00_2012_214)----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120428.231800.306465812317617923.hrs>