there are two bugs in the bpf code in dhcrelay. they're obviously
rare though.
the worst one is when skipping a packet, dhcrelay uses the = operator
instead of + when summing the bpf header and capture length.
the second bug is also in calculating how much space a packet
consumes to skip over it. bpf aligns packets to a bpf word boundary,
but reports buffer lenghts in bytes. a program has to calculate the
next packet boundary with these values, but dhcrelay isnt. the
result of this would be dhcrelay using packet data and the wrong
fields in a bpf header, which is bad.
the fix for this is to calculate the offset to the next packet once,
and add that to the offset when skipping a packet. the code currently
skips the bpf header to get at the packet, which is now changed so
the packet start is accounted separately.
ive only tested that the code still works, i havent come up with a
test for the cases this diff fixes. ok?
ps, i think the whole code smells a bit, but i like the free diffz.
Index: bpf.c
===================================================================
RCS file: /cvs/src/usr.sbin/dhcrelay/bpf.c,v
retrieving revision 1.19
diff -u -p -r1.19 bpf.c
--- bpf.c 19 Apr 2017 05:36:12 -0000 1.19
+++ bpf.c 24 Nov 2017 02:10:48 -0000
@@ -375,6 +375,7 @@ receive_packet(struct interface_info *in
int length = 0;
ssize_t offset = 0;
struct bpf_hdr hdr;
+ size_t bpflen, pktoff;
/*
* All this complexity is because BPF doesn't guarantee that
@@ -412,12 +413,14 @@ receive_packet(struct interface_info *in
memcpy(&hdr, &interface->rbuf[interface->rbuf_offset],
sizeof(hdr));
+ bpflen = BPF_WORDALIGN(hdr.bh_hdrlen + hdr.bh_caplen);
+ pktoff = interface->rbuf_offset + hdr.bh_hdrlen;
+
/*
* If the bpf header plus data doesn't fit in what's
* left of the buffer, stick head in sand yet again...
*/
- if (interface->rbuf_offset + hdr.bh_hdrlen + hdr.bh_caplen >
- interface->rbuf_len) {
+ if (interface->rbuf_offset + bpflen > interface->rbuf_len) {
interface->rbuf_offset = interface->rbuf_len;
continue;
}
@@ -427,18 +430,14 @@ receive_packet(struct interface_info *in
* the packet won't fit in the input buffer, all we can
* do is drop it.
*/
- if (hdr.bh_caplen != hdr.bh_datalen) {
- interface->rbuf_offset += hdr.bh_hdrlen =
- hdr.bh_caplen;
+ if (hdr.bh_caplen < hdr.bh_datalen) {
+ interface->rbuf_offset += bpflen;
continue;
}
- /* Skip over the BPF header... */
- interface->rbuf_offset += hdr.bh_hdrlen;
-
/* Decode the physical header... */
offset = decode_hw_header(interface->rbuf,
- interface->rbuf_len, interface->rbuf_offset, pc,
+ interface->rbuf_len, pktoff, pc,
interface->hw_address.htype);
/*
@@ -447,7 +446,7 @@ receive_packet(struct interface_info *in
* skip this packet.
*/
if (offset < 0) {
- interface->rbuf_offset += hdr.bh_caplen;
+ interface->rbuf_offset += bpflen;
continue;
}
@@ -457,27 +456,23 @@ receive_packet(struct interface_info *in
/* If the IP or UDP checksum was bad, skip the packet... */
if (offset < 0) {
- interface->rbuf_offset += hdr.bh_caplen;
+ interface->rbuf_offset += bpflen;
continue;
}
- hdr.bh_caplen -= offset - interface->rbuf_offset;
- interface->rbuf_offset = offset;
-
/*
* If there's not enough room to stash the packet data,
* we have to skip it (this shouldn't happen in real
* life, though).
*/
if (hdr.bh_caplen > len) {
- interface->rbuf_offset += hdr.bh_caplen;
+ interface->rbuf_offset += bpflen;
continue;
}
/* Copy out the data in the packet... */
- memcpy(buf, interface->rbuf + interface->rbuf_offset,
- hdr.bh_caplen);
- interface->rbuf_offset += hdr.bh_caplen;
+ memcpy(buf, interface->rbuf + pktoff, hdr.bh_caplen);
+ interface->rbuf_offset += bpflen;
return (hdr.bh_caplen);
} while (!length);
return (0);
