linux-kernel - Re: [PATCH] fs: ocfs2: dlm: Fix a sleep-in-atomic-context bug in dlm_print_one

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20181002154347.01e026b4e644d6b7efc76180@linux-foundation.org>
Date:   Tue, 2 Oct 2018 15:43:47 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Jia-Ju Bai <baijiaju1990@...il.com>
Cc:     mark@...heh.com, jlbec@...lplan.org, ocfs2-devel@....oracle.com,
        linux-kernel@...r.kernel.org
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 <baijiaju1990@...il.com> 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!)