[PATCH] Wrong behaviour of writes of IP packets through BPF fd

[Aurelien Nephtali]

Hi,

I think I've found a bug in the BPF stack (if I can call it a stack :p).
According to the bpf man, packets can be written directly through a bpf file
descriptor. But writing IP packets using write() doesn't seem to work, the
"ip_len" field of the ip header isn't sent in host byte order so the packet is
discarded by the remote host since the len of the packet doesn't match the
length of the data captured...
BSD is known to have a strange behaviour with the "ip_len" field, returning
EINVAL when this field is htons()'ized and passed to functions like write(),
send(), etc... and of course, writing of a BPF fd using write() doesn't
break this "rule". :/

But, strangely, as writing with write() doesn't work, writing with writev()
seems to work (?!). That's why "dhclient" works fine _EVEN_ if it htons()
"ip_len" field of his IP packets!

Attached are two patches, one against bpf.c (kernel) and the other against
packet.c (dhclient).
These patches are ugly and you can (at least you are encouraged to :p) modify
them or tell me what is good or not with them. They're only here to try to
illustrate what I'm trying to explain :)

For the BPF patch, I don't know if the test
        if (dst.sa_family == AF_UNSPEC)
is correct ... but it seems to work and I wonder why sa_family isn't AF_INET...

BTW, -STABLE/-RELEASE is also *affected* and I think the patch for bpf.c can
also be applied against -STABLE, I've checked, bpf.c from -STABLE and the one
from -CURRENT are the same.

Hope I was clear even if my english isn't as good as it should be :)
I plan to put this into a PR but I want some comments to be sure that's not
a "desired" feature.

-- Aurelien

--- sys/net/bpf.c       Tue Nov 19 03:50:46 2002
+++ sys/net/bpf.c       Tue Dec 24 19:11:49 2002
@@ -68,6 +68,8 @@
 #include <net/bpfdesc.h>
 
 #include <netinet/in.h>
+#include <netinet/in_systm.h>
+#include <netinet/ip.h>
 #include <netinet/if_ether.h>
 #include <sys/kernel.h>
 #include <sys/sysctl.h>
@@ -549,6 +551,8 @@
        int error;
        static struct sockaddr dst;
        int datlen;
+       struct ip *ip;
+       char *buf;
 
        if (d->bd_bif == 0)
                return (ENXIO);
@@ -572,6 +576,12 @@
 #ifdef MAC
        mac_create_mbuf_from_bpfdesc(d, m);
 #endif
+       buf = mtod(m, char *);
+       if (dst.sa_family == AF_UNSPEC) {
+               ip = (struct ip *) buf;
+               ip->ip_len = htons(ip->ip_len);
+       }
+
        error = (*ifp->if_output)(ifp, m, &dst, (struct rtentry *)0);
        mtx_unlock(&Giant);
        /*

--- /usr/src/contrib/isc-dhcp/common/packet.c   Tue Feb 19 12:04:33 2002
+++ /usr/src/contrib/isc-dhcp/common/packet.c   Tue Dec 24 19:14:17 2002
@@ -164,7 +164,7 @@
        
        /* Checksum the IP header... */
        ip.ip_sum = wrapsum (checksum ((unsigned char *)&ip, sizeof ip, 0));
-       
+       ip.ip_len = htons(ip.ip_len);
        /* Copy the ip header into the buffer... */
        memcpy (&buf [*bufix], &ip, sizeof ip);
        *bufix += sizeof ip;
@@ -275,12 +275,12 @@
   }
 
   /* Check the IP packet length. */
-  if (ntohs (ip -> ip_len) != buflen) {
-         if ((ntohs (ip -> ip_len + 2) & ~1) == buflen)
+  if (ip -> ip_len != buflen) {
+         if (((ip -> ip_len + 2) & ~1) == buflen)
                  ignore = 1;
          else
                  log_debug ("ip length %d disagrees with bytes received %d.",
-                            ntohs (ip -> ip_len), buflen);
+                            ip -> ip_len, buflen);
   }
 
   /* Copy out the IP source address... */

msg49294/pgp00000.pgp
Description: PGP signature