linux-kernel - Re: [PATCH v2 1/2] mm: Move page struct poisoning to CONFIG_DEBUG_VM_PAGE_INIT

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 5 Sep 2018 21:38:19 +0000
From:   Pasha Tatashin <Pavel.Tatashin@...rosoft.com>
To:     Alexander Duyck <alexander.duyck@...il.com>
CC:     linux-mm <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>,
        "Duyck, Alexander H" <alexander.h.duyck@...el.com>,
        Michal Hocko <mhocko@...e.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Subject: Re: [PATCH v2 1/2] mm: Move page struct poisoning to
 CONFIG_DEBUG_VM_PAGE_INIT_POISON



On 9/5/18 5:29 PM, Alexander Duyck wrote:
> On Wed, Sep 5, 2018 at 2:22 PM Pasha Tatashin
> <Pavel.Tatashin@...rosoft.com> wrote:
>>
>>
>>
>> On 9/5/18 5:13 PM, Alexander Duyck wrote:
>>> From: Alexander Duyck <alexander.h.duyck@...el.com>
>>>
>>> On systems with a large amount of memory it can take a significant amount
>>> of time to initialize all of the page structs with the PAGE_POISON_PATTERN
>>> value. I have seen it take over 2 minutes to initialize a system with
>>> over 12GB of RAM.
>>>
>>> In order to work around the issue I had to disable CONFIG_DEBUG_VM and then
>>> the boot time returned to something much more reasonable as the
>>> arch_add_memory call completed in milliseconds versus seconds. However in
>>> doing that I had to disable all of the other VM debugging on the system.
>>>
>>> Instead of keeping the value in CONFIG_DEBUG_VM I am adding a new CONFIG
>>> value called CONFIG_DEBUG_VM_PAGE_INIT_POISON that will control the page
>>> poisoning independent of the CONFIG_DEBUG_VM option.
>>>
>>> Signed-off-by: Alexander Duyck <alexander.h.duyck@...el.com>
>>> ---
>>>  include/linux/page-flags.h |    8 ++++++++
>>>  lib/Kconfig.debug          |   14 ++++++++++++++
>>>  mm/memblock.c              |    5 ++---
>>>  mm/sparse.c                |    4 +---
>>>  4 files changed, 25 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>>> index 74bee8cecf4c..0e95ca63375a 100644
>>> --- a/include/linux/page-flags.h
>>> +++ b/include/linux/page-flags.h
>>> @@ -13,6 +13,7 @@
>>>  #include <linux/mm_types.h>
>>>  #include <generated/bounds.h>
>>>  #endif /* !__GENERATING_BOUNDS_H */
>>> +#include <linux/string.h>
>>>
>>>  /*
>>>   * Various page->flags bits:
>>> @@ -162,6 +163,13 @@ static inline int PagePoisoned(const struct page *page)
>>>       return page->flags == PAGE_POISON_PATTERN;
>>>  }
>>>
>>> +static inline void page_init_poison(struct page *page, size_t size)
>>> +{
>>> +#ifdef CONFIG_DEBUG_VM_PAGE_INIT_POISON
>>> +     memset(page, PAGE_POISON_PATTERN, size);
>>> +#endif
>>> +}
>>> +
>>>  /*
>>>   * Page flags policies wrt compound pages
>>>   *
>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>>> index 613316724c6a..3b1277c52fed 100644
>>> --- a/lib/Kconfig.debug
>>> +++ b/lib/Kconfig.debug
>>> @@ -637,6 +637,20 @@ config DEBUG_VM_PGFLAGS
>>>
>>>         If unsure, say N.
>>>
>>> +config DEBUG_VM_PAGE_INIT_POISON
>>> +     bool "Enable early page metadata poisoning"
>>> +     default y
>>> +     depends on DEBUG_VM
>>> +     help
>>> +       Seed the page metadata with a poison pattern to improve the
>>> +       likelihood of detecting attempts to access the page prior to
>>> +       initialization by the memory subsystem.
>>> +
>>> +       This initialization can result in a longer boot time for systems
>>> +       with a large amount of memory.
>>
>> What happens when DEBUG_VM_PGFLAGS = y and
>> DEBUG_VM_PAGE_INIT_POISON = n ?
>>
>> We are testing for pattern that was not set?
>>
>> I think DEBUG_VM_PAGE_INIT_POISON must depend on DEBUG_VM_PGFLAGS instead.
>>
>> Looks good otherwise.
>>
>> Thank you,
>> Pavel
> 
> The problem is that I then end up in the same situation I had in the
> last patch where you have to have DEBUG_VM_PGFLAGS on in order to do
> the seeding with poison.

OK

> 
> I can wrap the bit of code in PagePoisoned to just always return false
> if we didn't set the pattern. I figure there is value to be had for
> running DEBUG_VM_PGFLAGS regardless of the poison check, or
> DEBUG_VM_PAGE_INIT_POISON without the PGFLAGS check. That is why I
> wanted to leave them independent.

How about:

Remove "depends on DEBUG_VM", but make DEBUG_VM_PGFLAGS to depend on
both DEBUG_VM and DEBUG_VM_PAGE_INIT_POISON?

DEBUG_VM_PGFLAGS is already extremely slow, so having this extra
dependency is OK.

Thank you,
Pavel

> 
> - Alex
>