From owner-freebsd-ipfw@FreeBSD.ORG Sat Apr 28 14:22:33 2012 Return-Path: Delivered-To: freebsd-ipfw@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 63EEE106564A; Sat, 28 Apr 2012 14:22:33 +0000 (UTC) (envelope-from hrs@FreeBSD.org) Received: from mail.allbsd.org (gatekeeper-int.allbsd.org [IPv6:2001:2f0:104:e002::2]) by mx1.freebsd.org (Postfix) with ESMTP id BC6908FC15; Sat, 28 Apr 2012 14:22:31 +0000 (UTC) Received: from alph.allbsd.org (p4242-ipbf1504funabasi.chiba.ocn.ne.jp [118.7.211.242]) (authenticated bits=128) by mail.allbsd.org (8.14.4/8.14.4) with ESMTP id q3SEMAqk096888; Sat, 28 Apr 2012 23:22:20 +0900 (JST) (envelope-from hrs@FreeBSD.org) Received: from localhost ([IPv6:::]) (authenticated bits=0) by alph.allbsd.org (8.14.4/8.14.4) with ESMTP id q3SEM7Wh044026; Sat, 28 Apr 2012 23:22:09 +0900 (JST) (envelope-from hrs@FreeBSD.org) Date: Sat, 28 Apr 2012 23:18:00 +0900 (JST) Message-Id: <20120428.231800.306465812317617923.hrs@allbsd.org> To: melifaro@FreeBSD.org, kudzu@tenebras.com From: Hiroki Sato In-Reply-To: <20120425.020518.406495893112283552.hrs@allbsd.org> References: <20120425.002600.1631867625819249738.hrs@allbsd.org> <4F96D11B.2060007@FreeBSD.org> <20120425.020518.406495893112283552.hrs@allbsd.org> X-PGPkey-fingerprint: BDB3 443F A5DD B3D0 A530 FFD7 4F2C D3D8 2793 CF2D X-Mailer: Mew version 6.4.50 on Emacs 23.4 / Mule 6.0 (HANACHIRUSATO) Mime-Version: 1.0 Content-Type: Multipart/Signed; protocol="application/pgp-signature"; micalg=pgp-sha1; boundary="--Security_Multipart0(Sat_Apr_28_23_18_00_2012_214)--" Content-Transfer-Encoding: 7bit X-Virus-Scanned: clamav-milter 0.97.3 at gatekeeper.allbsd.org X-Virus-Status: Clean X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.3 (mail.allbsd.org [133.31.130.32]); Sat, 28 Apr 2012 23:22:24 +0900 (JST) X-Spam-Status: No, score=-103.6 required=13.0 tests=BAYES_00, CONTENT_TYPE_PRESENT,FAKEDWORD_ZERO,RCVD_IN_RP_RNBL,SPF_SOFTFAIL, USER_IN_WHITELIST autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on gatekeeper.allbsd.org Cc: freebsd-ipfw@FreeBSD.org Subject: Re: CFR: ipfw0 pseudo-interface clonable X-BeenThere: freebsd-ipfw@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: IPFW Technical Discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 28 Apr 2012 14:22:33 -0000 ----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 wrote in <20120425.020518.406495893112283552.hrs@allbsd.org>: hr> "Alexander V. Chernikov" 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 wrote in : 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 #include #include +#include +#include #include /* for ETHERTYPE_IP */ #include +#include #include #include /* for IFT_ETHER */ #include /* 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)----