linux-kernel - Re: [PATCH] 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: <3fb4afd1-2eea-4a71-a914-f8208b11f9f4@app.fastmail.com>
Date:   Tue, 18 Oct 2022 09:03:01 +0200
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Giulio Benetti" <giulio.benetti@...ettiengineering.com>,
        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>,
        "Will Deacon" <will@...nel.org>
Subject: Re: [PATCH] ARM: mm: fix no-MMU ZERO_PAGE() implementation

On Tue, Oct 18, 2022, at 1:37 AM, 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. Then let's move ZERO_PAGE()
> definition to the top of pgtable.h to be in common between mmu.c and
> nommu.c.
>
> Signed-off-by: Giulio Benetti <giulio.benetti@...ettiengineering.com>

Maybe mention commit dc068f462179 ("m68knommu: set ZERO_PAGE() to the
allocated zeroed page") for the commit that fixed this first,
as well as the previous discussion at
https://lore.kernel.org/linux-m68k/2a462b23-5b8e-bbf4-ec7d-778434a3b9d7@google.com/T/#m1266ceb63ad140743174d6b3070364d3c9a5179b

It looks like we dropped the ball on this when it came up last.
I'm also not sure when we started requiring this, any idea?
I can see that microblaze-nommu used BUG() in ZERO_PAGE(), so at
whenever microblaze last worked, we clearly did not call it.

> +#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

In addition to your fix, I see that arm is the only architecture
that defines 'empty_zero_page' as a pointer to the page, when
everything else just makes it a pointer to the data itself,
or an 'extern char empty_zero_page[]' array, which we may want
to change for consistency.

There are three references to empty_zero_page in architecture
independent code, and while we don't seem to use any of them
on Arm, they would clearly be wrong if we did:

drivers/acpi/scan.c:#define INVALID_ACPI_HANDLE ((acpi_handle)empty_zero_page)
drivers/spi/spi-fsl-cpm.c:      mspi->dma_dummy_tx = dma_map_single(dev, empty_zero_page, PAGE_SIZE,
include/linux/raid/pq.h:# define raid6_empty_zero_page empty_zero_page

     Arnd