bpf_filter is hard to read.

the difficulty is that it looks like you give it packets in vanilla
memory buffers (ie, a pointer and a length) to read out of, but
packets in the kernel are in mbufs. so if you pass a buffer with a
zero length, the bpf filter code when built in the kernel magically
figures out to do mbuf operations instead.

i was going to change it to get rid of the buffer mode of operation
and explicitely use operations on mbufs all the time, but it turns
out bpf_filter is built as part of libpcap, and libpcap only knows
about vanilla buffers.

so i ended up with this.

this moves the guts of bpf_filter into a new _bpf_filter function
that takes an opaque void pointer to a "thing that has packet data
in it", and a set of function pointers that can do reads against
that opaque thing.

bpf_filter.c includes an implementation of these reads for buffers,
which provides the bpf_filter api that libpcap expects.

bpf.c provides an implementation of these reads for mbufs, and a
bpf_mfilter function that you can call to use it.

thoughts?

tests would be appreciated.

Index: sys/net/bpf.c
===================================================================
RCS file: /cvs/src/sys/net/bpf.c,v
retrieving revision 1.136
diff -u -p -r1.136 bpf.c
--- sys/net/bpf.c       29 Mar 2016 10:38:27 -0000      1.136
+++ sys/net/bpf.c       30 Mar 2016 05:13:44 -0000
@@ -1160,7 +1152,7 @@ bpf_tap(caddr_t arg, u_char *pkt, u_int 
                        bf = srp_enter(&d->bd_rfilter);
                        if (bf != NULL)
                                fcode = bf->bf_insns;
-                       slen = bpf_filter(fcode, pkt, pktlen, 0);
+                       slen = bpf_filter(fcode, pkt, pktlen, pktlen);
                        srp_leave(&d->bd_rfilter, bf);
                }
 
@@ -1254,7 +1244,7 @@ _bpf_mtap(caddr_t arg, struct mbuf *m, u
                        bf = srp_enter(&d->bd_rfilter);
                        if (bf != NULL)
                                fcode = bf->bf_insns;
-                       slen = bpf_filter(fcode, (u_char *)m, pktlen, 0);
+                       slen = bpf_mfilter(fcode, m, pktlen);
                        srp_leave(&d->bd_rfilter, bf);
                }
 
@@ -1748,4 +1738,104 @@ bpf_insn_dtor(void *null, void *f)
 
        free(insns, M_DEVBUF, bf->bf_len * sizeof(*insns));
        free(bf, M_DEVBUF, sizeof(*bf));
+}
+
+u_int32_t      bpf_mbuf_ldw(const void *, u_int32_t, int *);
+u_int32_t      bpf_mbuf_ldh(const void *, u_int32_t, int *);
+u_int32_t      bpf_mbuf_ldb(const void *, u_int32_t, int *);
+
+int            bpf_mbuf_copy(const struct mbuf *, u_int32_t,
+                   void *, u_int32_t);
+
+const struct bpf_ops bpf_mbuf_ops = {
+       bpf_mbuf_ldw,
+       bpf_mbuf_ldh,
+       bpf_mbuf_ldb,
+};
+
+int
+bpf_mbuf_copy(const struct mbuf *m, u_int32_t off, void *buf, u_int32_t len)
+{
+       u_int8_t *cp = buf;
+       u_int32_t count;
+
+       while (off >= m->m_len) {
+               off -= m->m_len;
+
+               m = m->m_next;
+               if (m == NULL)
+                       return (-1);
+       }
+
+       for (;;) {
+               count = min(m->m_len - off, len);
+               
+               memcpy(cp, m->m_data + off, count);
+               len -= count;
+
+               if (len == 0)
+                       return (0);
+
+               m = m->m_next;
+               if (m == NULL)
+                       break;
+
+               cp += count;
+               off = 0;
+       }
+
+       return (-1);
+}
+
+u_int32_t
+bpf_mbuf_ldw(const void *m0, u_int32_t k, int *err)
+{
+       u_int32_t v;
+
+       if (bpf_mbuf_copy(m0, k, &v, sizeof(v)) != 0) {
+               *err = 1;
+               return (0);
+       }
+
+       *err = 0;
+       return ntohl(v);
+}
+
+u_int32_t
+bpf_mbuf_ldh(const void *m0, u_int32_t k, int *err)
+{
+       u_int16_t v;
+
+       if (bpf_mbuf_copy(m0, k, &v, sizeof(v)) != 0) {
+               *err = 1;
+               return (0);
+       }
+
+       *err = 0;
+       return ntohs(v);
+}
+
+u_int32_t
+bpf_mbuf_ldb(const void *m0, u_int32_t k, int *err)
+{
+       const struct mbuf *m = m0;
+
+       while (k >= m->m_len) {
+               k -= m->m_len;
+
+               m = m->m_next;
+               if (m == NULL) {
+                       *err = 1;
+                       return (0);
+               }
+       }
+
+       *err = 0;
+       return (m->m_data[k]);
+}
+
+u_int
+bpf_mfilter(const struct bpf_insn *pc, const struct mbuf *m, u_int wirelen)
+{
+       return _bpf_filter(pc, &bpf_mbuf_ops, m, wirelen);
 }
Index: sys/net/bpf.h
===================================================================
RCS file: /cvs/src/sys/net/bpf.h,v
retrieving revision 1.51
diff -u -p -r1.51 bpf.h
--- sys/net/bpf.h       29 Mar 2016 10:38:27 -0000      1.51
+++ sys/net/bpf.h       30 Mar 2016 05:13:44 -0000
@@ -265,13 +265,28 @@ struct bpf_dltlist {
 };
 
 /*
+ * Load operations for _bpf_filter to use against the packet pointer.
+ */
+struct bpf_ops {
+       u_int32_t       (*ldw)(const void *, u_int32_t, int *);
+       u_int32_t       (*ldh)(const void *, u_int32_t, int *);
+       u_int32_t       (*ldb)(const void *, u_int32_t, int *);
+};
+
+/*
  * Macros for insn array initializers.
  */
 #define BPF_STMT(code, k) { (u_int16_t)(code), 0, 0, k }
 #define BPF_JUMP(code, k, jt, jf) { (u_int16_t)(code), jt, jf, k }
 
+u_int   bpf_filter(struct bpf_insn *, u_char *, u_int, u_int);
+
+u_int   _bpf_filter(const struct bpf_insn *, const struct bpf_ops *,
+            const void *, u_int);
+
 #ifdef _KERNEL
 struct ifnet;
+struct mbuf;
 
 int     bpf_validate(struct bpf_insn *, int);
 int     bpf_tap(caddr_t, u_char *, u_int, u_int);
@@ -283,7 +298,8 @@ int  bpf_mtap_ether(caddr_t, struct mbuf
 void    bpfattach(caddr_t *, struct ifnet *, u_int, u_int);
 void    bpfdetach(struct ifnet *);
 void    bpfilterattach(int);
-u_int   bpf_filter(struct bpf_insn *, u_char *, u_int, u_int);
+
+u_int   bpf_mfilter(const struct bpf_insn *, const struct mbuf *, u_int);
 #endif /* _KERNEL */
 
 /*
Index: sys/net/bpf_filter.c
===================================================================
RCS file: /cvs/src/sys/net/bpf_filter.c,v
retrieving revision 1.27
diff -u -p -r1.27 bpf_filter.c
--- sys/net/bpf_filter.c        13 May 2015 10:42:46 -0000      1.27
+++ sys/net/bpf_filter.c        30 Mar 2016 05:13:44 -0000
@@ -49,99 +49,75 @@
 #endif
 
 #include <sys/endian.h>
-#ifdef __STRICT_ALIGNMENT
-#define BPF_ALIGN
-#endif
-
-#ifndef BPF_ALIGN
-#define EXTRACT_SHORT(p)       ((u_int16_t)ntohs(*(u_int16_t *)p))
-#define EXTRACT_LONG(p)                (ntohl(*(u_int32_t *)p))
-#else
-#define EXTRACT_SHORT(p)\
-       ((u_int16_t)\
-               ((u_int16_t)*((u_char *)p+0)<<8|\
-                (u_int16_t)*((u_char *)p+1)<<0))
-#define EXTRACT_LONG(p)\
-               ((u_int32_t)*((u_char *)p+0)<<24|\
-                (u_int32_t)*((u_char *)p+1)<<16|\
-                (u_int32_t)*((u_char *)p+2)<<8|\
-                (u_int32_t)*((u_char *)p+3)<<0)
-#endif
 
 #ifdef _KERNEL
-#include <sys/mbuf.h>
-#define MINDEX(len, m, k) \
-{ \
-       len = m->m_len; \
-       while (k >= len) { \
-               k -= len; \
-               m = m->m_next; \
-               if (m == NULL) \
-                       return 0; \
-               len = m->m_len; \
-       } \
-}
-
 extern int bpf_maxbufsize;
+#endif
+
+#include <net/bpf.h>
 
-int    bpf_m_xword(struct mbuf *, u_int32_t, int *);
-int    bpf_m_xhalf(struct mbuf *, u_int32_t, int *);
+struct bpf_mem {
+       const u_char    *pkt;
+       u_int            len;
+};
+
+u_int32_t      bpf_mem_ldw(const void *, u_int32_t, int *);
+u_int32_t      bpf_mem_ldh(const void *, u_int32_t, int *);
+u_int32_t      bpf_mem_ldb(const void *, u_int32_t, int *);
+
+const struct bpf_ops bpf_mem_ops = {
+       bpf_mem_ldw,
+       bpf_mem_ldh,
+       bpf_mem_ldb,
+};
 
-int
-bpf_m_xword(struct mbuf *m, u_int32_t k, int *err)
+u_int32_t
+bpf_mem_ldw(const void *mem, u_int32_t k, int *err)
 {
-       int len;
-       u_char *cp, *np;
-       struct mbuf *m0;
+       const struct bpf_mem *bm = mem;
+       u_int32_t v;
 
        *err = 1;
-       MINDEX(len, m, k);
-       cp = mtod(m, u_char *) + k;
-       if (len >= k + 4) {
-               *err = 0;
-               return EXTRACT_LONG(cp);
-       }
-       m0 = m->m_next;
-       if (m0 == NULL || m0->m_len + len - k < 4)
-               return 0;
-       *err = 0;
-       np = mtod(m0, u_char *);
-       switch (len - k) {
 
-       case 1:
-               return (cp[0] << 24) | (np[0] << 16) | (np[1] << 8) | np[2];
+       if (k + sizeof(v) > bm->len)
+               return (0);
 
-       case 2:
-               return (cp[0] << 24) | (cp[1] << 16) | (np[0] << 8) | np[1];
+       memcpy(&v, bm->pkt + k, sizeof(v));
 
-       default:
-               return (cp[0] << 24) | (cp[1] << 16) | (cp[2] << 8) | np[0];
-       }
+       *err = 0;
+       return ntohl(v);
 }
 
-int
-bpf_m_xhalf(struct mbuf *m, u_int32_t k, int *err)
+u_int32_t
+bpf_mem_ldh(const void *mem, u_int32_t k, int *err)
 {
-       int len;
-       u_char *cp;
-       struct mbuf *m0;
+       const struct bpf_mem *bm = mem;
+       u_int16_t v;
 
        *err = 1;
-       MINDEX(len, m, k);
-       cp = mtod(m, u_char *) + k;
-       if (len >= k + 2) {
-               *err = 0;
-               return EXTRACT_SHORT(cp);
-       }
-       m0 = m->m_next;
-       if (m0 == NULL)
-               return 0;
+
+       if (k + sizeof(v) > bm->len)
+               return (0);
+
+       memcpy(&v, bm->pkt + k, sizeof(v));
+
        *err = 0;
-       return (cp[0] << 8) | mtod(m0, u_char *)[0];
+       return ntohs(v);
 }
-#endif
 
-#include <net/bpf.h>
+u_int32_t
+bpf_mem_ldb(const void *mem, u_int32_t k, int *err)
+{
+       const struct bpf_mem *bm = mem;
+
+       *err = 1;
+
+       if (k >= bm->len)
+               return (0);
+
+       *err = 0;
+       return bm->pkt[k];
+}
 
 /*
  * Execute the filter program starting at pc on the packet p
@@ -149,19 +125,34 @@ bpf_m_xhalf(struct mbuf *m, u_int32_t k,
  * buflen is the amount of data present
  */
 u_int
-bpf_filter(struct bpf_insn *pc, u_char *p, u_int wirelen, u_int buflen)
+bpf_filter(struct bpf_insn *pc, u_char *pkt, u_int wirelen, u_int buflen)
+{
+       struct bpf_mem bm;
+
+       bm.pkt = pkt;
+       bm.len = buflen;
+
+       return _bpf_filter(pc, &bpf_mem_ops, &bm, wirelen);
+}
+
+u_int
+_bpf_filter(const struct bpf_insn *pc, const struct bpf_ops *ops,
+    const void *pkt, u_int wirelen)
 {
        u_int32_t A = 0, X = 0;
        u_int32_t k;
        int32_t mem[BPF_MEMWORDS];
+       int err;
 
        bzero(mem, sizeof(mem));
 
-       if (pc == 0)
+       if (pc == NULL) {
                /*
                 * No filter means accept all.
                 */
                return (u_int)-1;
+       }
+
        --pc;
        while (1) {
                ++pc;
@@ -180,61 +171,21 @@ bpf_filter(struct bpf_insn *pc, u_char *
                        return (u_int)A;
 
                case BPF_LD|BPF_W|BPF_ABS:
-                       k = pc->k;
-                       if (k > buflen || sizeof(int32_t) > buflen - k) {
-#ifdef _KERNEL
-                               int merr;
-
-                               if (buflen != 0)
-                                       return 0;
-                               A = bpf_m_xword((struct mbuf *)p, k, &merr);
-                               if (merr != 0)
-                                       return 0;
-                               continue;
-#else
+                       A = ops->ldw(pkt, pc->k, &err);
+                       if (err != 0)
                                return 0;
-#endif
-                       }
-                       A = EXTRACT_LONG(&p[k]);
                        continue;
 
                case BPF_LD|BPF_H|BPF_ABS:
-                       k = pc->k;
-                       if (k > buflen || sizeof(int16_t) > buflen - k) {
-#ifdef _KERNEL
-                               int merr;
-
-                               if (buflen != 0)
-                                       return 0;
-                               A = bpf_m_xhalf((struct mbuf *)p, k, &merr);
-                               if (merr != 0)
-                                       return 0;
-                               continue;
-#else
+                       A = ops->ldh(pkt, pc->k, &err);
+                       if (err != 0)
                                return 0;
-#endif
-                       }
-                       A = EXTRACT_SHORT(&p[k]);
                        continue;
 
                case BPF_LD|BPF_B|BPF_ABS:
-                       k = pc->k;
-                       if (k >= buflen) {
-#ifdef _KERNEL
-                               struct mbuf *m;
-                               int len;
-
-                               if (buflen != 0)
-                                       return 0;
-                               m = (struct mbuf *)p;
-                               MINDEX(len, m, k);
-                               A = mtod(m, u_char *)[k];
-                               continue;
-#else
+                       A = ops->ldb(pkt, pc->k, &err);
+                       if (err != 0)
                                return 0;
-#endif
-                       }
-                       A = p[k];
                        continue;
 
                case BPF_LD|BPF_W|BPF_LEN:
@@ -247,80 +198,31 @@ bpf_filter(struct bpf_insn *pc, u_char *
 
                case BPF_LD|BPF_W|BPF_IND:
                        k = X + pc->k;
-                       if (k > buflen || sizeof(int32_t) > buflen - k) {
-#ifdef _KERNEL
-                               int merr;
-
-                               if (buflen != 0)
-                                       return 0;
-                               A = bpf_m_xword((struct mbuf *)p, k, &merr);
-                               if (merr != 0)
-                                       return 0;
-                               continue;
-#else
+                       A = ops->ldw(pkt, k, &err);
+                       if (err != 0)
                                return 0;
-#endif
-                       }
-                       A = EXTRACT_LONG(&p[k]);
                        continue;
 
                case BPF_LD|BPF_H|BPF_IND:
                        k = X + pc->k;
-                       if (k > buflen || sizeof(int16_t) > buflen - k) {
-#ifdef _KERNEL
-                               int merr;
-
-                               if (buflen != 0)
-                                       return 0;
-                               A = bpf_m_xhalf((struct mbuf *)p, k, &merr);
-                               if (merr != 0)
-                                       return 0;
-                               continue;
-#else
+                       A = ops->ldh(pkt, k, &err);
+                       if (err != 0)
                                return 0;
-#endif
-                       }
-                       A = EXTRACT_SHORT(&p[k]);
                        continue;
 
                case BPF_LD|BPF_B|BPF_IND:
                        k = X + pc->k;
-                       if (k >= buflen) {
-#ifdef _KERNEL
-                               struct mbuf *m;
-                               int len;
-
-                               if (buflen != 0)
-                                       return 0;
-                               m = (struct mbuf *)p;
-                               MINDEX(len, m, k);
-                               A = mtod(m, u_char *)[k];
-                               continue;
-#else
+                       A = ops->ldb(pkt, k, &err);
+                       if (err != 0)
                                return 0;
-#endif
-                       }
-                       A = p[k];
                        continue;
 
                case BPF_LDX|BPF_MSH|BPF_B:
-                       k = pc->k;
-                       if (k >= buflen) {
-#ifdef _KERNEL
-                               struct mbuf *m;
-                               int len;
-
-                               if (buflen != 0)
-                                       return 0;
-                               m = (struct mbuf *)p;
-                               MINDEX(len, m, k);
-                               X = (mtod(m, u_char *)[k] & 0xf) << 2;
-                               continue;
-#else
+                       X = ops->ldb(pkt, pc->k, &err);
+                       if (err != 0)
                                return 0;
-#endif
-                       }
-                       X = (p[pc->k] & 0xf) << 2;
+                       X &= 0xf;
+                       X <<= 2;
                        continue;
 
                case BPF_LD|BPF_IMM:
Index: sys/net/bpfdesc.h
===================================================================
RCS file: /cvs/src/sys/net/bpfdesc.h,v
retrieving revision 1.29
diff -u -p -r1.29 bpfdesc.h
--- sys/net/bpfdesc.h   3 Dec 2015 16:27:32 -0000       1.29
+++ sys/net/bpfdesc.h   30 Mar 2016 05:13:44 -0000
@@ -80,7 +80,6 @@ struct bpf_d {
        u_char          bd_locked;      /* true if descriptor is locked */
        u_char          bd_fildrop;     /* true if filtered packets will be 
dropped */
        u_char          bd_dirfilt;     /* direction filter */
-       u_int           bd_queue;       /* the queue the user wants to watch (0 
== all) */
        int             bd_hdrcmplt;    /* false to fill in src lladdr 
automatically */
        int             bd_async;       /* non-zero if packet reception should 
generate signal */
        int             bd_sig;         /* signal to send upon packet reception 
*/
Index: lib/libpcap/pcap.h
===================================================================
RCS file: /cvs/src/lib/libpcap/pcap.h,v
retrieving revision 1.16
diff -u -p -r1.16 pcap.h
--- lib/libpcap/pcap.h  11 Apr 2014 04:08:58 -0000      1.16
+++ lib/libpcap/pcap.h  30 Mar 2016 05:13:44 -0000
@@ -226,8 +226,6 @@ void        pcap_freealldevs(pcap_if_t *);
 
 const char *pcap_lib_version(void);
 
-/* XXX this guy lives in the bpf tree */
-u_int  bpf_filter(struct bpf_insn *, u_char *, u_int, u_int);
 char   *bpf_image(struct bpf_insn *, int);
 
 int    pcap_get_selectable_fd(pcap_t *);

Reply via email to