From: Wes Campaigne <westacular@gmail.com>
To: netfilter-devel@vger.kernel.org
Subject: [PATCH 3/4] xtables: fix the broken detection/removal of redundant addresses
Date: Mon, 21 Feb 2011 19:10:12 -0500 [thread overview]
Message-ID: <1298333413-14253-4-git-send-email-westacular@gmail.com> (raw)
In-Reply-To: <1298333413-14253-1-git-send-email-westacular@gmail.com>
This same block of code, apparently to detect if addresses are
identical after applying the mask, and to skip the duplicates and
the ones made redundant by the mask, has been present and unchanged
from as far back as I could find (circa iptables 1.2).
By inspection, it was wrong, and always has been: once the code
finds a duplicate, it will drop the rest of the array one by one as
it re-detects the same duplicate over and over. When the addresses
came from a single hostname lookup, and their order was random,
then this created unpredictable behaviour by iptables, which seem
to ignore some of those addresses at random times.
I suspect the original idea also involved a swap between the
duplicate and the address from the (current) end of the array, but
a line of code to do that seems to have never existed. I've finally
added it. (Well, as much as is needed: there doesn't need to be a
full swap, because we're just going to ignore the duplicate,
pretend the array is one shorter, and never look at the contents of
the end again. So, we can get away with just copying from the end.)
Signed-off-by: Wes Campaigne <westacular@gmail.com>
---
xtables.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/xtables.c b/xtables.c
index 9a51e81..c96efa0 100644
--- a/xtables.c
+++ b/xtables.c
@@ -1306,13 +1306,14 @@ void xtables_ipparse_any(const char *name, struct in_addr **addrpp,
strcpy(buf, "0.0.0.0");
addrp = *addrpp = ipparse_hostnetwork(buf, naddrs);
+ /* Shuffle from end of array to remove redundant addresses */
n = *naddrs;
for (i = 0, j = 0; i < n; ++i) {
addrp[j++].s_addr &= maskp->s_addr;
for (k = 0; k < j - 1; ++k)
if (addrp[k].s_addr == addrp[j-1].s_addr) {
- --*naddrs;
- --j;
+ memcpy(&addrp[--j], &addrp[--*naddrs],
+ sizeof(struct in_addr));
break;
}
}
@@ -1608,6 +1609,7 @@ void xtables_ip6parse_any(const char *name, struct in6_addr **addrpp,
strcpy(buf, "::");
addrp = *addrpp = ip6parse_hostnetwork(buf, naddrs);
+ /* Shuffle from end of array to remove redundant addresses */
n = *naddrs;
for (i = 0, j = 0; i < n; ++i) {
for (k = 0; k < 4; ++k)
@@ -1615,8 +1617,8 @@ void xtables_ip6parse_any(const char *name, struct in6_addr **addrpp,
++j;
for (k = 0; k < j - 1; ++k)
if (IN6_ARE_ADDR_EQUAL(&addrp[k], &addrp[j - 1])) {
- --*naddrs;
- --j;
+ memcpy(&addrp[--j], &addrp[--*naddrs],
+ sizeof(struct in_addr));
break;
}
}
--
1.7.1
next prev parent reply other threads:[~2011-02-22 0:10 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-02-22 0:10 xtables: various fixes for handling multiple src/dst addresses Wes Campaigne
2011-02-22 0:10 ` [PATCH 1/4] xtables: use *all* IPv6 addresses resolved from a hostname Wes Campaigne
2011-02-22 0:10 ` [PATCH 2/4] xtables: fix excessive memory allocation in host_to_ipaddr Wes Campaigne
2011-02-22 0:10 ` Wes Campaigne [this message]
2011-02-22 0:10 ` [PATCH 4/4] xtables: use the correct loop count when applying masks to addresses Wes Campaigne
2011-02-22 3:48 ` xtables: various fixes for handling multiple src/dst addresses Jan Engelhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1298333413-14253-4-git-send-email-westacular@gmail.com \
--to=westacular@gmail.com \
--cc=netfilter-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).