linux-kernel - Re: [PATCH v2 1/2] ARM: mm: fix no-MMU ZERO

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: <64e16669-8bcc-a05e-0eb1-9ae644431d95@benettiengineering.com>
Date:   Fri, 4 Nov 2022 21:07:00 +0100
From:   Giulio Benetti <giulio.benetti@...ettiengineering.com>
To:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Cc:     Russell King <linux@...linux.org.uk>,
        Anshuman Khandual <anshuman.khandual@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kefeng Wang <wangkefeng.wang@...wei.com>,
        Russell King <rmk+kernel@...linux.org.uk>,
        Arnd Bergmann <arnd@...db.de>, Will Deacon <will@...nel.org>
Subject: Re: [PATCH v2 1/2] ARM: mm: fix no-MMU ZERO_PAGE() implementation

Hello Arnd, Russell, All,

is this patch ok or has it some changes to do?

While instead [PATCH 2/2] has a NAK and can be dropped.

Best regards
-- 
Giulio Benetti
Benetti Engineering sas

On 19/10/22 00:25, Giulio Benetti wrote:
> Actually in no-MMU SoCs(i.e. i.MXRT) ZERO_PAGE(vaddr) expands to
> ```
> virt_to_page(0)
> ```
> that in order expands to:
> ```
> pfn_to_page(virt_to_pfn(0))
> ```
> and then virt_to_pfn(0) to:
> ```
> #define virt_to_pfn(0) \
> 	((((unsigned long)(0) - PAGE_OFFSET) >> PAGE_SHIFT) + \
> 	 PHYS_PFN_OFFSET)
> ```
> where PAGE_OFFSET and PHYS_PFN_OFFSET are the DRAM offset(0x80000000) and
> PAGE_SHIFT is 12. This way we obtain 16MB(0x01000000) summed to the base of
> DRAM(0x80000000).
> When ZERO_PAGE(0) is then used, for example in bio_add_page(), the page
> gets an address that is out of DRAM bounds.
> So instead of using fake virtual page 0 let's allocate a dedicated
> zero_page during paging_init() and assign it to a global 'struct page *
> empty_zero_page' the same way mmu.c does and it's the same approach used
> in m68k with commit dc068f462179 as discussed here[0]. Then let's move
> ZERO_PAGE() definition to the top of pgtable.h to be in common between
> mmu.c and nommu.c.
> 
> [0]: https://lore.kernel.org/linux-m68k/2a462b23-5b8e-bbf4-ec7d-778434a3b9d7@google.com/T/#m1266ceb63ad140743174d6b3070364d3c9a5179b
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@...ettiengineering.com>
> ---
> V1->V2:
> * improve commit log as suggested by Arnd Bergmann
> ---
>   arch/arm/include/asm/pgtable-nommu.h |  6 ------
>   arch/arm/include/asm/pgtable.h       | 16 +++++++++-------
>   arch/arm/mm/nommu.c                  | 19 +++++++++++++++++++
>   3 files changed, 28 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm/include/asm/pgtable-nommu.h b/arch/arm/include/asm/pgtable-nommu.h
> index d16aba48fa0a..090011394477 100644
> --- a/arch/arm/include/asm/pgtable-nommu.h
> +++ b/arch/arm/include/asm/pgtable-nommu.h
> @@ -44,12 +44,6 @@
>   
>   typedef pte_t *pte_addr_t;
>   
> -/*
> - * ZERO_PAGE is a global shared page that is always zero: used
> - * for zero-mapped memory areas etc..
> - */
> -#define ZERO_PAGE(vaddr)	(virt_to_page(0))
> -
>   /*
>    * Mark the prot value as uncacheable and unbufferable.
>    */
> diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
> index 78a532068fec..ef48a55e9af8 100644
> --- a/arch/arm/include/asm/pgtable.h
> +++ b/arch/arm/include/asm/pgtable.h
> @@ -10,6 +10,15 @@
>   #include <linux/const.h>
>   #include <asm/proc-fns.h>
>   
> +#ifndef __ASSEMBLY__
> +/*
> + * ZERO_PAGE is a global shared page that is always zero: used
> + * for zero-mapped memory areas etc..
> + */
> +extern struct page *empty_zero_page;
> +#define ZERO_PAGE(vaddr)	(empty_zero_page)
> +#endif
> +
>   #ifndef CONFIG_MMU
>   
>   #include <asm-generic/pgtable-nopud.h>
> @@ -139,13 +148,6 @@ extern pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
>    */
>   
>   #ifndef __ASSEMBLY__
> -/*
> - * ZERO_PAGE is a global shared page that is always zero: used
> - * for zero-mapped memory areas etc..
> - */
> -extern struct page *empty_zero_page;
> -#define ZERO_PAGE(vaddr)	(empty_zero_page)
> -
>   
>   extern pgd_t swapper_pg_dir[PTRS_PER_PGD];
>   
> diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
> index c42debaded95..c1494a4dee25 100644
> --- a/arch/arm/mm/nommu.c
> +++ b/arch/arm/mm/nommu.c
> @@ -26,6 +26,13 @@
>   
>   unsigned long vectors_base;
>   
> +/*
> + * empty_zero_page is a special page that is used for
> + * zero-initialized data and COW.
> + */
> +struct page *empty_zero_page;
> +EXPORT_SYMBOL(empty_zero_page);
> +
>   #ifdef CONFIG_ARM_MPU
>   struct mpu_rgn_info mpu_rgn_info;
>   #endif
> @@ -148,9 +155,21 @@ void __init adjust_lowmem_bounds(void)
>    */
>   void __init paging_init(const struct machine_desc *mdesc)
>   {
> +	void *zero_page;
> +
>   	early_trap_init((void *)vectors_base);
>   	mpu_setup();
> +
> +	/* allocate the zero page. */
> +	zero_page = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> +	if (!zero_page)
> +		panic("%s: Failed to allocate %lu bytes align=0x%lx\n",
> +		      __func__, PAGE_SIZE, PAGE_SIZE);
> +
>   	bootmem_init();
> +
> +	empty_zero_page = virt_to_page(zero_page);
> +	flush_dcache_page(empty_zero_page);
>   }
>   
>   /*