2017-06-05 03:36:55

by Jia-Ju Bai

Subject: [PATCH] ubifs: Fix a sleep-in-atomic bug in ubifs_read_nnode

The driver may sleep under a spin lock, and the function call path is:
ubifs_change_lp (acquire the lock by spin_lock)
change_category
ubifs_remove_from_cat
remove_from_lpt_heap
dbg_check_heap
ubifs_lpt_lookup
ubifs_read_nnode
kzalloc(GFP_NOFS) --> may sleep

To fix it, "GFP_NOFS" is replaced with "GFP_ATOMIC".

Signed-off-by: Jia-Ju Bai <[email protected]>
---
fs/ubifs/lpt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ubifs/lpt.c b/fs/ubifs/lpt.c
index 9a51710..4247934c 100644
--- a/fs/ubifs/lpt.c
+++ b/fs/ubifs/lpt.c
@@ -1205,7 +1205,7 @@ int ubifs_read_nnode(struct ubifs_info *c, struct ubifs_nnode *parent, int iip)
lnum = c->lpt_lnum;
offs = c->lpt_offs;
}
- nnode = kzalloc(sizeof(struct ubifs_nnode), GFP_NOFS);
+ nnode = kzalloc(sizeof(struct ubifs_nnode), GFP_ATOMIC);
if (!nnode) {
err = -ENOMEM;
goto out;
--
1.7.9.5

2017-06-05 08:25:46

by Richard Weinberger

Subject: Re: [PATCH] ubifs: Fix a sleep-in-atomic bug in ubifs_read_nnode

Jia-Ju Bai,

Am 05.06.2017 um 05:38 schrieb Jia-Ju Bai:
> The driver may sleep under a spin lock, and the function call path is:
> ubifs_change_lp (acquire the lock by spin_lock)
> change_category
> ubifs_remove_from_cat
> remove_from_lpt_heap
> dbg_check_heap
> ubifs_lpt_lookup
> ubifs_read_nnode
> kzalloc(GFP_NOFS) --> may sleep
>
> To fix it, "GFP_NOFS" is replaced with "GFP_ATOMIC".

So, this happens only when dbg_check_heap() is activated, right?

Thanks,
//richard

2017-06-05 08:39:42

by Richard Weinberger

Subject: Re: [PATCH] ubifs: Fix a sleep-in-atomic bug in ubifs_read_nnode

Am 05.06.2017 um 10:25 schrieb Richard Weinberger:
> Jia-Ju Bai,
>
> Am 05.06.2017 um 05:38 schrieb Jia-Ju Bai:
>> The driver may sleep under a spin lock, and the function call path is:
>> ubifs_change_lp (acquire the lock by spin_lock)
>> change_category
>> ubifs_remove_from_cat
>> remove_from_lpt_heap
>> dbg_check_heap
>> ubifs_lpt_lookup
>> ubifs_read_nnode
>> kzalloc(GFP_NOFS) --> may sleep
>>
>> To fix it, "GFP_NOFS" is replaced with "GFP_ATOMIC".
>
> So, this happens only when dbg_check_heap() is activated, right?

Why do we call change_category() anyway with c->space_lock held?

Thanks,
//richard

2017-06-05 08:41:58

by Jia-Ju Bai

Subject: Re: [PATCH] ubifs: Fix a sleep-in-atomic bug in ubifs_read_nnode

On 06/05/2017 04:25 PM, Richard Weinberger wrote:
> Jia-Ju Bai,
>
> Am 05.06.2017 um 05:38 schrieb Jia-Ju Bai:
>> The driver may sleep under a spin lock, and the function call path is:
>> ubifs_change_lp (acquire the lock by spin_lock)
>> change_category
>> ubifs_remove_from_cat
>> remove_from_lpt_heap
>> dbg_check_heap
>> ubifs_lpt_lookup
>> ubifs_read_nnode
>> kzalloc(GFP_NOFS) --> may sleep
>>
>> To fix it, "GFP_NOFS" is replaced with "GFP_ATOMIC".
> So, this happens only when dbg_check_heap() is activated, right?
>
> Thanks,
> //richard
Yes, I think so.

Thanks,
Jia-Ju Bai