lkml.org 
[lkml]   [2010]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
DateSat, 13 Mar 2010 07:24:46 +0100
FromWilly Tarreau <>
SubjectRe: [Stable-review] [104/145] netfilter: xt_recent: fix false match
On Fri, Mar 12, 2010 at 04:27:17PM -0800, Greg KH wrote:
> 2.6.32-stable review patch. If anyone has any objections, please let me know.
>
> ----------------
> From: Tim Gardner <tim.gardner@canonical.com>
>
> commit 8ccb92ad41cb311e52ad1b1fe77992c7f47a3b63 upstream.
>
> A rule with a zero hit_count will always match.
>
> Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
>
> ---
> net/netfilter/xt_recent.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/net/netfilter/xt_recent.c
> +++ b/net/netfilter/xt_recent.c
> @@ -260,7 +260,7 @@ recent_mt(const struct sk_buff *skb, con
> for (i = 0; i < e->nstamps; i++) {
> if (info->seconds && time_after(time, e->stamps[i]))
> continue;
> - if (++hits >= info->hit_count) {
> + if (info->hit_count && ++hits >= info->hit_count) {
> ret = !ret;
> break;
> }

I don't know if this has any undesired side effect or not, but the
logic is changed now since "hits" will not be increased anymore when
info->hit_count is zero. And the code does not make it obvious to me
what the intended purpose was.

For this reason I always find it dangerous to change variables in
if() conditions because it's where we change operations the most
frequently when fixing bugs.
Willy



\
 
 \ /
  Last update: 2010-03-13 07:27    [from the cache]
©2003-2010