Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 24 Dec 2002 21:58:19 +0100
From:      Aurelien Nephtali <aurelien.nephtali@wanadoo.fr>
To:        current@freebsd.org
Subject:   [PATCH] Wrong behaviour of writes of IP packets through BPF fd
Message-ID:  <20021224205819.GA2042@nebula.wanadoo.fr>

next in thread | raw e-mail | index | archive | help

--qcHopEYAB45HaUaB
Content-Type: multipart/mixed; boundary="VbJkn9YxBvnuCH5J"
Content-Disposition: inline


--VbJkn9YxBvnuCH5J
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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

--VbJkn9YxBvnuCH5J
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="bpf.patch"
Content-Transfer-Encoding: quoted-printable

--- 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>
=20
 #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;
=20
 	if (d->bd_bif =3D=3D 0)
 		return (ENXIO);
@@ -572,6 +576,12 @@
 #ifdef MAC
 	mac_create_mbuf_from_bpfdesc(d, m);
 #endif
+	buf =3D mtod(m, char *);
+	if (dst.sa_family =3D=3D AF_UNSPEC) {
+		ip =3D (struct ip *) buf;
+		ip->ip_len =3D htons(ip->ip_len);
+	}
+
 	error =3D (*ifp->if_output)(ifp, m, &dst, (struct rtentry *)0);
 	mtx_unlock(&Giant);
 	/*

--VbJkn9YxBvnuCH5J
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="dhclient.patch"
Content-Transfer-Encoding: quoted-printable

--- /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 @@
 =09
 	/* Checksum the IP header... */
 	ip.ip_sum =3D wrapsum (checksum ((unsigned char *)&ip, sizeof ip, 0));
-=09
+	ip.ip_len =3D htons(ip.ip_len);
 	/* Copy the ip header into the buffer... */
 	memcpy (&buf [*bufix], &ip, sizeof ip);
 	*bufix +=3D sizeof ip;
@@ -275,12 +275,12 @@
   }
=20
   /* Check the IP packet length. */
-  if (ntohs (ip -> ip_len) !=3D buflen) {
-	  if ((ntohs (ip -> ip_len + 2) & ~1) =3D=3D buflen)
+  if (ip -> ip_len !=3D buflen) {
+	  if (((ip -> ip_len + 2) & ~1) =3D=3D buflen)
 		  ignore =3D 1;
 	  else
 		  log_debug ("ip length %d disagrees with bytes received %d.",
-			     ntohs (ip -> ip_len), buflen);
+			     ip -> ip_len, buflen);
   }
=20
   /* Copy out the IP source address... */

--VbJkn9YxBvnuCH5J--

--qcHopEYAB45HaUaB
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (FreeBSD)

iD8DBQE+CMprDNsbHbt8ok8RAuA4AJsEwj1S4cFo5WdEut2OFSANeJeL5QCgo5BQ
kPLyEzhhDVF5JrKVVaQn+VQ=
=mzVk
-----END PGP SIGNATURE-----

--qcHopEYAB45HaUaB--

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20021224205819.GA2042>