2018-09-01 11:27:16

by Jia-Ju Bai

Subject: [PATCH] fs: ocfs2: dlm: Fix a sleep-in-atomic-context bug in dlm_print_one_mle()

The kernel module may sleep with holding a spinlock.

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

[FUNC] get_zeroed_page(GFP_NOFS)
fs/ocfs2/dlm/dlmdebug.c, 332: get_zeroed_page in dlm_print_one_mle
fs/ocfs2/dlm/dlmmaster.c, 240: dlm_print_one_mle in __dlm_put_mle
fs/ocfs2/dlm/dlmmaster.c, 255: __dlm_put_mle in dlm_put_mle
fs/ocfs2/dlm/dlmmaster.c, 254: spin_lock in dlm_put_ml

[FUNC] get_zeroed_page(GFP_NOFS)
fs/ocfs2/dlm/dlmdebug.c, 332: get_zeroed_page in dlm_print_one_mle
fs/ocfs2/dlm/dlmmaster.c, 240: dlm_print_one_mle in __dlm_put_mle
fs/ocfs2/dlm/dlmmaster.c, 222: __dlm_put_mle in dlm_put_mle_inuse
fs/ocfs2/dlm/dlmmaster.c, 219: spin_lock in dlm_put_mle_inuse

To fix this bug, GFP_NOFS is replaced with GFP_ATOMIC.

This bug is found by my static analysis tool DSAC.

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

diff --git a/fs/ocfs2/dlm/dlmdebug.c b/fs/ocfs2/dlm/dlmdebug.c
index 9b984cae4c4e..1d6dc8422899 100644
--- a/fs/ocfs2/dlm/dlmdebug.c
+++ b/fs/ocfs2/dlm/dlmdebug.c
@@ -329,7 +329,7 @@ void dlm_print_one_mle(struct dlm_master_list_entry *mle)
{
char *buf;

- buf = (char *) get_zeroed_page(GFP_NOFS);
+ buf = (char *) get_zeroed_page(GFP_ATOMIC);
if (buf) {
dump_mle(mle, buf, PAGE_SIZE - 1);
free_page((unsigned long)buf);
--
2.17.0

2018-10-02 22:44:29

by Andrew Morton

Subject: Re: [PATCH] fs: ocfs2: dlm: Fix a sleep-in-atomic-context bug in dlm_print_one_mle()

On Sat, 1 Sep 2018 19:25:28 +0800 Jia-Ju Bai <[email protected]> wrote:

> The kernel module may sleep with holding a spinlock.
>
> The function call paths (from bottom to top) in Linux-4.16 are:
>
> [FUNC] get_zeroed_page(GFP_NOFS)
> fs/ocfs2/dlm/dlmdebug.c, 332: get_zeroed_page in dlm_print_one_mle
> fs/ocfs2/dlm/dlmmaster.c, 240: dlm_print_one_mle in __dlm_put_mle
> fs/ocfs2/dlm/dlmmaster.c, 255: __dlm_put_mle in dlm_put_mle
> fs/ocfs2/dlm/dlmmaster.c, 254: spin_lock in dlm_put_ml
>
> [FUNC] get_zeroed_page(GFP_NOFS)
> fs/ocfs2/dlm/dlmdebug.c, 332: get_zeroed_page in dlm_print_one_mle
> fs/ocfs2/dlm/dlmmaster.c, 240: dlm_print_one_mle in __dlm_put_mle
> fs/ocfs2/dlm/dlmmaster.c, 222: __dlm_put_mle in dlm_put_mle_inuse
> fs/ocfs2/dlm/dlmmaster.c, 219: spin_lock in dlm_put_mle_inuse
>
> To fix this bug, GFP_NOFS is replaced with GFP_ATOMIC.
>
> This bug is found by my static analysis tool DSAC.
>
> ...
>
> --- a/fs/ocfs2/dlm/dlmdebug.c
> +++ b/fs/ocfs2/dlm/dlmdebug.c
> @@ -329,7 +329,7 @@ void dlm_print_one_mle(struct dlm_master_list_entry *mle)
> {
> char *buf;
>
> - buf = (char *) get_zeroed_page(GFP_NOFS);
> + buf = (char *) get_zeroed_page(GFP_ATOMIC);
> if (buf) {
> dump_mle(mle, buf, PAGE_SIZE - 1);
> free_page((unsigned long)buf);

Fair enough. It's pretty sad code here, replying on the page allocator
in this situation. But it's only debug stuff so nobody is likely to
care much.

(And that page didn't need to be zeroed!)