linux-kernel - Re: [PATCH] arm64: define __alloc_zeroed_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: <20200312164922.GC21120@lakrids.cambridge.arm.com>
Date:   Thu, 12 Mar 2020 16:49:22 +0000
From:   Mark Rutland <mark.rutland@....com>
To:     glider@...gle.com
Cc:     catalin.marinas@....com, will.deacon@....com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        keescook@...omium.org, akpm@...ux-foundation.org
Subject: Re: [PATCH] arm64: define __alloc_zeroed_user_highpage

On Thu, Mar 12, 2020 at 04:59:20PM +0100, glider@...gle.com wrote:
> When running the kernel with init_on_alloc=1, calling the default
> implementation of __alloc_zeroed_user_highpage() from include/linux/highmem.h
> leads to double-initialization of the allocated page (first by the page
> allocator, then by clear_user_page().
> Calling alloc_page_vma() with __GFP_ZERO, similarly to e.g. x86, seems
> to be enough to ensure the user page is zeroed only once.

Just to check, is there a functional ussue beyond the redundant zeroing,
or is this jsut a performance issue?

On architectures with real highmem, does GFP_HIGHUSER prevent the
allocator from zeroing the page in this case, or is the architecture
prevented from allocating from highmem?

This feels like something we should be able to fix in the generic
implementation of __alloc_zeroed_user_highpage(), with an additional
check to see if init_on_alloc is in use.

Thanks,
Mark.

> 
> Signed-off-by: Alexander Potapenko <glider@...gle.com>
> ---
>  arch/arm64/include/asm/page.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/page.h b/arch/arm64/include/asm/page.h
> index d39ddb258a049..75d6cd23a6790 100644
> --- a/arch/arm64/include/asm/page.h
> +++ b/arch/arm64/include/asm/page.h
> @@ -21,6 +21,10 @@ extern void __cpu_copy_user_page(void *to, const void *from,
>  extern void copy_page(void *to, const void *from);
>  extern void clear_page(void *to);
>  
> +#define __alloc_zeroed_user_highpage(movableflags, vma, vaddr) \
> +	alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
> +#define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE
> +
>  #define clear_user_page(addr,vaddr,pg)  __cpu_clear_user_page(addr, vaddr)
>  #define copy_user_page(to,from,vaddr,pg) __cpu_copy_user_page(to, from, vaddr)
>  
> -- 
> 2.25.1.481.gfbce0eb801-goog
>