public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Valentijn Sessink <valentyn@blub•net>
To: netdev@vger•kernel.org
Subject: Re: [PATCH net 2/2] ipv6: ip6_dst_check needs to check for expired dst_entries
Date: Thu, 24 Oct 2013 09:49:07 +0200	[thread overview]
Message-ID: <5268D0F3.3080904@blub.net> (raw)
In-Reply-To: <20131024054824.GA15744@order.stressinduktion.org>

On 24-10-13 07:48, Hannes Frederic Sowa wrote:
> On receiving a packet too big icmp error we check if our current cached
> dst_entry in the socket is still valid. This validation check did not
> care about the expiration of the (cached) route.
> 
> The error path I traced down:
> The socket receives a packet too big mtu notification. It still has a
> valid dst_entry and thus issues the ip6_rt_pmtu_update on this dst_entry,
> setting RTF_EXPIRE and updates the dst.expiration value (which could
> fail because of not up-to-date expiration values, see previous patch).
> 
> In some seldom cases we race with a) the ip6_fib gc or b) another routing
> lookup which would result in a recreation of the cached rt6_info from its
> parent non-cached rt6_info. While copying the rt6_info we reinitialize the
> metrics store by copying it over from the parent thus invalidating the
> just installed pmtu update (both dsts use the same key to the inetpeer
> storage). The dst_entry with the just invalidated metrics data would
> just get its RTF_EXPIRES flag cleared and would continue to stay valid
> for the socket.
> 
> We should have not issued the pmtu update on the already expired dst_entry
> in the first placed. By checking the expiration on the dst entry and
> doing a relookup in case it is out of date we close the race because
> we would install a new rt6_info into the fib before we issue the pmtu
> update, thus closing this race.
> 
> Not reliably updating the dst.expire value was fixed by the patch "ipv6:
> reset dst.expires value when clearing expire flag".
> 
> Reported-by: Steinar H. Gunderson <sgunderson@bigfoot•com>
> Reported-by: Valentijn Sessink <valentyn@blub•net>
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6•org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion•org>
> ---
> I would propose this patch for -stable.
> 
>  net/ipv6/route.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index f54e3a1..04e17b3 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1087,10 +1087,13 @@ static struct dst_entry *ip6_dst_check(struct dst_entry *dst, u32 cookie)
>  	if (rt->rt6i_genid != rt_genid_ipv6(dev_net(rt->dst.dev)))
>  		return NULL;
>  
> -	if (rt->rt6i_node && (rt->rt6i_node->fn_sernum == cookie))
> -		return dst;
> +	if (!rt->rt6i_node || (rt->rt6i_node->fn_sernum != cookie))
> +		return NULL;
>  
> -	return NULL;
> +	if (rt6_check_expired(rt))
> +		return NULL;
> +
> +	return dst;
>  }
>  
>  static struct dst_entry *ip6_negative_advice(struct dst_entry *dst)

Tested-by: Valentijn Sessink <valentyn@blub•net>

  parent reply	other threads:[~2013-10-24  7:58 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-24  5:48 [PATCH net 2/2] ipv6: ip6_dst_check needs to check for expired dst_entries Hannes Frederic Sowa
2013-10-24  6:40 ` Eric Dumazet
2013-10-24  7:49 ` Valentijn Sessink [this message]
2013-10-25 23:27 ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5268D0F3.3080904@blub.net \
    --to=valentyn@blub$(echo .)net \
    --cc=netdev@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox