netdev - Re: [PATCH net-next 1/3] ipv4: properly refresh rtable entries on pmtu/redirect events

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]

Message-ID: <20130528115357.4d9d281e@vostro>
Date:	Tue, 28 May 2013 11:53:57 +0300
From:	Timo Teras <timo.teras@....fi>
To:	Julian Anastasov <ja@....bg>
Cc:	netdev@...r.kernel.org,
	Steffen Klassert <steffen.klassert@...unet.com>
Subject: Re: [PATCH net-next 1/3] ipv4: properly refresh rtable entries on
 pmtu/redirect events

On Tue, 28 May 2013 11:25:55 +0300 (EEST)
Julian Anastasov <ja@....bg> wrote:

> 
> 	Hello,
> 
> On Tue, 28 May 2013, Timo Teräs wrote:
> 
> > This reverts commit 05ab86c5 (xfrm4: Invalidate all ipv4 routes on
> > IPsec pmtu events). Flushing all cached entries is not needed.
> > 
> > Instead, invalidate only the related next hop dsts to recheck for
> > the added next hop exception where needed. This also fixes a subtle
> > race due to bumping generation id's before updating the pmtu.
> > 
> > Cc: Steffen Klassert <steffen.klassert@...unet.com>
> > Signed-off-by: Timo Teräs <timo.teras@....fi>
> > ---
> 
> > diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> > index 550781a..561a378 100644
> > --- a/net/ipv4/route.c
> > +++ b/net/ipv4/route.c
> > @@ -594,11 +594,25 @@ static inline u32 fnhe_hashfun(__be32 daddr)
> >  	return hval & (FNHE_HASH_SIZE - 1);
> >  }
> >  
> > +static void fill_route_from_fnhe(struct rtable *rt, struct
> > fib_nh_exception *fnhe) +{
> > +	rt->rt_pmtu = fnhe->fnhe_pmtu;
> > +	rt->dst.expires = fnhe->fnhe_expires;
> 
> 	The 'if (time_before' ... dst_set_expires() logic from
> rt_bind_exception() is removed, may be it should be moved here,
> i.e. fnhe_pmtu should be ignored if expired.

That code would not help much. The route's rt_pmtu is never reset to
zero after the fnhe expires, so this would not make much difference.
The old rt_pmtu and dst.expires would be left there anyway. All rt
accesses check for expires too.

This was actually intentional on the old code, as non-zero rt_pmtu
implied that we had "next hop exception route" instead of "next hop
route" and affected how the rt was invalidated in pmtu update.

If we want to clear out these fields, then it would make sense to have
rt_bind_exception() to reset these on expiry to the struct
fib_nh_exception directly and keep the direct assignments in
fill_route_from_fnhe().

- Timo
--
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