netdev - Re: [PATCH] net: implement emergency route cache rebulds when gc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 30 Sep 2008 18:49:22 +0400
From:	"Denis V. Lunev" <den@...allels.com>
To:	Neil Horman <nhorman@...driver.com>
Cc:	netdev@...r.kernel.org, kuznet@....inr.ac.ru, davem@...emloft.net,
	pekkas@...core.fi, jmorris@...ei.org, yoshfuji@...ux-ipv6.org,
	kaber@...sh.net
Subject: Re: [PATCH] net: implement emergency route cache rebulds when
	gc_elasticity is exceeded

On Tue, 2008-09-30 at 10:35 -0400, Neil Horman wrote:
> On Tue, Sep 30, 2008 at 06:17:12PM +0400, Denis V. Lunev wrote:
> > The patch is wrong in respect to
> >   for_each_net
> > usage.
> > 
> > It should be used under rtnl. It is not held at the moment :(
> > 
> Thank you, I'll be sure to take that into consideration when we determine what
> the proper algorithm to implement here is.
> 
> Although, I used the rt_secret_rebuild timer function as my guide here, and it
> doesn't hold the rtnl lock either, or is that because its running in a softirq
> context?
no. There is no lock as there is no iteration over net namespace list.
Each namespace has its own timer.

The simplest approach I see right now is to re-iterate over current dst
cache chain and mark only those namespaces. This is at least fair as
only one namespace can be attacked (not entire node).

All locking for this is in place.

Regards,
	Den

> Neil
> 
> > Regards,
> > 	Den
> > 
> > On Mon, 2008-09-29 at 15:12 -0400, Neil Horman wrote:
> > > Hey all-
> > > 	We currently have the ability to disable our route cache secret interval
> > > rebuild timer (by setting it to zero), but if we do that its possible for an
> > > attacker (if they guess our route cache hash secret, to fill our system with
> > > routes that all hash to the same bucket, destroying our performance.  This patch
> > > provides a backstop for that issues.  In the event that our rebuild interval is
> > > disabled (or very large), if any hash chain exceeds ip_rt_gc_elasticity, we do
> > > an emergency hash rebuild.  During the hash rebuild we:
> > > 1) warn the user of the emergency
> > > 2) disable the rebuild timer
> > > 3) invalidate the route caches
> > > 4) re-enable the rebuild timer with its old value
> > > 
> > > Regards
> > > Neil
> > > 
> > > Signed-off-by: Neil Horman <nhorman@...driver.com>
> > > 
> > > 
> > >  route.c |   36 +++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 35 insertions(+), 1 deletion(-)
> > > 
> > > 
> > > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > > index 6ee5354..b95e02a 100644
> > > --- a/net/ipv4/route.c
> > > +++ b/net/ipv4/route.c
> > > @@ -145,7 +145,7 @@ static struct dst_entry *ipv4_negative_advice(struct dst_entry *dst);
> > >  static void		 ipv4_link_failure(struct sk_buff *skb);
> > >  static void		 ip_rt_update_pmtu(struct dst_entry *dst, u32 mtu);
> > >  static int rt_garbage_collect(struct dst_ops *ops);
> > > -
> > > +static void rt_emergency_hash_rebuild(void);
> > >  
> > >  static struct dst_ops ipv4_dst_ops = {
> > >  	.family =		AF_INET,
> > > @@ -1056,6 +1056,15 @@ restart:
> > >  			*candp = cand->u.dst.rt_next;
> > >  			rt_free(cand);
> > >  		}
> > > +	} else if (chain_length > ip_rt_gc_elasticity) {
> > > +		/*
> > > +		 * We didn't find a route entry we could safely free,
> > > +		 * yet our chain length is over our elasticity value.
> > > +		 * Someone may have guessed our hash secret and is artifically
> > > +		 * filling up our route cache.  Lets do an emergency rebuild
> > > +		 * to be safe
> > > +		 */
> > > +		rt_emergency_hash_rebuild();
> > >  	}
> > >  
> > >  	/* Try to bind route to arp only if it is output
> > > @@ -2946,6 +2955,31 @@ static void rt_secret_reschedule(int old)
> > >  	rtnl_unlock();
> > >  }
> > >  
> > > +static void rt_secret_rebuild_oneshot(void) {
> > > +	struct net *net;
> > > +	int old_interval = ip_rt_secret_interval;
> > > +
> > > +	ip_rt_secret_interval = 0;
> > > +
> > > +	rt_secret_reschedule(old_interval);
> > > +
> > > +	for_each_net(net) 
> > > +		rt_cache_invalidate(net);
> > > +
> > > +	ip_rt_secret_interval = old_interval;
> > > +
> > > +	rt_secret_reschedule(0);
> > > +}
> > > +
> > > +static void rt_emergency_hash_rebuild(void) {
> > > +	if (net_ratelimit()) {
> > > +		printk(KERN_WARNING "Route hash chain too long!\n");
> > > +		printk(KERN_WARNING "Adjust your secret_interval!\n");
> > > +	}
> > > +
> > > +	rt_secret_rebuild_oneshot();
> > > +}
> > > +
> > >  static int ipv4_sysctl_rt_secret_interval(ctl_table *ctl, int write,
> > >  					  struct file *filp,
> > >  					  void __user *buffer, size_t *lenp,
> > 
> > 
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html