On Mon, May 23, 2016 at 12:03:55AM +0900, Taehee Yoo wrote:
> 2016-05-17 19:38 GMT+09:00 Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>:
> > On Sat, May 14, 2016 at 10:19:16PM +0900, Taehee Yoo wrote:
> >> when register to helper, each helper adds port to name.
> >> correct form is 'protocol name-port' but irc, sip and tftp adds
> >> a iterator value. so it fix it.
> >
> > Could you track since when this works in this way?
> >
> > This inconsistency has been probably there since long time ago, and we
> > expose this names through iptables -m helper.
> >
> > What I mean is: I understand this is inconsistent, but if we change
> > this now, we may break existing rulesets.
>
>
> Thank you for your review.
> And Apologize for late reply.
>
> I agree that patch destroys so much rulesets.
> but I want to solve the issue that is helper cannot check duplicated
> helper rules.
> nf_conntrack_helper_register() checks name && l3num && protonum to
> check duplicated rules.
> but tftp, sip and irc helper always have unique helper name because
> that includes iterator value.
> (tftp-1, tftp-2, tftp-3 ...)
> helper-name is good method to check duplicated rules.
> but we need another check method to solve this issue and keep rulsets.
> so far, my idea is that using help callback function's pointer address.
> pseudo code is : "if (port && l3num && protonum && help)"
>
> Do you have any advice?
Probably something like this?
The idea is to compare the helper name, stripping off the '-value'
from the name so we catch if the user specific duplicated ports via
module option, which is what is causing problems to you, right?
diff --git a/net/netfilter/nf_conntrack_helper.c b/net/netfilter/nf_conntrack_helper.c
index f703adb..5785034 100644
--- a/net/netfilter/nf_conntrack_helper.c
+++ b/net/netfilter/nf_conntrack_helper.c
@@ -361,9 +361,9 @@ EXPORT_SYMBOL_GPL(nf_ct_helper_log);
int nf_conntrack_helper_register(struct nf_conntrack_helper *me)
{
- int ret = 0;
struct nf_conntrack_helper *cur;
unsigned int h = helper_hash(&me->tuple);
+ int ret = 0, len;
BUG_ON(me->expect_policy == NULL);
BUG_ON(me->expect_class_max >= NF_CT_MAX_EXPECT_CLASSES);
@@ -371,7 +371,13 @@ int nf_conntrack_helper_register(struct nf_conntrack_helper *me)
mutex_lock(&nf_ct_helper_mutex);
hlist_for_each_entry(cur, &nf_ct_helper_hash[h], hnode) {
- if (strncmp(cur->name, me->name, NF_CT_HELPER_NAME_LEN) == 0 &&
+ slash = strchr(cur->name, '-');
+ if (slash)
+ len = slash - cur->name;
+ else
+ len = NF_CT_HELPER_NAME_LEN;
+
+ if (strncmp(cur->name, me->name, len) == 0 &&
cur->tuple.src.l3num == me->tuple.src.l3num &&
cur->tuple.dst.protonum == me->tuple.dst.protonum) {
ret = -EEXIST;
