Hiroki Sato <[email protected]> wrote in <[email protected]>:
hr> "Alexander V. Chernikov" <[email protected]> wrote hr> in <[email protected]>: 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 <[email protected]> wrote in <CAHu1Y70kbxu6dquUKvrLwQwjLBuCiv2wu7QitoUHro17=rj...@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
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
pgpRPHezJVeXW.pgp
Description: PGP signature
