linux-kernel - Re: [PATCH] mm: avoid zeroing user movable page twice with 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: <434596ce-5514-42f3-84f5-f8a70954f058@suse.cz>
Date: Thu, 5 Dec 2024 18:24:50 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: Zi Yan <ziy@...dia.com>, Geert Uytterhoeven <geert@...ux-m68k.org>,
 David Hildenbrand <david@...hat.com>
Cc: Matthew Wilcox <willy@...radead.org>, linux-mm@...ck.org,
 Andrew Morton <akpm@...ux-foundation.org>, Miaohe Lin
 <linmiaohe@...wei.com>, Kefeng Wang <wangkefeng.wang@...wei.com>,
 John Hubbard <jhubbard@...dia.com>, "Huang, Ying" <ying.huang@...el.com>,
 Ryan Roberts <ryan.roberts@....com>, Alexander Potapenko
 <glider@...gle.com>, Kees Cook <keescook@...omium.org>,
 linux-kernel@...r.kernel.org, linux-mips@...r.kernel.org
Subject: Re: [PATCH] mm: avoid zeroing user movable page twice with
 init_on_alloc=1

On 12/5/24 17:05, Zi Yan wrote:
> On 5 Dec 2024, at 3:10, David Hildenbrand wrote:
>>>
>>> Kernel log confirms it's enabled:
>>> -mem auto-init: stack:off, heap alloc:off, heap free:off
>>> +mem auto-init: stack:off, heap alloc:on, heap free:off
>>
>> If I'm not wrong that's expected ... because we'll be double-zeroing that memory, clearing the cache :)
>>
>> I guess the question is, how *effective* is CONFIG_INIT_ON_ALLOC_DEFAULT_ON on systems to prevent exposing un-zeroed data to userspace, when it doesn't end up doing the flush we really need.
> 
> Hi Geert,
> 
> Is it possible to run a 32bit kernel with HIGHMEM and
> CONFIG_INIT_ON_ALLOC_DEFAULT_ON on the machine (of course with my patch
> reverted)? Just to check my reasoning below.
> 
> Thanks.
> 
> 
> Yes, it should work, since I forgot the actual issue is HIGHMEM+cache flush, not just cache flush is needed after clearing user page.
> 
> For arch which needs to flush cache after clearing user page, with HIGHMEM,
> init_on_alloc first clears the page using kmap_addr0 without flushing
> the cache, then clear_user_page() clears the page using kmap_addr1
> with cache flush. After returning to userspace, the cache lines of
> kmap_addr0 will be evicted and written back to RAM eventually, corrupting
> user data with 0s, because no one flushes them before returning to userspace.
> 
> For a proper fix, I will add ARCH_HAS_OPS_AFTER_CLEAR_USER_PAGE and
> make mips, sh, sparc, arm, xtensa, nios2, m68k, parisc, csky, arc, and powerpc
> select it, then make alloc_zeroed() returns false if
> ARCH_HAS_OPS_AFTER_CLEAR_USER_PAGE is enabled.
> 
> If my reasoning above is verified to be true, I will send a separate patch
> to disable CONFIG_INIT_ON_ALLOC_DEFAULT_ON if HIGHMEM &&
> ARCH_HAS_OPS_AFTER_CLEAR_USER_PAGE.

If your reasoning is true, wouldn't any other user of kmap_local_page() of a
highpage on such system also leave the cache unflushed in case the page is
ever reused as a userspace page?

> Best Regards,
> Yan, Zi