From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH net] ipv6: use fib6_info_hold_safe() when necessary Date: Mon, 23 Jul 2018 08:28:42 -0600 Message-ID: <853fa714-9529-684b-78ed-a545c24a3894@gmail.com> References: <20180722035632.136090-1-tracywwnj@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: Eric Dumazet , Martin KaFai Lau To: Wei Wang , David Miller , netdev@vger.kernel.org Return-path: Received: from mail-pl0-f65.google.com ([209.85.160.65]:44362 "EHLO mail-pl0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2388556AbeGWPaQ (ORCPT ); Mon, 23 Jul 2018 11:30:16 -0400 Received: by mail-pl0-f65.google.com with SMTP id m16-v6so297061pls.11 for ; Mon, 23 Jul 2018 07:28:45 -0700 (PDT) In-Reply-To: <20180722035632.136090-1-tracywwnj@gmail.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 7/21/18 9:56 PM, Wei Wang wrote: > From: Wei Wang > > 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 > Acked-by: Eric Dumazet > --- > 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 Thanks for fixing.