Bug 12753
| Summary: | /proc/net/xt_recent/: +IP / -IP commands broken for IPv4 | ||
|---|---|---|---|
| Product: | Networking | Reporter: | Josef Drexler (joe-lk) |
| Component: | Netfilter/Iptables | Assignee: | networking_netfilter-iptables (networking_netfilter-iptables) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | CC: | alan, hozniath |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.28.7 | Subsystem: | |
| Regression: | No | Bisected commit-id: | |
Reply-To: akpm@linux-foundation.org (switched to email. Please respond via emailed reply-to-all, not via the bugzilla web interface). On Sun, 22 Feb 2009 15:11:56 -0800 (PST) bugme-daemon@bugzilla.kernel.org wrote: > http://bugzilla.kernel.org/show_bug.cgi?id=12753 > > Summary: /proc/net/xt_recent/: +IP / -IP commands broken for IPv4 > Product: Networking > Version: 2.5 > KernelVersion: 2.6.28.7 > Platform: All > OS/Version: Linux > Tree: Mainline > Status: NEW > Severity: normal > Priority: P1 > Component: Netfilter/Iptables > AssignedTo: networking_netfilter-iptables@kernel-bugs.osdl.org > ReportedBy: joe-lk@ttdpatch.net > > > Latest working kernel version: up to at least 2.6.24 > Earliest failing kernel version: first noticed with upgrade to 2.6.28 So it's a regression? > Distribution: Debian lenny, Ubuntu 8.04 > Hardware Environment: x86 > > Problem Description: > An uninitialized buffer causes IPv4 addresses added manually (via the +IP > command to the proc interface) to never match any packets. Similarly, the -IP > command fails to remove IPv4 addresses. > > Details: > In the function recent_entry_lookup, the xt_recent module does comparisons of > the entire nf_inet_addr union value, both for IPv4 and IPv6 addresses. For > addresses initialized from actual packets the remaining 12 bytes not occupied > by the IPv4 are zeroed so this works correctly. However when setting the > nf_inet_addr addr variable in the recent_mt_proc_write function, only the > IPv4 > bytes are initialized and the remaining 12 bytes contain garbage. > > Hence addresses added in this way never match any packets, unless these > uninitialized 12 bytes happened to be zero by coincidence. Similarly, > addresses > cannot consistently be removed using the proc interface due to mismatch of > the > garbage bytes (although it will sometimes work to remove an address that was > added manually). > > Reading the /proc/net/xt_recent/ entries hides this problem because this only > uses the first 4 bytes when displaying IPv4 addresses. OK. > Steps to reproduce: > $ iptables -I INPUT -m recent --rcheck -j LOG > $ echo +169.254.156.239 > /proc/net/xt_recent/DEFAULT > $ cat /proc/net/xt_recent/DEFAULT > src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 > > [At this point no packets from 169.254.156.239 are being logged.] > > $ iptables -I INPUT -s 169.254.156.239 -m recent --set > $ cat /proc/net/xt_recent/DEFAULT > src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 > src=169.254.156.239 ttl: 255 last_seen: 126184 oldest_pkt: 4 125434, 125684, > 125934, 126184 > > [At this point, adding the address via an iptables rule, packets are being > logged correctly.] > > $ echo -169.254.156.239 > /proc/net/xt_recent/DEFAULT > $ cat /proc/net/xt_recent/DEFAULT > src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 > src=169.254.156.239 ttl: 255 last_seen: 126992 oldest_pkt: 10 125434, 125684, > 125934, 126184, 126434, 126684, 126934, 126991, 126991, 126992 > $ echo -169.254.156.239 > /proc/net/xt_recent/DEFAULT > $ cat /proc/net/xt_recent/DEFAULT > src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 > src=169.254.156.239 ttl: 255 last_seen: 126992 oldest_pkt: 10 125434, 125684, > 125934, 126184, 126434, 126684, 126934, 126991, 126991, 126992 > > [Removing the address via /proc interface failed evidently.] > > > Possible solutions: > - initialize the addr variable in recent_mt_proc_write > - compare only 4 bytes for IPv4 addresses in recent_entry_lookup > > Simplest fix: > --- linux-2.6.28.7/net/netfilter/xt_recent.c.org 2009-02-22 > 17:34:19.000000000 +0100 > +++ linux-2.6.28.7/net/netfilter/xt_recent.c 2009-02-22 17:34:21.000000000 > +0100 > @@ -544,7 +544,7 @@ > struct recent_entry *e; > char buf[sizeof("+b335:1d35:1e55:dead:c0de:1715:5afe:c0de")]; > const char *c = buf; > - union nf_inet_addr addr; > + union nf_inet_addr addr = {}; > u_int16_t family; > bool add, succ; > hm, that function does some pretty ugly things. I wonder if the same bug exists elsewhere (or might do so in the future). A more general fix would be to write a new in6_to_nf_inet_addr() and in4_to_nf_inet_addr() which correctly initialise the whole union. Andrew Morton wrote: > (switched to email. Please respond via emailed reply-to-all, not via the > bugzilla web interface). >> >> Latest working kernel version: up to at least 2.6.24 >> Earliest failing kernel version: first noticed with upgrade to 2.6.28 > > So it's a regression? Yes. The fix is on its way upstream. >> Possible solutions: >> - initialize the addr variable in recent_mt_proc_write >> - compare only 4 bytes for IPv4 addresses in recent_entry_lookup >> >> Simplest fix: >> --- linux-2.6.28.7/net/netfilter/xt_recent.c.org 2009-02-22 >> 17:34:19.000000000 +0100 >> +++ linux-2.6.28.7/net/netfilter/xt_recent.c 2009-02-22 >> 17:34:21.000000000 >> +0100 >> @@ -544,7 +544,7 @@ >> struct recent_entry *e; >> char buf[sizeof("+b335:1d35:1e55:dead:c0de:1715:5afe:c0de")]; >> const char *c = buf; >> - union nf_inet_addr addr; >> + union nf_inet_addr addr = {}; >> u_int16_t family; >> bool add, succ; >> > > hm, that function does some pretty ugly things. > > I wonder if the same bug exists elsewhere (or might do so in the > future). A more general fix would be to write a new > in6_to_nf_inet_addr() and in4_to_nf_inet_addr() which correctly > initialise the whole union. I don't think thats necessary, there are no intentions of adding more text-based interfaces. commit 325fb5b4d26038cba665dd0d8ee09555321061f0 |
Latest working kernel version: up to at least 2.6.24 Earliest failing kernel version: first noticed with upgrade to 2.6.28 Distribution: Debian lenny, Ubuntu 8.04 Hardware Environment: x86 Problem Description: An uninitialized buffer causes IPv4 addresses added manually (via the +IP command to the proc interface) to never match any packets. Similarly, the -IP command fails to remove IPv4 addresses. Details: In the function recent_entry_lookup, the xt_recent module does comparisons of the entire nf_inet_addr union value, both for IPv4 and IPv6 addresses. For addresses initialized from actual packets the remaining 12 bytes not occupied by the IPv4 are zeroed so this works correctly. However when setting the nf_inet_addr addr variable in the recent_mt_proc_write function, only the IPv4 bytes are initialized and the remaining 12 bytes contain garbage. Hence addresses added in this way never match any packets, unless these uninitialized 12 bytes happened to be zero by coincidence. Similarly, addresses cannot consistently be removed using the proc interface due to mismatch of the garbage bytes (although it will sometimes work to remove an address that was added manually). Reading the /proc/net/xt_recent/ entries hides this problem because this only uses the first 4 bytes when displaying IPv4 addresses. Steps to reproduce: $ iptables -I INPUT -m recent --rcheck -j LOG $ echo +169.254.156.239 > /proc/net/xt_recent/DEFAULT $ cat /proc/net/xt_recent/DEFAULT src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 [At this point no packets from 169.254.156.239 are being logged.] $ iptables -I INPUT -s 169.254.156.239 -m recent --set $ cat /proc/net/xt_recent/DEFAULT src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 src=169.254.156.239 ttl: 255 last_seen: 126184 oldest_pkt: 4 125434, 125684, 125934, 126184 [At this point, adding the address via an iptables rule, packets are being logged correctly.] $ echo -169.254.156.239 > /proc/net/xt_recent/DEFAULT $ cat /proc/net/xt_recent/DEFAULT src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 src=169.254.156.239 ttl: 255 last_seen: 126992 oldest_pkt: 10 125434, 125684, 125934, 126184, 126434, 126684, 126934, 126991, 126991, 126992 $ echo -169.254.156.239 > /proc/net/xt_recent/DEFAULT $ cat /proc/net/xt_recent/DEFAULT src=169.254.156.239 ttl: 0 last_seen: 119910 oldest_pkt: 1 119910 src=169.254.156.239 ttl: 255 last_seen: 126992 oldest_pkt: 10 125434, 125684, 125934, 126184, 126434, 126684, 126934, 126991, 126991, 126992 [Removing the address via /proc interface failed evidently.] Possible solutions: - initialize the addr variable in recent_mt_proc_write - compare only 4 bytes for IPv4 addresses in recent_entry_lookup Simplest fix: --- linux-2.6.28.7/net/netfilter/xt_recent.c.org 2009-02-22 17:34:19.000000000 +0100 +++ linux-2.6.28.7/net/netfilter/xt_recent.c 2009-02-22 17:34:21.000000000 +0100 @@ -544,7 +544,7 @@ struct recent_entry *e; char buf[sizeof("+b335:1d35:1e55:dead:c0de:1715:5afe:c0de")]; const char *c = buf; - union nf_inet_addr addr; + union nf_inet_addr addr = {}; u_int16_t family; bool add, succ;