Re: xt_recent fails with kernel 3.19.0

[Florian Westphal <fw@strlen.de>]

Chris Vine <chris@cvine.freeserve.co.uk> wrote:
> On Thu, 12 Feb 2015 12:52:02 +0100
> Florian Westphal <fw@strlen.de> wrote:
> > Florian Westphal <fw@strlen.de> wrote:
> > > I'll see if we can fix this in a better way.
> > 
> > What about this, it will transparently grow the table as needed,
> > we simply have to take the lock and make sure we zap all existing
> > entries (needed since those entries don't have enough room for
> > the larger nstamp_mask entry count)?
> > 
> > diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
> > --- a/net/netfilter/xt_recent.c
> > +++ b/net/netfilter/xt_recent.c
> > @@ -378,12 +378,11 @@ static int recent_mt_check(const struct
> > xt_mtchk_param *par, mutex_lock(&recent_mutex);
> >  	t = recent_table_lookup(recent_net, info->name);
> >  	if (t != NULL) {
> > -		if (info->hit_count > t->nstamps_max_mask) {
> > -			pr_info("hitcount (%u) is larger than packets to be remembered (%u) for table %s\n",
> > -				info->hit_count, t->nstamps_max_mask + 1,
> > -				info->name);
> > -			ret = -EINVAL;
> > -			goto out;
> > +		if (nstamp_mask > t->nstamps_max_mask) {
> > +			spin_lock_bh(&recent_lock);
> > +			recent_table_flush(t);
> > +			t->nstamps_max_mask = nstamp_mask;
> > +			spin_unlock_bh(&recent_lock);
> >  		}
> >  
> >  		t->refcnt++;
> 
> I don't know your code but forgive me for asking one thing.  The
> previous versions of this code (both in the 3.18 and 3.19 kernels)
> checked the value of hit_count for sanity.

nstamp_mask is computed based on hitcount.

> This patch seems to be doing
> something different, and I note that nstamps_max_mask is
> unconditionally set later in recent_mt_check() anyway.

No, its only set if recent_table_lookup returns NULL.
We return soon after we bump the refcnt when we take this branch.

> Can the check for the value of hit_count simply be omitted?  In what
> circumstances can it be anything other than true?

You mean when nstamp_mask > t->nstamps_max_mask is false?

e.g.
iptables -A foo -m recent --hitcount 5
iptables -A foo -m recent --hitcount 4

(2nd rule finds existing table with mask 7).