Re: [nf-next PATCH 2/2] netfilter: xt_recent: Largely lift restrictions on max hitcount value

[Phil Sutter <phil@nwl.cc>]

On Thu, Jun 13, 2024 at 04:41:05PM +0200, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > Support tracking of up to 2^32-1 packets per table. Since users provide
> > the hitcount value in a __u32 variable, they can't exceed the max value
> > anymore.
> > 
> > Requested-by: Fabio <pedretti.fabio@gmail.com>
> > Link: https://bugzilla.netfilter.org/show_bug.cgi?id=1745
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  net/netfilter/xt_recent.c | 15 +++++----------
> >  1 file changed, 5 insertions(+), 10 deletions(-)
> > 
> > diff --git a/net/netfilter/xt_recent.c b/net/netfilter/xt_recent.c
> > index 60259280b2d5..77ac4964e2dc 100644
> > --- a/net/netfilter/xt_recent.c
> > +++ b/net/netfilter/xt_recent.c
> > @@ -59,9 +59,9 @@ MODULE_PARM_DESC(ip_list_gid, "default owning group of /proc/net/xt_recent/* fil
> >  /* retained for backwards compatibility */
> >  static unsigned int ip_pkt_list_tot __read_mostly;
> >  module_param(ip_pkt_list_tot, uint, 0400);
> > -MODULE_PARM_DESC(ip_pkt_list_tot, "number of packets per IP address to remember (max. 255)");
> > +MODULE_PARM_DESC(ip_pkt_list_tot, "number of packets per IP address to remember (max. 2^32 - 1)");
> >  
> > -#define XT_RECENT_MAX_NSTAMPS	256
> > +#define XT_RECENT_MAX_NSTAMPS	(1ULL << 32)
> 
> Won't that allow massive mem hog?

You're right, struct recent_entry may become ~32GB in size.

> Actually I think this is already a mem hog, unbounded
> allocations from time where we had no untrusted netns :-(

With the current max of 255 stamps, entries are at max 1KB in size. Is
this bad already? Given unrestricted rule counts, there are various ways
to cause large memory allocation, no?

How about restricting MAX_NSTAMPS to 1<<16? Max entry size is 568B, a
little less insane than th 32GB I thoughtlessly proposed above. :)

Cheers, Phil