* [PATCH net v2] ipv6/addrconf: fix a null-ptr-deref bug for ip6_ptr
@ 2022-07-26 11:50 Ziyang Xuan
2022-07-26 12:13 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: Ziyang Xuan @ 2022-07-26 11:50 UTC (permalink / raw)
To: davem, yoshfuji, dsahern, edumazet, kuba, pabeni, netdev; +Cc: linux-kernel
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")
Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.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;
+
return !!idev->cnf.ignore_routes_with_linkdown;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net v2] ipv6/addrconf: fix a null-ptr-deref bug for ip6_ptr
2022-07-26 11:50 [PATCH net v2] ipv6/addrconf: fix a null-ptr-deref bug for ip6_ptr Ziyang Xuan
@ 2022-07-26 12:13 ` Eric Dumazet
2022-07-26 15:03 ` David Ahern
0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2022-07-26 12:13 UTC (permalink / raw)
To: Ziyang Xuan
Cc: David Miller, Hideaki YOSHIFUJI, David Ahern, Jakub Kicinski,
Paolo Abeni, netdev, LKML
On Tue, Jul 26, 2022 at 1:50 PM Ziyang Xuan
<william.xuanziyang@huawei.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.
> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.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;
> +
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 ?
> return !!idev->cnf.ignore_routes_with_linkdown;
> }
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v2] ipv6/addrconf: fix a null-ptr-deref bug for ip6_ptr
2022-07-26 12:13 ` Eric Dumazet
@ 2022-07-26 15:03 ` David Ahern
2022-07-27 9:11 ` Ziyang Xuan (William)
0 siblings, 1 reply; 5+ messages in thread
From: David Ahern @ 2022-07-26 15:03 UTC (permalink / raw)
To: Eric Dumazet, Ziyang Xuan
Cc: David Miller, Hideaki YOSHIFUJI, Jakub Kicinski, Paolo Abeni,
netdev, LKML
On 7/26/22 6:13 AM, Eric Dumazet wrote:
> On Tue, Jul 26, 2022 at 1:50 PM Ziyang Xuan
> <william.xuanziyang@huawei.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.
>
>> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.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@kernel.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 ?
>
for silly MTU games yes, that could happen.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v2] ipv6/addrconf: fix a null-ptr-deref bug for ip6_ptr
2022-07-26 15:03 ` David Ahern
@ 2022-07-27 9:11 ` Ziyang Xuan (William)
2022-07-28 1:38 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Ziyang Xuan (William) @ 2022-07-27 9:11 UTC (permalink / raw)
To: David Ahern, Eric Dumazet
Cc: David Miller, Hideaki YOSHIFUJI, Jakub Kicinski, Paolo Abeni,
netdev, LKML
> On 7/26/22 6:13 AM, Eric Dumazet wrote:
>> On Tue, Jul 26, 2022 at 1:50 PM Ziyang Xuan
>> <william.xuanziyang@huawei.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@huawei.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@kernel.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.
> .
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net v2] ipv6/addrconf: fix a null-ptr-deref bug for ip6_ptr
2022-07-27 9:11 ` Ziyang Xuan (William)
@ 2022-07-28 1:38 ` Jakub Kicinski
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2022-07-28 1:38 UTC (permalink / raw)
To: Ziyang Xuan (William)
Cc: David Ahern, Eric Dumazet, David Miller, Hideaki YOSHIFUJI,
Paolo Abeni, netdev, LKML
On Wed, 27 Jul 2022 17:11:14 +0800 Ziyang Xuan (William) wrote:
> > Yes, that is the right Fixes commit.
>
> OK
Please repost with that changed.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-07-28 1:38 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-26 11:50 [PATCH net v2] ipv6/addrconf: fix a null-ptr-deref bug for ip6_ptr Ziyang Xuan
2022-07-26 12:13 ` Eric Dumazet
2022-07-26 15:03 ` David Ahern
2022-07-27 9:11 ` Ziyang Xuan (William)
2022-07-28 1:38 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.