lkml.org 
[lkml]   [2010]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [Stable-review] [104/145] netfilter: xt_recent: fix false match
On 03/12/2010 11:24 PM, Willy Tarreau wrote:
> 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
>

Willy - I agree with you that changing variables in an if() clause can
be dangerous. I did consider the possibility for side effects in this
case, but decided to go with the simplest patch since 'hits' is local to
the scope of the the surrounding else if() clause and is used in no
other place.

rtg
--
Tim Gardner tim.gardner@canonical.com


\
 
 \ /
  Last update: 2010-03-13 16:17    [W:0.985 / U:0.084 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site