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

Message-ID: <87ms456cpf.fsf@oracle.com>
Date: Fri, 28 Nov 2025 13:59:08 -0800
From: Ankur Arora <ankur.a.arora@...cle.com>
To: Lance Yang <ioworker0@...il.com>
Cc: david@...nel.org, akpm@...ux-foundation.org, ankur.a.arora@...cle.com,
        boris.ostrovsky@...cle.com, bp@...en8.de, chleroy@...nel.org,
        dave.hansen@...ux.intel.com, hpa@...or.com, konrad.wilk@...cle.com,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org, luto@...nel.org,
        mingo@...hat.com, mjguzik@...il.com, peterz@...radead.org,
        raghavendra.kt@....com, tglx@...utronix.de, willy@...radead.org,
        x86@...nel.org, Lance Yang <lance.yang@...ux.dev>
Subject: Re: [PATCH v9 2/7] mm: introduce clear_pages() and clear_user_pages()


Lance Yang <ioworker0@...il.com> writes:

> From: Lance Yang <lance.yang@...ux.dev>
>
>
> On Mon, 24 Nov 2025 11:26:56 +0100, David Hildenbrand (Red Hat) wrote:
>> 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().
>
> Hmm, I suspect there might be a subtle issue with the build flow on SPARC ...
>
> Inside include/linux/mm.h, the guard checks for clear_user_pages (plural).
> Since SPARC doesn't define that, the header provides the static inline
> fallback.
>
> However, mm/util.c includes that header. And since SPARC does define
> clear_user_page (singular), the .c file proceeds to compile the non-static
> definition as well.
>
> Wouldn't that result in the compiler seeing both a static inline and a
> non-static definition in the same translation unit? It seems like this
> would trigger a redefinition error ...

Yeah it would.

I had only posted the linux/highmem.h header bits for brevity but the
full patch removes the mm/util.c bits.

Sorry about the confusion.

--
ankur