Re: [RFC] mm: Fix memory corruption caused by deferred page initialization

[Gavin Shan <gwshan@linux.vnet.ibm.com>]

On Mon, Mar 28, 2016 at 12:48:27AM +1100, Gavin Shan wrote:
>On Sun, Mar 27, 2016 at 12:37:09AM +1100, Gavin Shan wrote:
>>On Sat, Mar 26, 2016 at 08:47:17PM +1100, Michael Ellerman wrote:
>>>Hi Gavin,
>>>
>>>On Fri, 2016-25-03 at 16:05:29 UTC, Gavin Shan wrote:
>>>> During deferred page initialization, the pages are moved from memblock
>>>> or bootmem to buddy allocator without checking they were reserved. Those
>>>> reserved pages can be reallocated to somebody else by buddy/slab allocator.
>>>> It leads to memory corruption and potential kernel crash eventually.
>>>
>>>Can you give me a bit more detail on what the bug is?
>>>
>>>I haven't seen any issues on my systems, but I realise now I haven't enabled
>>>DEFERRED_STRUCT_PAGE_INIT - I assumed it was enabled by default.
>>>
>>>How did this get tested before submission?
>>>
>>
>>Michael, I have to reply with same context in another thread in case 
>>somebody else wants to understand more: Li, who is in the cc list, is
>>backporting deferred page initialization (CONFIG_DEFERRED_STRUCT_PAGE_INIT)
>>from upstream kernel to RHEL 7.2 or 7.3 kernel (3.10.0-357.el7). RHEL kernel
>>has (!CONFIG_NO_BOOTMEM && CONFIG_DEFERRED_STRUCT_PAGE_INIT), meaning
>>bootmem is enabled. She eventually runs into kernel crash and I jumped
>>in to help understanding the root cause.
>>
>>There're two related kernel config options: ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT
>>and DEFERRED_STRUCT_PAGE_INIT. The former one is enabled on PPC by default.
>>The later one isn't enabled by default.
>>
>>There are two test cases I had:
>>
>>- With (!CONFIG_NO_BOOTMEM && CONFIG_DEFERRED_STRUCT_PAGE_INIT)
>>on PowerNV platform, upstream kernel (4.5.rc7) and additional patch to support
>>bootmem as it was removed on powerpc a while ago.
>>
>>- With (CONFIG_NO_BOOTMEM && CONFIG_DEFERRED_STRUCT_PAGE_INIT) on PowerNV platform,
>>upstream kernel (4.5.rc7), I dumped the reserved memblock regions and added printk
>>in function deferred_init_memmap() to check if memblock reserved PFN 0x1fff80 (one
>>page in memblock reserved region#31, refer to the below kernel log) is released
>>to buddy allocator or not when doing deferred page struct initialization. I did
>>see that PFN is released to buddy allocator at that time. However, I didn't see
>>kernel crash and it would be luck and the current deferred page struct initialization
>>implementation: The pages in region [0, 2GB] except the memblock reserved ones are
>>presented to buddy allocator at early stage. It's not deferred. So for the pages in
>>[0, 2GB], we don't have consistency issue between memblock and buddy allocator.
>>The pages in region [2GB ...] are all presented to buddy allocator despite they're
>>reserved in memblock or not. It ensures the kernel text section isn't corrupted
>>and we're lucky not seeing program interrupt because of illegal instruction.
>>
>
>After more debugging, it turns out that Michael is correct: we don't have problem
>when CONFIG_NO_BOOTMEM=y. In the case, the page frames in [2G ...] is marked as
>reserved in early stage (as below function calls reveal). During the deferred
>initialization stage, those reserved pages won't be released to buddy allocator:
>
>- Below function calls mark reserved pages according to memblock reserved regions:
>  init/main.c::start_kernel()
>  init/main.c::mm_init()
>  arch/powerpc/mm/mem.c::mem_init()
>  nobootmem.c::free_all_bootmem()            <-> bootmem.c::free_all_bootmem() on !CONFIG_NO_BOOTMEM
>  nobootmem.c::free_low_memory_core_early()
>  nobootmem.c::reserve_bootmem_region()
>
>- In page_alloc.c::deferred_init_memmap(), the reserved pages aren't released
>  to buddy allocator with below check:
>
>                        if (page->flags) {
>                                VM_BUG_ON(page_zone(page) != zone);
>                                goto free_range;
>                        }
>
>
>So the issue is only existing when CONFIG_NO_BOOTMEM=n. The alternative fix would
>be similar to what we have on !CONFIG_NO_BOOTMEM: In early stage, all page structs
>for bootmem reserved pages are initialized and mark them with PG_reserved. I'm
>not sure it's worthy to fix it as we won't support bootmem as Michael mentioned.
>

Mel, could you please confirm if we need a fix on !CONFIG_NO_BOOTMEM? If we need,
I'll respin and send a patch for review.

Thanks,
Gavin