netdev - Re: [PATCH net] ipv6: use fib6_info_hold

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: <853fa714-9529-684b-78ed-a545c24a3894@gmail.com>
Date:   Mon, 23 Jul 2018 08:28:42 -0600
From:   David Ahern <dsahern@...il.com>
To:     Wei Wang <weiwan@...gle.com>, David Miller <davem@...emloft.net>,
        netdev@...r.kernel.org
Cc:     Eric Dumazet <edumazet@...gle.com>, Martin KaFai Lau <kafai@...com>
Subject: Re: [PATCH net] ipv6: use fib6_info_hold_safe() when necessary

On 7/21/18 9:56 PM, Wei Wang wrote:
> From: Wei Wang <weiwan@...gle.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@...kaller.appspotmail.com
> Reported-by: syzbot+8ae62d67f647abeeceb9@...kaller.appspotmail.com
> Reported-by: syzbot+3f08feb14086930677d0@...kaller.appspotmail.com
> Signed-off-by: Wei Wang <weiwan@...gle.com>
> Acked-by: Eric Dumazet <edumazet@...gle.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@...il.com>

Thanks for fixing.