netdev - Re: [PATCH net] ipv6: don't let tb6_root node share routes with other node

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: <20180118224631.72rxcu72purfsmy7@kafai-mbp.dhcp.thefacebook.com>
Date:   Thu, 18 Jan 2018 14:47:22 -0800
From:   Martin KaFai Lau <kafai@...com>
To:     Wei Wang <weiwan@...gle.com>
CC:     David Miller <davem@...emloft.net>, <netdev@...r.kernel.org>,
        Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH net] ipv6: don't let tb6_root node share routes with
 other node

On Thu, Jan 18, 2018 at 10:40:03AM -0800, Wei Wang wrote:
> From: Wei Wang <weiwan@...gle.com>
> 
> After commit 4512c43eac7e, if we add a route to the subtree of tb6_root
> which does not have any route attached to it yet, the current code will
> let tb6_root and the node in the subtree share the same route.
> This could cause problem cause tb6_root has RTN_INFO flag marked and the
You meant the RTN_RTINFO check in fib6_purge_rt()?

> tree repair and clean up code will not work properly.
> This commit makes sure tb6_root->leaf points back to null_entry instead
> of sharing route with other node.
> 
> It fixes the following syzkaller reported issue:
> BUG: KASAN: use-after-free in ipv6_prefix_equal include/net/ipv6.h:540 [inline]
> BUG: KASAN: use-after-free in fib6_add_1+0x165f/0x1790 net/ipv6/ip6_fib.c:618
> Read of size 8 at addr ffff8801bc043498 by task syz-executor5/19819
> 
> CPU: 1 PID: 19819 Comm: syz-executor5 Not tainted 4.15.0-rc7+ #186
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:17 [inline]
>  dump_stack+0x194/0x257 lib/dump_stack.c:53
>  print_address_description+0x73/0x250 mm/kasan/report.c:252
>  kasan_report_error mm/kasan/report.c:351 [inline]
>  kasan_report+0x25b/0x340 mm/kasan/report.c:409
>  __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:430
>  ipv6_prefix_equal include/net/ipv6.h:540 [inline]
>  fib6_add_1+0x165f/0x1790 net/ipv6/ip6_fib.c:618
>  fib6_add+0x5fa/0x1540 net/ipv6/ip6_fib.c:1214
>  __ip6_ins_rt+0x6c/0x90 net/ipv6/route.c:1003
>  ip6_route_add+0x141/0x190 net/ipv6/route.c:2790
>  ipv6_route_ioctl+0x4db/0x6b0 net/ipv6/route.c:3299
>  inet6_ioctl+0xef/0x1e0 net/ipv6/af_inet6.c:520
>  sock_do_ioctl+0x65/0xb0 net/socket.c:958
>  sock_ioctl+0x2c2/0x440 net/socket.c:1055
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  do_vfs_ioctl+0x1b1/0x1520 fs/ioctl.c:686
>  SYSC_ioctl fs/ioctl.c:701 [inline]
>  SyS_ioctl+0x8f/0xc0 fs/ioctl.c:692
>  entry_SYSCALL_64_fastpath+0x23/0x9a
> RIP: 0033:0x452ac9
> RSP: 002b:00007fd42b321c58 EFLAGS: 00000212 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 000000000071bea0 RCX: 0000000000452ac9
> RDX: 0000000020fd7000 RSI: 000000000000890b RDI: 0000000000000013
> RBP: 000000000000049e R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000212 R12: 00000000006f4f70
> R13: 00000000ffffffff R14: 00007fd42b3226d4 R15: 0000000000000000
> 
> Fixes: 4512c43eac7e ("ipv6: remove null_entry before adding default route")
> Signed-off-by: Wei Wang <weiwan@...gle.com>
> Acked-by: Eric Dumazet <edumazet@...gle.com>
> ---
>  net/ipv6/ip6_fib.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
> index 9dcc3924a975..217683d40f12 100644
> --- a/net/ipv6/ip6_fib.c
> +++ b/net/ipv6/ip6_fib.c
> @@ -1226,8 +1226,14 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
>  		}
>  
>  		if (!rcu_access_pointer(fn->leaf)) {
> -			atomic_inc(&rt->rt6i_ref);
> -			rcu_assign_pointer(fn->leaf, rt);
> +			if (fn->fn_flags & RTN_TL_ROOT) {
> +				/* put back null_entry for root node */
> +				rcu_assign_pointer(fn->leaf,
> +					    info->nl_net->ipv6.ip6_null_entry);
> +			} else {
> +				atomic_inc(&rt->rt6i_ref);
> +				rcu_assign_pointer(fn->leaf, rt);
> +			}
>  		}
>  		fn = sn;
>  	}
> -- 
> 2.16.0.rc1.238.g530d649a79-goog
>