| Summary: | ipfw fragment logging misses first frag | ||
|---|---|---|---|
| Product: | Base System | Reporter: | Crist J. Clark <cjc> |
| Component: | kern | Assignee: | bill fumerola <billf> |
| Status: | Closed FIXED | ||
| Severity: | Affects Only Me | CC: | ipfw |
| Priority: | Normal | ||
| Version: | 4.1-STABLE | ||
| Hardware: | Any | ||
| OS: | Any | ||
Responsible Changed From-To: freebsd-bugs->billf This looks good, I'll take it. State Changed From-To: open->closed Closed per cjc's request. |
Logging of fragmented IP datagrams is logged in sys/netinet/ip_fw.c with the following code, if ((ip->ip_off & IP_OFFMASK)) snprintf(SNPARGS(fragment, 0), " Fragment = %d", ip->ip_off & IP_OFFMASK); else fragment[0] = '\0'; That is, it tests if this datagram has a non-zero offset, and if it does, it prints the offset (somewhat misleadingly labeled as "Fragment ="). There is a problem with this methodology. It misses first fragments, that is, fragments with zero offset. Fix: There are a number of issues with ipfw logging. For fragments, there is no way to tell in logs which fragments belong together, first fragments are not seen, nor are final fragments marked. Adding all of that would mean making changes that some people would not like if for no other reason than it would be a change. However, adding the functionality to log first fragments appropriately is a trivial code change and I cannot think of any reason why someone would argue against it, That is, rather than check if the fragment offset is non-zero, we just check if the fragment offset is non-zero _or_ the more-fragments bit is set. This will catch initial fragments with zero offset. This has no runtime cost and simply makes the logging a little more precise. Thanks.--SPIaQZw6nclCr0KaMhpvkLlnWUWNI1GrkswH7EHk3CZolD4K Content-Type: text/plain; name="file.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="file.diff" --- ip_fw.c.orig Sun Dec 10 12:22:54 2000 +++ ip_fw.c Sun Dec 10 12:24:43 2000 @@ -607,7 +607,7 @@ break; } - if ((ip->ip_off & IP_OFFMASK)) + if ((ip->ip_off & (IP_OFFMASK | IP_MF))) snprintf(SNPARGS(fragment, 0), " Fragment = %d", ip->ip_off & IP_OFFMASK); else How-To-Repeat: If you have a machine running ipfw with logging enabled, try this little trick, # ipfw add 10 pass log icmp from 127.0.0.1 to 127.0.0.1 in via lo0 # ifconfig lo0 mtu 1000 # ping -c 1 -s 5000 127.0.0.1 PING 127.0.0.1 (127.0.0.1): 5000 data bytes 5008 bytes from 127.0.0.1: icmp_seq=0 ttl=255 time=0.657 ms --- 127.0.0.1 ping statistics --- 1 packets transmitted, 1 packets received, 0% packet loss # ifconfig lo0 mtu 16384 # ipfw delete 10 Now notice the log entries generated, ipfw: 10 Accept ICMP:8.0 127.0.0.1 127.0.0.1 in via lo0 ipfw: 10 Accept ICMP 127.0.0.1 127.0.0.1 in via lo0 Fragment = 122 ipfw: 10 Accept ICMP 127.0.0.1 127.0.0.1 in via lo0 Fragment = 244 ... ipfw: 10 Accept ICMP 127.0.0.1 127.0.0.1 in via lo0 Fragment = 610 The first fragment is not identified as a fragment.