[PATCH] net: ipv6: route: Fix a sleep-in-atomic-context bug in ip6_convert_metrics()

[PATCH] net: ipv6: route: Fix a sleep-in-atomic-context bug in ip6_convert_metrics()

[Jia-Ju Bai]

The kernel module may sleep with holding a spinlock.

The function call paths (from bottom to top) in Linux-4.16 are:

[FUNC] kzalloc(GFP_KERNEL)
net/ipv6/route.c, 2430: 
	kzalloc in ip6_convert_metrics
net/ipv6/route.c, 2890: 
	ip6_convert_metrics in ip6_route_add
net/ipv6/addrconf.c, 2322: 
	ip6_route_add in addrconf_prefix_route
net/ipv6/addrconf.c, 3331: 
	addrconf_prefix_route in fixup_permanent_addr
net/ipv6/addrconf.c, 3354: 
	fixup_permanent_addr in addrconf_permanent_addr
net/ipv6/addrconf.c, 3358: 
	_raw_write_lock_bh in addrconf_permanent_addr

To fix this bug, GFP_KERNEL is replaced with GFP_ATOMIC.

This bug is found by my static analysis tool DSAC.

Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
 net/ipv6/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index ec18b3ce8b6d..d15e72def7c1 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2742,7 +2742,7 @@ static int ip6_convert_metrics(struct net *net, struct fib6_info *rt,
 	if (!cfg->fc_mx)
 		return 0;
 
-	p = kzalloc(sizeof(*rt->fib6_metrics), GFP_KERNEL);
+	p = kzalloc(sizeof(*rt->fib6_metrics), GFP_ATOMIC);
 	if (unlikely(!p))
 		return -ENOMEM;
 
-- 
2.17.0

Re: [PATCH] net: ipv6: route: Fix a sleep-in-atomic-context bug in ip6_convert_metrics()

[David Ahern]

On 9/1/18 5:19 AM, Jia-Ju Bai wrote:
> The kernel module may sleep with holding a spinlock.
...

> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index ec18b3ce8b6d..d15e72def7c1 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -2742,7 +2742,7 @@ static int ip6_convert_metrics(struct net *net, struct fib6_info *rt,
>  	if (!cfg->fc_mx)
>  		return 0;
>  
> -	p = kzalloc(sizeof(*rt->fib6_metrics), GFP_KERNEL);
> +	p = kzalloc(sizeof(*rt->fib6_metrics), GFP_ATOMIC);
>  	if (unlikely(!p))
>  		return -ENOMEM;
>  
> 

This is the wrong solution. I'll take care of it next week after the
holiday weekend.

Re: [PATCH] net: ipv6: route: Fix a sleep-in-atomic-context bug in ip6_convert_metrics()

[David Ahern]

On 9/1/18 5:19 AM, Jia-Ju Bai wrote:
> The kernel module may sleep with holding a spinlock.
> 
> The function call paths (from bottom to top) in Linux-4.16 are:
> 
> [FUNC] kzalloc(GFP_KERNEL)
> net/ipv6/route.c, 2430: 
> 	kzalloc in ip6_convert_metrics
> net/ipv6/route.c, 2890: 
> 	ip6_convert_metrics in ip6_route_add
> net/ipv6/addrconf.c, 2322: 
> 	ip6_route_add in addrconf_prefix_route
> net/ipv6/addrconf.c, 3331: 
> 	addrconf_prefix_route in fixup_permanent_addr
> net/ipv6/addrconf.c, 3354: 
> 	fixup_permanent_addr in addrconf_permanent_addr
> net/ipv6/addrconf.c, 3358: 
> 	_raw_write_lock_bh in addrconf_permanent_addr
> 
> To fix this bug, GFP_KERNEL is replaced with GFP_ATOMIC.
> 
> This bug is found by my static analysis tool DSAC.

No kernel change is needed. Your static analysis tool and you in sending
out patches need to take into context.

ip6_convert_metrics only calls kzalloc when fc_mx is set. fc_mx is only
set via the RTA_METRICS attribute and only from the userspace call path.
Hence, kzalloc with GFP_KERNEL is the appropriate argument.

Re: [PATCH] net: ipv6: route: Fix a sleep-in-atomic-context bug in ip6_convert_metrics()

[Jia-Ju Bai]

On 2018/9/4 10:40, David Ahern wrote:
> On 9/1/18 5:19 AM, Jia-Ju Bai wrote:
>> The kernel module may sleep with holding a spinlock.
>>
>> The function call paths (from bottom to top) in Linux-4.16 are:
>>
>> [FUNC] kzalloc(GFP_KERNEL)
>> net/ipv6/route.c, 2430:
>> 	kzalloc in ip6_convert_metrics
>> net/ipv6/route.c, 2890:
>> 	ip6_convert_metrics in ip6_route_add
>> net/ipv6/addrconf.c, 2322:
>> 	ip6_route_add in addrconf_prefix_route
>> net/ipv6/addrconf.c, 3331:
>> 	addrconf_prefix_route in fixup_permanent_addr
>> net/ipv6/addrconf.c, 3354:
>> 	fixup_permanent_addr in addrconf_permanent_addr
>> net/ipv6/addrconf.c, 3358:
>> 	_raw_write_lock_bh in addrconf_permanent_addr
>>
>> To fix this bug, GFP_KERNEL is replaced with GFP_ATOMIC.
>>
>> This bug is found by my static analysis tool DSAC.
> No kernel change is needed. Your static analysis tool and you in sending
> out patches need to take into context.
>
> ip6_convert_metrics only calls kzalloc when fc_mx is set. fc_mx is only
> set via the RTA_METRICS attribute and only from the userspace call path.
> Hence, kzalloc with GFP_KERNEL is the appropriate argument.

Oh, sorry for my false report.


Best wishes,
Jia-Ju Bai