linux-kernel - Re: [PATCH v9 2/7] mm: introduce clear_pages() and clear_user

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

Message-ID: <c9c75803-c8d3-4f9d-b726-d0aa19926b88@kernel.org>
Date: Mon, 24 Nov 2025 11:26:56 +0100
From: "David Hildenbrand (Red Hat)" <david@...nel.org>
To: "Christophe Leroy (CS GROUP)" <chleroy@...nel.org>,
 Ankur Arora <ankur.a.arora@...cle.com>, linux-kernel@...r.kernel.org,
 linux-mm@...ck.org, x86@...nel.org
Cc: akpm@...ux-foundation.org, bp@...en8.de, dave.hansen@...ux.intel.com,
 hpa@...or.com, mingo@...hat.com, mjguzik@...il.com, luto@...nel.org,
 peterz@...radead.org, tglx@...utronix.de, willy@...radead.org,
 raghavendra.kt@....com, boris.ostrovsky@...cle.com, konrad.wilk@...cle.com
Subject: Re: [PATCH v9 2/7] mm: introduce clear_pages() and clear_user_pages()

Replying here while I am already at it.

>> +#ifndef clear_pages
>> +/**
>> + * clear_pages() - clear a page range for kernel-internal use.
>> + * @addr: start address
>> + * @npages: number of pages
>> + *
>> + * Use clear_user_pages() instead when clearing a page range to be
>> + * mapped to user space.
>> + *
>> + * Does absolutely no exception handling.
>> + */
>> +static inline void clear_pages(void *addr, unsigned int npages)
>> +{
>> +	do {
>> +		clear_page(addr);
>> +		addr += PAGE_SIZE;
>> +	} while (--npages);
> 
> Why a 'do while' instead of a 'while' ?

More efficient when we know that npages > 0.

> 
> Are you certain that this function will never ever be called with a nul
> npages ?

That is the expectation here, yes. We should probably document that 
expectation.

> 
>> +}
>> +#endif
>> +
>>    #ifndef clear_user_page
>>    /**
>>     * clear_user_page() - clear a page to be mapped to user space
>> @@ -3901,6 +3921,27 @@ static inline void clear_user_page(void *addr, unsigned long vaddr, struct page
>>    }
>>    #endif
>>    
>> +/**
>> + * clear_user_pages() - clear a page range to be mapped to user space
>> + * @addr: start address
>> + * @vaddr: start address of the user mapping
>> + * @page: start page
>> + * @npages: number of pages
>> + *
>> + * Assumes that the region (@addr, +@...ges) has been validated
>> + * already so this does no exception handling.
>> + */
>> +#ifdef clear_user_pages
>> +void clear_user_pages(void *addr, unsigned long vaddr,
>> +		struct page *page, unsigned int npages);
> 
> By doing this you forbid architectures to define it as a static inline,
> is that wanted ?

Note that this is not the intention. The intention is to either use a 
direct mapping to clear_pages(), or fallback to the variant in mm/util.c.

The architecture is currently never expected to provide clear_user_pages().

Wondering if we can make that cleaner.

I'm wondering if the dependency on highmem.h here in mm.h is rather the 
problem.

How I hate this macro crap with arch overrides.

> 
>> +#else
>> +static inline void clear_user_pages(void *addr, unsigned long vaddr,
>> +		struct page *page, unsigned int npages)
>> +{
>> +	clear_pages(addr, npages);
>> +}
>> +#endif
>> +
>>    #ifdef __HAVE_ARCH_GATE_AREA
>>    extern struct vm_area_struct *get_gate_vma(struct mm_struct *mm);
>>    extern int in_gate_area_no_mm(unsigned long addr);
>> diff --git a/mm/util.c b/mm/util.c
>> index 8989d5767528..3c6cd44db1bd 100644
>> --- a/mm/util.c
>> +++ b/mm/util.c
>> @@ -1344,3 +1344,16 @@ bool page_range_contiguous(const struct page *page, unsigned long nr_pages)
>>    }
>>    EXPORT_SYMBOL(page_range_contiguous);
>>    #endif
>> +
>> +#ifdef clear_user_page
>> +void clear_user_pages(void *addr,
> 
> What happens if clear_user_page is defined but not clear_user_pages ? In
> that case it seems like the definition in linux/mm.h will conflict.

The generic mm.h variant will not set clear_user_page() and consequently 
we map directly to clear_pages().

-- 
Cheers

David