| Message ID | 1480046337-9521-1-git-send-email-fgao@ikuai8.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
On Fri, Nov 25, 2016 at 11:58 AM, <fgao@ikuai8.com> wrote: > From: Gao Feng <fgao@ikuai8.com> > > I lost one test case in the commit for xt_multiport. > For example, the rule is "-m multiport --dports 22,80,443". > When first port is unmatched and the second is matched, the curent codes > could not return the right result. > It would return false directly when the first port is unmatched. > > Fixes: dd2602d00f80 ("netfilter: xt_multiport: Use switch case instead > of multiple condition checks") > > Signed-off-by: Gao Feng <fgao@ikuai8.com> > --- > net/netfilter/xt_multiport.c | 28 +++++++++++++++++++--------- > 1 file changed, 19 insertions(+), 9 deletions(-) > > diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c > index ec06fb1..a99da44 100644 > --- a/net/netfilter/xt_multiport.c > +++ b/net/netfilter/xt_multiport.c > @@ -41,29 +41,39 @@ > /* range port matching */ > e = minfo->ports[++i]; > pr_debug("src or dst matches with %d-%d?\n", s, e); > - > switch (minfo->flags) { > case XT_MULTIPORT_SOURCE: > - return (src >= s && src <= e) ^ minfo->invert; > + if (src >= s && src <= e) > + return true ^ minfo->invert; > + break; > case XT_MULTIPORT_DESTINATION: > - return (dst >= s && dst <= e) ^ minfo->invert; > + if (dst >= s && dst <= e) > + return true ^ minfo->invert; > + break; > case XT_MULTIPORT_EITHER: > - return ((dst >= s && dst <= e) || > - (src >= s && src <= e)) ^ minfo->invert; > + if ((dst >= s && dst <= e) || > + (src >= s && src <= e)) > + return true ^ minfo->invert; > + break; > default: > break; > } > } else { > /* exact port matching */ > pr_debug("src or dst matches with %d?\n", s); > - > switch (minfo->flags) { > case XT_MULTIPORT_SOURCE: > - return (src == s) ^ minfo->invert; > + if (src == s) > + return true ^ minfo->invert; > + break; > case XT_MULTIPORT_DESTINATION: > - return (dst == s) ^ minfo->invert; > + if (dst == s) > + return true ^ minfo->invert; > + break; > case XT_MULTIPORT_EITHER: > - return (src == s || dst == s) ^ minfo->invert; > + if (src == s || dst == s) > + return true ^ minfo->invert; > + break; > default: > break; > } > -- > 1.9.1 > > Sorry, please ignore this commit. I write wrong title. It should be for "nf-next", not "net-next". And this patch removed two space lines. Regards Feng -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2016-11-25 at 11:58 +0800, fgao@ikuai8.com wrote: > From: Gao Feng <fgao@ikuai8.com> > > I lost one test case in the commit for xt_multiport. > For example, the rule is "-m multiport --dports 22,80,443". > When first port is unmatched and the second is matched, the curent codes > could not return the right result. > It would return false directly when the first port is unmatched. > > Fixes: dd2602d00f80 ("netfilter: xt_multiport: Use switch case instead > of multiple condition checks") > > Signed-off-by: Gao Feng <fgao@ikuai8.com> Note that the Fixes: tag should immediately precede your 'Signed-off-by:' No empty new line between the two tags. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 25, 2016 at 12:11 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Fri, 2016-11-25 at 11:58 +0800, fgao@ikuai8.com wrote: >> From: Gao Feng <fgao@ikuai8.com> >> >> I lost one test case in the commit for xt_multiport. >> For example, the rule is "-m multiport --dports 22,80,443". >> When first port is unmatched and the second is matched, the curent codes >> could not return the right result. >> It would return false directly when the first port is unmatched. >> >> Fixes: dd2602d00f80 ("netfilter: xt_multiport: Use switch case instead >> of multiple condition checks") >> >> Signed-off-by: Gao Feng <fgao@ikuai8.com> > > Note that the Fixes: tag should immediately precede your > 'Signed-off-by:' > > No empty new line between the two tags. OK, I get it and will send another update. Regards Feng > > Thanks. > > -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/xt_multiport.c b/net/netfilter/xt_multiport.c index ec06fb1..a99da44 100644 --- a/net/netfilter/xt_multiport.c +++ b/net/netfilter/xt_multiport.c @@ -41,29 +41,39 @@ /* range port matching */ e = minfo->ports[++i]; pr_debug("src or dst matches with %d-%d?\n", s, e); - switch (minfo->flags) { case XT_MULTIPORT_SOURCE: - return (src >= s && src <= e) ^ minfo->invert; + if (src >= s && src <= e) + return true ^ minfo->invert; + break; case XT_MULTIPORT_DESTINATION: - return (dst >= s && dst <= e) ^ minfo->invert; + if (dst >= s && dst <= e) + return true ^ minfo->invert; + break; case XT_MULTIPORT_EITHER: - return ((dst >= s && dst <= e) || - (src >= s && src <= e)) ^ minfo->invert; + if ((dst >= s && dst <= e) || + (src >= s && src <= e)) + return true ^ minfo->invert; + break; default: break; } } else { /* exact port matching */ pr_debug("src or dst matches with %d?\n", s); - switch (minfo->flags) { case XT_MULTIPORT_SOURCE: - return (src == s) ^ minfo->invert; + if (src == s) + return true ^ minfo->invert; + break; case XT_MULTIPORT_DESTINATION: - return (dst == s) ^ minfo->invert; + if (dst == s) + return true ^ minfo->invert; + break; case XT_MULTIPORT_EITHER: - return (src == s || dst == s) ^ minfo->invert; + if (src == s || dst == s) + return true ^ minfo->invert; + break; default: break; }