final words on udp/ICMP dest unreach issue [+PATCH]
Subsystems (2)
hi!
concerning the bug discussed in the "missing icmp errors for udp packets"-thread on netdev a solution has been found.
here comes the bug (see net/ipv4/icmp.c):
#define XRLIM_BURST_FACTOR 6
int xrlim_allow(struct dst_entry *dst, int timeout) {
unsigned long now;
now = jiffies;
dst->rate_tokens += now - dst->rate_last;
dst->rate_last = now;
#1: if (dst->rate_tokens > XRLIM_BURST_FACTOR*timeout)
#2: dst->rate_tokens = XRLIM_BURST_FACTOR*timeout;
#3: if (dst->rate_tokens >= timeout) {
dst->rate_tokens -= timeout;
return 1;
} return 0;
}
for timeout=0 rate_tokens will be reset to 0 tokens (#2), since #1 always holds.
(icmp ping does have timeout=0, for instance)
this doesn't cause the packet to be filtered, since in #2 holds, but will cause the following packet to be filtered, if sent before (now - dst->rate_last) < timeout.
(note: timeout is not 0 in this inequation, since it's the timeout of the icmp type of the following packet)
a patch is attached.
thanks to all contributors, especially pekka savola, for discovering that pinging before udp scanning will cause the troubles, and alexey for suppling an alternative patch (for those intrested:
http://therapy.endorphin.org/alexey.patch)
alan, please take care of that.
greets, clemens
net/ipv4/icmp.c renamed from net/ipv4/icmp.c~ +6 -5
@@ -16,6 +16,9 @@* Other than that this module is a complete rewrite. * * Fixes: +* Clemens Fruhwirth : Fix incorrect limiting in xrlim_allow +* for a packet succedding a packet with +* timeout==0. * Mike Shaver : RFC1122 checks. * Alan Cox : Multicast ping reply as self. * Alan Cox : Fix atomicity lockup in ip_build_xmit
@@ -223,11 +226,6 @@* Note that the same dst_entry fields are modified by functions in * route.c too, but these work for packet destinations while xrlim_allow * works for icmp destinations. This means the rate limiting information -* for one "ip object" is shared. -* -* Note that the same dst_entry fields are modified by functions in -* route.c too, but these work for packet destinations while xrlim_allow -* works for icmp destinations. This means the rate limiting information * for one "ip object" is shared - and these ICMPs are twice limited: * by source and by destination. *
@@ -241,9 +239,12 @@ {unsigned long now; +if (0 == timeout)return 1 ; + now = jiffies; dst->rate_tokens += now - dst->rate_last; dst->rate_last = now; +if (dst->rate_tokens >XRLIM_BURST_FACTOR *timeout) dst->rate_tokens =XRLIM_BURST_FACTOR *timeout;if (dst->rate_tokens >= timeout) {