linux-kernel - Re: [PATCH V2] mm: add GFP_ATOMIC flag after local_lock

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: <20210706095639.GS3840@techsingularity.net>
Date:   Tue, 6 Jul 2021 10:56:39 +0100
From:   Mel Gorman <mgorman@...hsingularity.net>
To:     Wang Qing <wangqing@...o.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        "open list:MEMORY MANAGEMENT" <linux-mm@...ck.org>,
        open list <linux-kernel@...r.kernel.org>,
        Qiang.Zhang@...driver.com
Subject: Re: [PATCH V2] mm: add GFP_ATOMIC flag after local_lock_irqsave

On Tue, Jul 06, 2021 at 05:24:31PM +0800, Wang Qing wrote:
> prep_new_page() will allocate memory in some scenarios. 
> 
> Call Trace:
> __dump_stack lib/dump_stack.c:79 [inline]
> dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:96
> ___might_sleep.cold+0x1f1/0x237 kernel/sched/core.c:9153
> prepare_alloc_pages+0x3da/0x580 mm/page_alloc.c:5179
> __alloc_pages+0x12f/0x500 mm/page_alloc.c:5375
> alloc_pages+0x18c/0x2a0 mm/mempolicy.c:2272
> stack_depot_save+0x39d/0x4e0 lib/stackdepot.c:303
> save_stack+0x15e/0x1e0 mm/page_owner.c:120
> __set_page_owner+0x50/0x290 mm/page_owner.c:181
> prep_new_page mm/page_alloc.c:2445 [inline]
> __alloc_pages_bulk+0x8b9/0x1870 mm/page_alloc.c:5313
> 
> So we add GFP_ATOMIC and remove GFP_KERNEL flag.
> 
> Reported-and-tested-by: syzbot+b07d8440edb5f8988eea@...kaller.appspotmail.com
> Signed-off-by: Wang Qing <wangqing@...o.com>

This will pass in the wrong flags to kasan potentially and the wrong GFP
mask will be stored in page_owner->gfp_mask. If you think this is the
best approach, the flags should be set to GFP_ATOMIC at the places page
owner allocates memory (stack_depot_save?). The caveat there is that
page owner tracking may be impaired if the atomic allocations fail. That
brings us back to either disabling the bulk allocator if page owner
tracking is enabled or doing the enabling/disabling only when page owner
tracking is enabled and goto the point where pagesets.lock is taken and
PCP looked up with a comment stating that it incurs a performance
penalty that is acceptable when page owner tracking is on.

-- 
Mel Gorman
SUSE Labs