netdev - Re: [PATCH net,v2] ipv4: use new_gw for redirect neigh lookup

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 for Android: free password hash cracker in your pocket

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

Date:   Sun, 13 Nov 2016 12:25:41 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     stephen.suryaputra.lin@...il.com
Cc:     netdev@...r.kernel.org, ssurya@...e.org
Subject: Re: [PATCH net,v2] ipv4: use new_gw for redirect neigh lookup

From: Stephen Suryaputra Lin <stephen.suryaputra.lin@...il.com>
Date: Thu, 10 Nov 2016 11:16:15 -0500

> In v2.6, ip_rt_redirect() calls arp_bind_neighbour() which returns 0
> and then the state of the neigh for the new_gw is checked. If the state
> isn't valid then the redirected route is deleted. This behavior is
> maintained up to v3.5.7 by check_peer_redirect() because rt->rt_gateway
> is assigned to peer->redirect_learned.a4 before calling
> ipv4_neigh_lookup().
> 
> After commit 5943634fc559 ("ipv4: Maintain redirect and PMTU info in
> struct rtable again."), ipv4_neigh_lookup() is performed without the
> rt_gateway assigned to the new_gw. In the case when rt_gateway (old_gw)
> isn't zero, the function uses it as the key. The neigh is most likely
> valid since the old_gw is the one that sends the ICMP redirect message.
> Then the new_gw is assigned to fib_nh_exception. The problem is: the
> new_gw ARP may never gets resolved and the traffic is blackholed.
> 
> So, use the new_gw for neigh lookup.
> 
> Changes from v1:
>  - use __ipv4_neigh_lookup instead (per Eric Dumazet).
> 
> Fixes: 5943634fc559 ("ipv4: Maintain redirect and PMTU info in struct rtable again.")
> Signed-off-by: Stephen Suryaputra Lin <ssurya@...e.org>

Looks good, applied and queued up for -stable.

Thanks.