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: <9942C08D-C188-461C-B731-F08DE294CD2B@nvidia.com>
Date: Wed, 04 Dec 2024 11:16:51 -0500
From: Zi Yan <ziy@...dia.com>
To: Vlastimil Babka <vbabka@...e.cz>
Cc: Geert Uytterhoeven <geert@...ux-m68k.org>, linux-mm@...ck.org,
 Andrew Morton <akpm@...ux-foundation.org>,
 David Hildenbrand <david@...hat.com>,
 "Matthew Wilcox (Oracle)" <willy@...radead.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 4 Dec 2024, at 10:41, Vlastimil Babka wrote:

> On 12/4/24 16:24, Zi Yan wrote:
>> On 4 Dec 2024, at 5:41, Geert Uytterhoeven wrote:
>>
>> The provided config does not have THP on, so the changes to mm/huge_memory.c
>> and mm/memory.c do not apply.
>>
>> Can you try the patch below and see if the machine boots? Thanks.
>
> Hmm looks like mips has some involved clear_user_page()
> in arch/mips/include/asm/page.h
>
> So maybe the clearing done as part of page allocator isn't enough here.
>
Basically, mips needs to flush data cache if kmap address is aliased to
userspace address. This means when mips has THP on, the patch below
is not enough to fix the issue.

In post_alloc_hook(), it does not make sense to pass userspace address
in to determine whether to flush dcache or not.

One way to fix it is to add something like arch_userpage_post_alloc()
to flush dcache if kmap address is aliased to userspace address.
But my questions are that
1) if kmap address will always be the same for two separate kmap_local() calls,
2) how much overheads the additional kmap_local() and kunmap_local() have.

>>
>> diff --git a/include/linux/highmem.h b/include/linux/highmem.h
>> index 6e452bd8e7e3..bec9bd715acf 100644
>> --- a/include/linux/highmem.h
>> +++ b/include/linux/highmem.h
>> @@ -224,7 +224,13 @@ static inline
>>  struct folio *vma_alloc_zeroed_movable_folio(struct vm_area_struct *vma,
>>  				   unsigned long vaddr)
>>  {
>> -	return vma_alloc_folio(GFP_HIGHUSER_MOVABLE | __GFP_ZERO, 0, vma, vaddr);
>> +	struct folio *folio;
>> +
>> +	folio = vma_alloc_folio(GFP_HIGHUSER_MOVABLE, 0, vma, vaddr);
>> +	if (folio)
>> +		clear_user_highpage(&folio->page, vaddr);
>> +
>> +	return folio;
>>  }
>>  #endif
>>
>>
>> Best Regards,
>> Yan, Zi
>>


Best Regards,
Yan, Zi