On 27.04.2012 03:44, Hiroki Sato wrote:
"Alexander V. Chernikov"<melif...@freebsd.org> wrote
in<4f96e71b.9020...@freebsd.org>:
me> On 24.04.2012 21:05, Hiroki Sato wrote:
me> > "Alexander V. Chernikov"<melif...@freebsd.org> wrote
me> > in<4f96d11b.2060...@freebsd.org>:
me> >
me> > me> On 24.04.2012 19:26, Hiroki Sato wrote:
me> > me> > Any objection to commit this patch? The primary motivation for
me> > this
me> > me> > change is that presence of the interface by default increases
me> > size of
me> > me> > the interface list, which is returned by NET_RT_IFLIST sysctl
me> > even
me> > me> > when the sysadmin does not need it. Also this pseudo-interface
me> > can
me> > me> > confuse the sysadmin and/or network-related userland utilities
me> > like
me> > me> > SNMP agent. With this patch, one can use ifconfig(8) to
me> > me> > create/destroy the pseudo-interface as necessary.
me> > me>
me> > me> ipfw_log() log_if usage is not protected, so it is possible to
me> > trigger
me> > me> use-after-free.
me> >
me> > Ah, right. I will revise lock handling and resubmit the patch.
me> >
me> > me> Maybe it is better to have some interface flag which makes
me> > me> NET_RT_IFLIST skip given interface ?
me> >
me> > I do not think so. NET_RT_IFLIST should be able to list all of the
me> > interfaces because it is the purpose.
me> Okay, another try (afair already discussed somewhere):
me> Do we really need all BPF providers to have ifnets?
me> It seems that removing all bp_bif depends from BPF code is not so hard
me> task.
Hmm, I cannot imagine how to decouple ifnet from the bpf code because
bpf heavily depends on it in its API (you probably know better than
me). Do you have any specific idea?
Proof-of-concept patch attached.
Unfortunately, there are problems with this approach, too.
pcap_findalldevs() uses external to BPF method (possibly NET_RT_IFLIST),
so programs relying on that function for showing some kind of combo-box
(like wireshark) with all possible variant won't allow user to specify
such interface.
Additionally, tcpdump assumes that passed interface name is real and
warns us that SIOCGIFADDR returns error.
-- Hiroki
diff --git a/sys/net/bpf.c b/sys/net/bpf.c
index 6bff58e..d8ecc02 100644
--- a/sys/net/bpf.c
+++ b/sys/net/bpf.c
@@ -654,7 +654,7 @@ bpf_attachd(struct bpf_d *d, struct bpf_if *bp)
CTR3(KTR_NET, "%s: bpf_attach called by pid %d, adding to %s list",
__func__, d->bd_pid, d->bd_writer ? "writer" : "active");
- if (op_w == 0)
+ if ((op_w == 0) && (bp->bif_ifp != NULL))
EVENTHANDLER_INVOKE(bpf_track, bp->bif_ifp, bp->bif_dlt, 1);
}
@@ -696,7 +696,8 @@ bpf_upgraded(struct bpf_d *d)
CTR2(KTR_NET, "%s: upgrade required by pid %d", __func__, d->bd_pid);
- EVENTHANDLER_INVOKE(bpf_track, bp->bif_ifp, bp->bif_dlt, 1);
+ if (bp->bif_ifp != NULL)
+ EVENTHANDLER_INVOKE(bpf_track, bp->bif_ifp, bp->bif_dlt, 1);
}
/*
@@ -744,14 +745,14 @@ bpf_detachd_locked(struct bpf_d *d)
bpf_bpfd_cnt--;
/* Call event handler iff d is attached */
- if (error == 0)
+ if ((error == 0) && (ifp != NULL))
EVENTHANDLER_INVOKE(bpf_track, ifp, bp->bif_dlt, 0);
/*
* Check if this descriptor had requested promiscuous mode.
* If so, turn it off.
*/
- if (d->bd_promisc) {
+ if (d->bd_promisc && ifp != NULL) {
d->bd_promisc = 0;
CURVNET_SET(ifp->if_vnet);
error = ifpromisc(ifp, 0);
@@ -1034,7 +1035,10 @@ bpfwrite(struct cdev *dev, struct uio *uio, int ioflag)
return (ENXIO);
}
- ifp = d->bd_bif->bif_ifp;
+ if ((ifp = d->bd_bif->bif_ifp) == NULL) {
+ d->bd_wdcount++;
+ return (ENXIO);
+ }
if ((ifp->if_flags & IFF_UP) == 0) {
d->bd_wdcount++;
@@ -1266,8 +1270,10 @@ bpfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int
flags,
if (d->bd_bif == NULL)
error = EINVAL;
else {
- ifp = d->bd_bif->bif_ifp;
- error = (*ifp->if_ioctl)(ifp, cmd, addr);
+ if ((ifp = d->bd_bif->bif_ifp) == NULL)
+ error = EINVAL;
+ else
+ error = (*ifp->if_ioctl)(ifp, cmd,
addr);
}
break;
}
@@ -1322,6 +1328,13 @@ bpfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int
flags,
error = EINVAL;
break;
}
+
+ if (d->bd_bif->bif_ifp == NULL) {
+ /* Silently ignore fake interfaces */
+ error = 0;
+ break;
+ }
+
if (d->bd_promisc == 0) {
error = ifpromisc(d->bd_bif->bif_ifp, 1);
if (error == 0)
@@ -1398,8 +1411,13 @@ bpfioctl(struct cdev *dev, u_long cmd, caddr_t addr, int
flags,
struct ifnet *const ifp = d->bd_bif->bif_ifp;
struct ifreq *const ifr = (struct ifreq *)addr;
- strlcpy(ifr->ifr_name, ifp->if_xname,
- sizeof(ifr->ifr_name));
+ if (ifp == NULL) {
+ /* Fake interface */
+ strlcpy(ifr->ifr_name, d->bd_bif->ifname,
+ sizeof(ifr->ifr_name));
+ } else
+ strlcpy(ifr->ifr_name, ifp->if_xname,
+ sizeof(ifr->ifr_name));
}
BPF_UNLOCK();
break;
@@ -1844,10 +1862,19 @@ bpf_setif(struct bpf_d *d, struct ifreq *ifr)
BPF_LOCK_ASSERT();
theywant = ifunit(ifr->ifr_name);
- if (theywant == NULL || theywant->if_bpf == NULL)
- return (ENXIO);
+ if (theywant == NULL || theywant->if_bpf == NULL) {
+ /* Check for fake interface existance */
+ LIST_FOREACH(bp, &bpf_iflist, bif_next) {
+ if (bp->bif_ifp != NULL)
+ continue;
+ if (!strcmp(bp->ifname, ifr->ifr_name))
+ break;
+ }
- bp = theywant->if_bpf;
+ if (bp == NULL)
+ return (ENXIO);
+ } else
+ bp = theywant->if_bpf;
/* Check if interface is not being detached from BPF */
BPFIF_RLOCK(bp);
@@ -2072,7 +2099,8 @@ bpf_tap(struct bpf_if *bp, u_char *pkt, u_int pktlen)
if (gottime < bpf_ts_quality(d->bd_tstamp))
gottime = bpf_gettime(&bt, d->bd_tstamp, NULL);
#ifdef MAC
- if (mac_bpfdesc_check_receive(d, bp->bif_ifp) == 0)
+ if (bp->bif_ifp == NULL ||
+ (mac_bpfdesc_check_receive(d, bp->bif_ifp) ==
0))
#endif
catchpacket(d, pkt, pktlen, slen,
bpf_append_bytes, &bt);
@@ -2082,6 +2110,7 @@ bpf_tap(struct bpf_if *bp, u_char *pkt, u_int pktlen)
BPFIF_RUNLOCK(bp);
}
+/* Note i CAN be NULL */
#define BPF_CHECK_DIRECTION(d, r, i) \
(((d)->bd_direction == BPF_D_IN && (r) != (i)) || \
((d)->bd_direction == BPF_D_OUT && (r) == (i)))
@@ -2131,7 +2160,8 @@ bpf_mtap(struct bpf_if *bp, struct mbuf *m)
if (gottime < bpf_ts_quality(d->bd_tstamp))
gottime = bpf_gettime(&bt, d->bd_tstamp, m);
#ifdef MAC
- if (mac_bpfdesc_check_receive(d, bp->bif_ifp) == 0)
+ if ((bp->bif_ifp == NULL) ||
+ (mac_bpfdesc_check_receive(d, bp->bif_ifp) ==
0))
#endif
catchpacket(d, (u_char *)m, pktlen, slen,
bpf_append_mbuf, &bt);
@@ -2187,7 +2217,8 @@ bpf_mtap2(struct bpf_if *bp, void *data, u_int dlen,
struct mbuf *m)
if (gottime < bpf_ts_quality(d->bd_tstamp))
gottime = bpf_gettime(&bt, d->bd_tstamp, m);
#ifdef MAC
- if (mac_bpfdesc_check_receive(d, bp->bif_ifp) == 0)
+ if ((bp->bif_ifp == NULL) ||
+ (mac_bpfdesc_check_receive(d, bp->bif_ifp) ==
0))
#endif
catchpacket(d, (u_char *)&mb, pktlen, slen,
bpf_append_mbuf, &bt);
@@ -2481,6 +2512,44 @@ bpfattach2(struct ifnet *ifp, u_int dlt, u_int hdrlen,
struct bpf_if **driverp)
}
/*
+ * Attach fake interface to bpf. ifname is interface name to be attached,
+ * dlt is the link layer type, and hdrlen is the fixed size of the link header
+ * (variable length headers are not yet supporrted).
+ */
+void
+bpfattach3(char *ifname, u_int dlt, u_int hdrlen, struct bpf_if **driverp)
+{
+ struct bpf_if *bp;
+ int len;
+
+ len = strlen(ifname) + 1;
+
+ /* Assume bpf_if to be properly aligned */
+ bp = malloc(sizeof(*bp) + len, M_BPF, M_NOWAIT | M_ZERO);
+ if (bp == NULL)
+ panic("bpfattach");
+
+ LIST_INIT(&bp->bif_dlist);
+ LIST_INIT(&bp->bif_wlist);
+ bp->ifname = (char *)(bp + 1);
+ strlcpy(bp->ifname, ifname, len);
+ bp->bif_dlt = dlt;
+ rw_init(&bp->bif_lock, "bpf interface lock");
+ KASSERT(*driverp == NULL, ("bpfattach3: driverp already initialized"));
+ *driverp = bp;
+
+ BPF_LOCK();
+ LIST_INSERT_HEAD(&bpf_iflist, bp, bif_next);
+ BPF_UNLOCK();
+
+ bp->bif_hdrlen = hdrlen;
+
+ if (bootverbose)
+ printf("%s: bpf attached\n", bp->ifname);
+}
+
+
+/*
* Detach bpf from an interface. This involves detaching each descriptor
* associated with the interface. Notify each descriptor as it's detached
* so that any sleepers wake up and get ENXIO.
@@ -2543,6 +2612,67 @@ bpfdetach(struct ifnet *ifp)
}
/*
+ * Detach bpf from the fake interface. This involves detaching each descriptor
+ * associated with the interface. Notify each descriptor as it's detached
+ * so that any sleepers wake up and get ENXIO.
+ */
+void
+bpfdetach3(char *ifname)
+{
+ struct bpf_if *bp;
+ struct bpf_d *d;
+#ifdef INVARIANTS
+ int ndetached;
+
+ ndetached = 0;
+#endif
+
+ BPF_LOCK();
+ /* Find all bpf_if struct's which reference ifp and detach them. */
+ do {
+ LIST_FOREACH(bp, &bpf_iflist, bif_next) {
+ if (bp->bif_ifp != NULL)
+ continue;
+ if (!strcmp(bp->ifname, ifname))
+ break;
+ }
+ if (bp != NULL)
+ LIST_REMOVE(bp, bif_next);
+
+ if (bp != NULL) {
+#ifdef INVARIANTS
+ ndetached++;
+#endif
+ while ((d = LIST_FIRST(&bp->bif_dlist)) != NULL) {
+ bpf_detachd_locked(d);
+ BPFD_LOCK(d);
+ bpf_wakeup(d);
+ BPFD_UNLOCK(d);
+ }
+ /* Free writer-only descriptors */
+ while ((d = LIST_FIRST(&bp->bif_wlist)) != NULL) {
+ bpf_detachd_locked(d);
+ BPFD_LOCK(d);
+ bpf_wakeup(d);
+ BPFD_UNLOCK(d);
+ }
+
+ /*
+ * Since this interface is fake we can free our
+ * structure immediately.
+ */
+ rw_destroy(&bp->bif_lock);
+ free(bp, M_BPF);
+ }
+ } while (bp != NULL);
+ BPF_UNLOCK();
+
+#ifdef INVARIANTS
+ if (ndetached == 0)
+ printf("bpfdetach: %s was not attached\n", ifname);
+#endif
+}
+/*
* Interface departure handler.
* Note departure event does not guarantee interface is going down.
*/
@@ -2591,6 +2721,9 @@ bpf_getdltlist(struct bpf_d *d, struct bpf_dltlist *bfl)
LIST_FOREACH(bp, &bpf_iflist, bif_next) {
if (bp->bif_ifp != ifp)
continue;
+ /* Compare fake interfaces by name */
+ if (ifp == NULL && strcmp(d->bd_bif->ifname, bp->ifname))
+ continue;
if (bfl->bfl_list != NULL) {
if (n >= bfl->bfl_len)
return (ENOMEM);
@@ -2620,7 +2753,13 @@ bpf_setdlt(struct bpf_d *d, u_int dlt)
ifp = d->bd_bif->bif_ifp;
LIST_FOREACH(bp, &bpf_iflist, bif_next) {
- if (bp->bif_ifp == ifp && bp->bif_dlt == dlt)
+ if (bp->bif_ifp != ifp)
+ continue;
+
+ if (ifp == NULL && strcmp(d->bd_bif->ifname, bp->ifname))
+ continue;
+
+ if (bp->bif_dlt == dlt)
break;
}
@@ -2715,8 +2854,10 @@ bpfstats_fill_xbpf(struct xbpf_d *d, struct bpf_d *bd)
d->bd_hlen = bd->bd_hlen;
d->bd_bufsize = bd->bd_bufsize;
d->bd_pid = bd->bd_pid;
- strlcpy(d->bd_ifname,
- bd->bd_bif->bif_ifp->if_xname, IFNAMSIZ);
+ if (bd->bd_bif->bif_ifp != NULL)
+ strlcpy(d->bd_ifname, bd->bd_bif->bif_ifp->if_xname, IFNAMSIZ);
+ else
+ strlcpy(d->bd_ifname, bd->bd_bif->ifname, IFNAMSIZ);
d->bd_locked = bd->bd_locked;
d->bd_wcount = bd->bd_wcount;
d->bd_wdcount = bd->bd_wdcount;
diff --git a/sys/net/bpf.h b/sys/net/bpf.h
index ba2b8ce..808e8a7 100644
--- a/sys/net/bpf.h
+++ b/sys/net/bpf.h
@@ -1226,6 +1226,7 @@ struct bpf_if {
struct rwlock bif_lock; /* interface lock */
LIST_HEAD(, bpf_d) bif_wlist; /* writer-only list */
int flags; /* Interface flags */
+ char *ifname; /* Fake interface name */
#endif
};
@@ -1236,7 +1237,9 @@ void bpf_mtap(struct bpf_if *, struct mbuf *);
void bpf_mtap2(struct bpf_if *, void *, u_int, struct mbuf *);
void bpfattach(struct ifnet *, u_int, u_int);
void bpfattach2(struct ifnet *, u_int, u_int, struct bpf_if **);
+void bpfattach3(char *, u_int, u_int, struct bpf_if **);
void bpfdetach(struct ifnet *);
+void bpfdetach3(char *);
void bpfilterattach(int);
u_int bpf_filter(const struct bpf_insn *, u_char *, u_int, u_int);
diff --git a/sys/netinet/ipfw/ip_fw_log.c b/sys/netinet/ipfw/ip_fw_log.c
index 983fe3b..e1eb817 100644
--- a/sys/netinet/ipfw/ip_fw_log.c
+++ b/sys/netinet/ipfw/ip_fw_log.c
@@ -89,64 +89,28 @@ ipfw_log_bpf(int onoff)
{
}
#else /* !WITHOUT_BPF */
-static struct ifnet *log_if; /* hook to attach to bpf */
-
-/* we use this dummy function for all ifnet callbacks */
-static int
-log_dummy(struct ifnet *ifp, u_long cmd, caddr_t addr)
-{
- return EINVAL;
-}
-
-static int
-ipfw_log_output(struct ifnet *ifp, struct mbuf *m,
- struct sockaddr *dst, struct route *ro)
-{
- if (m != NULL)
- m_freem(m);
- return EINVAL;
-}
-
-static void
-ipfw_log_start(struct ifnet* ifp)
-{
- panic("ipfw_log_start() must not be called");
-}
+static struct bpf_if *log_bpfif = NULL; /* hook to attach to bpf */
+#define BPF_IFNAME "ipfw0"
+#define IPFW_MTAP(_if_bpf,_data,_dlen,_m) do { \
+ if (bpf_peers_present(_if_bpf)) { \
+ M_ASSERTVALID(_m); \
+ bpf_mtap2((_if_bpf),(_data),(_dlen),(_m)); \
+ } \
+} while (0)
static const u_char ipfwbroadcastaddr[6] =
{ 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
-
void
ipfw_log_bpf(int onoff)
{
- struct ifnet *ifp;
-
if (onoff) {
- if (log_if)
- return;
- ifp = if_alloc(IFT_ETHER);
- if (ifp == NULL)
+ if (log_bpfif)
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;
+ bpfattach3(BPF_IFNAME, DLT_EN10MB, 14, &log_bpfif);
} else {
- if (log_if) {
- ether_ifdetach(log_if);
- if_free(log_if);
- }
- log_if = NULL;
+ if (log_bpfif != NULL)
+ bpfdetach3(BPF_IFNAME);
+ log_bpfif = NULL;
}
}
#endif /* !WITHOUT_BPF */
@@ -167,16 +131,16 @@ ipfw_log(struct ip_fw *f, u_int hlen, struct ip_fw_args
*args,
if (V_fw_verbose == 0) {
#ifndef WITHOUT_BPF
- if (log_if == NULL || log_if->if_bpf == NULL)
+ if (log_bpfif == NULL)
return;
if (args->eh) /* layer2, use orig hdr */
- BPF_MTAP2(log_if, args->eh, ETHER_HDR_LEN, m);
+ IPFW_MTAP(log_bpfif, args->eh, ETHER_HDR_LEN, m);
else
/* Add fake header. Later we will store
* more info in the header.
*/
- BPF_MTAP2(log_if, "DDDDDDSSSSSS\x08\x00",
ETHER_HDR_LEN, m);
+ IPFW_MTAP(log_bpfif, "DDDDDDSSSSSS\x08\x00",
ETHER_HDR_LEN, m);
#endif /* !WITHOUT_BPF */
return;
}
_______________________________________________
freebsd-ipfw@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-ipfw
To unsubscribe, send any mail to "freebsd-ipfw-unsubscr...@freebsd.org"
