linux-kernel - Re: [PATCH] mm: page_alloc: ignore init_on

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]

Message-ID: <469d6417-64a5-fe99-1214-bb0351833b74@suse.cz>
Date:   Mon, 29 Mar 2021 14:10:02 +0200
From:   Vlastimil Babka <vbabka@...e.cz>
To:     David Hildenbrand <david@...hat.com>,
        Sergei Trofimovich <slyfox@...too.org>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org
Cc:     linux-kernel@...r.kernel.org,
        Andrey Konovalov <andreyknvl@...il.com>
Subject: Re: [PATCH] mm: page_alloc: ignore init_on_free=1 for page alloc

On 3/26/21 2:48 PM, David Hildenbrand wrote:
> On 26.03.21 12:26, Sergei Trofimovich wrote:
>> init_on_free=1 does not guarantee that free pages contain only zero bytes.
>>
>> Some examples:
>> 1. page_poison=on takes presedence over init_on_alloc=1 / ini_on_free=1
> 
> s/ini_on_free/init_on_free/
> 
>> 2. free_pages_prepare() always poisons pages:
>>
>>         if (want_init_on_free())
>>             kernel_init_free_pages(page, 1 << order);
>>         kernel_poison_pages(page, 1 << order
> 
> In next/master, it's the other way around already.

And that should be OK as the order should not matter, as long as they are indeed
exclusive. They should be after Sergei's v2 fix.
As long as kasan_free_nondeferred_pages() which follows doesn't do anything
unexpected to poisoned pages (I haven't check).


> commit 855a9c4018f3219db8be7e4b9a65ab22aebfde82
> Author: Andrey Konovalov <andreyknvl@...il.com>
> Date:   Thu Mar 18 17:01:40 2021 +1100
> 
>     kasan, mm: integrate page_alloc init with HW_TAGS

But the mmotm patch/-next commit also changes post_alloc_hook()

Before the patch it was:
kernel_unpoison_pages(page, 1 << order);
...
kernel_init_free_pages(page, 1 << order);

Now it is (for !kasan_has_integrated_init()):

kernel_init_free_pages(page, 1 << order);

kernel_unpoison_pages(page, 1 << order);

That has to be wrong, because we init the page with zeroes and then call
kernel_unpoison_pages() which checks for the 0xaa pattern. Andrey?

>>
>> I observed use of poisoned pages as the crash on ia64 booted with
>> init_on_free=1 init_on_alloc=1 (CONFIG_PAGE_POISONING=y config).
>> There pmd page contained 0xaaaaaaaa poison pages and led to early crash.
>>
>> The change drops the assumption that init_on_free=1 guarantees free
>> pages to contain zeros.
>>
>> Alternative would be to make interaction between runtime poisoning and
>> sanitizing options and build-time debug flags like CONFIG_PAGE_POISONING
>> more coherent. I took the simpler path.
>>
> 
> I thought latest work be Vlastimil tried to tackle that. To me, it feels like
> page_poison=on  and init_on_free=1 should bail out and disable one of both
> things. Having both at the same time doesn't sound helpful.
> 
>> Tested the fix on rx3600.
> 
> Fixes: ?
> 
>