Re: [PATCH net] ipv6: use fib6_info_hold_safe() when necessary

[David Ahern <dsahern@gmail.com>]

On 7/21/18 9:56 PM, Wei Wang wrote:
> From: Wei Wang <weiwan@google.com>
> 
> In the code path where only rcu read lock is held, e.g. in the route
> lookup code path, it is not safe to directly call fib6_info_hold()
> because the fib6_info may already have been deleted but still exists
> in the rcu grace period. Holding reference to it could cause double
> free and crash the kernel.
> 
> This patch adds a new function fib6_info_hold_safe() and replace
> fib6_info_hold() in all necessary places.
> 
> Syzbot reported 3 crash traces because of this. One of them is:
> 8021q: adding VLAN 0 to HW filter on device team0
> IPv6: ADDRCONF(NETDEV_CHANGE): team0: link becomes ready
> dst_release: dst:(____ptrval____) refcnt:-1
> dst_release: dst:(____ptrval____) refcnt:-2
> WARNING: CPU: 1 PID: 4845 at include/net/dst.h:239 dst_hold include/net/dst.h:239 [inline]
> WARNING: CPU: 1 PID: 4845 at include/net/dst.h:239 ip6_setup_cork+0xd66/0x1830 net/ipv6/ip6_output.c:1204
> dst_release: dst:(____ptrval____) refcnt:-1
> Kernel panic - not syncing: panic_on_warn set ...
> 

...

> 
> Fixes: 93531c674315 (net/ipv6: separate handling of FIB entries from dst based routes)
> Reported-by: syzbot+902e2a1bcd4f7808cef5@syzkaller.appspotmail.com
> Reported-by: syzbot+8ae62d67f647abeeceb9@syzkaller.appspotmail.com
> Reported-by: syzbot+3f08feb14086930677d0@syzkaller.appspotmail.com
> Signed-off-by: Wei Wang <weiwan@google.com>
> Acked-by: Eric Dumazet <edumazet@google.com>
> ---
>  include/net/ip6_fib.h |  5 +++++
>  net/ipv6/addrconf.c   |  3 ++-
>  net/ipv6/route.c      | 41 +++++++++++++++++++++++++++++++----------
>  3 files changed, 38 insertions(+), 11 deletions(-)

Reviewed-by: David Ahern <dsahern@gmail.com>

Thanks for fixing.