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
| ||
|
Date: Wed, 27 Jul 2022 17:11:14 +0800 From: "Ziyang Xuan (William)" <william.xuanziyang@...wei.com> To: David Ahern <dsahern@...nel.org>, Eric Dumazet <edumazet@...gle.com> CC: David Miller <davem@...emloft.net>, Hideaki YOSHIFUJI <yoshfuji@...ux-ipv6.org>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev <netdev@...r.kernel.org>, LKML <linux-kernel@...r.kernel.org> Subject: Re: [PATCH net v2] ipv6/addrconf: fix a null-ptr-deref bug for ip6_ptr > On 7/26/22 6:13 AM, Eric Dumazet wrote: >> On Tue, Jul 26, 2022 at 1:50 PM Ziyang Xuan >> <william.xuanziyang@...wei.com> wrote: >>> >>> Change net device's MTU to smaller than IPV6_MIN_MTU or unregister >>> device while matching route. That may trigger null-ptr-deref bug >>> for ip6_ptr probability as following. >>> >>> ========================================================= >>> BUG: KASAN: null-ptr-deref in find_match.part.0+0x70/0x134 >>> Read of size 4 at addr 0000000000000308 by task ping6/263 >>> >>> CPU: 2 PID: 263 Comm: ping6 Not tainted 5.19.0-rc7+ #14 >>> Call trace: >>> dump_backtrace+0x1a8/0x230 >>> show_stack+0x20/0x70 >>> dump_stack_lvl+0x68/0x84 >>> print_report+0xc4/0x120 >>> kasan_report+0x84/0x120 >>> __asan_load4+0x94/0xd0 >>> find_match.part.0+0x70/0x134 >>> __find_rr_leaf+0x408/0x470 >>> fib6_table_lookup+0x264/0x540 >>> ip6_pol_route+0xf4/0x260 >>> ip6_pol_route_output+0x58/0x70 >>> fib6_rule_lookup+0x1a8/0x330 >>> ip6_route_output_flags_noref+0xd8/0x1a0 >>> ip6_route_output_flags+0x58/0x160 >>> ip6_dst_lookup_tail+0x5b4/0x85c >>> ip6_dst_lookup_flow+0x98/0x120 >>> rawv6_sendmsg+0x49c/0xc70 >>> inet_sendmsg+0x68/0x94 >>> >>> Reproducer as following: >>> Firstly, prepare conditions: >>> $ip netns add ns1 >>> $ip netns add ns2 >>> $ip link add veth1 type veth peer name veth2 >>> $ip link set veth1 netns ns1 >>> $ip link set veth2 netns ns2 >>> $ip netns exec ns1 ip -6 addr add 2001:0db8:0:f101::1/64 dev veth1 >>> $ip netns exec ns2 ip -6 addr add 2001:0db8:0:f101::2/64 dev veth2 >>> $ip netns exec ns1 ifconfig veth1 up >>> $ip netns exec ns2 ifconfig veth2 up >>> $ip netns exec ns1 ip -6 route add 2000::/64 dev veth1 metric 1 >>> $ip netns exec ns2 ip -6 route add 2001::/64 dev veth2 metric 1 >>> >>> Secondly, execute the following two commands in two ssh windows >>> respectively: >>> $ip netns exec ns1 sh >>> $while true; do ip -6 addr add 2001:0db8:0:f101::1/64 dev veth1; ip -6 route add 2000::/64 dev veth1 metric 1; ping6 2000::2; done >>> >>> $ip netns exec ns1 sh >>> $while true; do ip link set veth1 mtu 1000; ip link set veth1 mtu 1500; sleep 5; done >>> >>> It is because ip6_ptr has been assigned to NULL in addrconf_ifdown() firstly, >>> then ip6_ignore_linkdown() accesses ip6_ptr directly without NULL check. >>> >>> cpu0 cpu1 >>> fib6_table_lookup >>> __find_rr_leaf >>> addrconf_notify [ NETDEV_CHANGEMTU ] >>> addrconf_ifdown >>> RCU_INIT_POINTER(dev->ip6_ptr, NULL) >>> find_match >>> ip6_ignore_linkdown >>> >>> So we can add NULL check for ip6_ptr before using in ip6_ignore_linkdown() to >>> fix the null-ptr-deref bug. >>> >>> Fixes: 6d3d07b45c86 ("ipv6: Refactor fib6_ignore_linkdown") >> >> If we need to backport, I guess dcd1f572954f ("net/ipv6: Remove fib6_idev") >> already had the bug. > > Yes, that is the right Fixes commit. OK > >> >>> Signed-off-by: Ziyang Xuan <william.xuanziyang@...wei.com> >>> >>> --- >>> v2: >>> - Use NULL check in ip6_ignore_linkdown() but synchronize_net() in >>> addrconf_ifdown() >>> - Add timing analysis of the problem >>> >>> --- >>> include/net/addrconf.h | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/include/net/addrconf.h b/include/net/addrconf.h >>> index f7506f08e505..c04f359655b8 100644 >>> --- a/include/net/addrconf.h >>> +++ b/include/net/addrconf.h >>> @@ -405,6 +405,9 @@ static inline bool ip6_ignore_linkdown(const struct net_device *dev) >>> { >>> const struct inet6_dev *idev = __in6_dev_get(dev); >>> >>> + if (unlikely(!idev)) >>> + return true; >>> + > > Reviewed-by: David Ahern <dsahern@...nel.org> > >> >> Note that we might read a non NULL pointer here, but read it again >> later in rt6_score_route(), >> since another thread could switch the pointer under us ? >> Yes, this patch just cover the problem I'm having. I have checked the codes in kernel, there are some scenarios using __in6_dev_get() without NULL check and rtnl_lock. There is a possibility of null-ptr-deref bug. I will give a patch to fix them later. > > for silly MTU games yes, that could happen. > . >
Powered by blists - more mailing lists