[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1502725333.4936.18.camel@edumazet-glaptop3.roam.corp.google.com>
Date: Mon, 14 Aug 2017 08:42:13 -0700
From: Eric Dumazet <eric.dumazet@...il.com>
To: Florian Westphal <fw@...len.de>
Cc: netdev@...r.kernel.org, Roopa Prabhu <roopa@...ulusnetworks.com>,
David Ahern <dsahern@...il.com>
Subject: Re: [PATCH net] ipv4: route: fix inet_rtm_getroute induced crash
On Mon, 2017-08-14 at 00:52 +0200, Florian Westphal wrote:
> "ip route get $daddr iif eth0 from $saddr" causes:
> BUG: KASAN: use-after-free in ip_route_input_rcu+0x1535/0x1b50
> Call Trace:
> ip_route_input_rcu+0x1535/0x1b50
> ip_route_input_noref+0xf9/0x190
> tcp_v4_early_demux+0x1a4/0x2b0
> ip_rcv+0xbcb/0xc05
> __netif_receive_skb+0x9c/0xd0
> netif_receive_skb_internal+0x5a8/0x890
>
> Problem is that inet_rtm_getroute calls either ip_route_input_rcu (if an
> iif was provided) or ip_route_output_key_hash_rcu.
>
> But ip_route_input_rcu, unlike ip_route_output_key_hash_rcu, already
> associates the dst_entry with the skb. This clears the SKB_DST_NOREF
> bit (i.e. skb_dst_drop will release/free the entry while it should not).
>
> Thus only set the dst if we called ip_route_output_key_hash_rcu().
>
> I tested this patch by running:
> while true;do ip r get 10.0.1.2;done > /dev/null &
> while true;do ip r get 10.0.1.2 iif eth0 from 10.0.1.1;done > /dev/null &
> ... and saw no crash or memory leak.
>
> Cc: Roopa Prabhu <roopa@...ulusnetworks.com>
> Cc: David Ahern <dsahern@...il.com>
> Fixes: ba52d61e0ff ("ipv4: route: restore skb_dst_set in inet_rtm_getroute")
> Signed-off-by: Florian Westphal <fw@...len.de>
> ---
Ouch
Acked-by: Eric Dumazet <edumazet@...gle.com>
Powered by blists - more mailing lists