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

Message-ID: <2a3dac67-d8c5-0c09-38fc-d4bd559c18b7@benettiengineering.com>
Date:   Wed, 19 Oct 2022 19:07:13 +0200
From:   Giulio Benetti <giulio.benetti@...ettiengineering.com>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>,
        Arnd Bergmann <arnd@...db.de>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Anshuman Khandual <anshuman.khandual@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kefeng Wang <wangkefeng.wang@...wei.com>,
        Will Deacon <will@...nel.org>
Subject: Re: [PATCH] ARM: mm: fix no-MMU ZERO_PAGE() implementation

Hello Russell, Arnd, All,

On 19/10/22 11:09, Russell King (Oracle) wrote:
> On Tue, Oct 18, 2022 at 09:03:01AM +0200, Arnd Bergmann wrote:
>> 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.
> 
> ARM's implementation is the utterly sensible implementation IMHO.
> 
> When the only users in the kernel _were_ ZERO_PAGE() for this, which
> is defined to return a struct page pointer, there was no need to make
> "empty_zero_page" anything but a struct page pointer, rather than a
> runtime translation from an address to a struct page.
> 
> IMHO, we should _not_ be exposing empty_zero_page to devices - we
> certainly do not want the DMA API performing cache maintenance on
> this page since the primary purpose of this page is to fill in
> userspace BSS pages that have not been written.
> 
> ACPI's use is just to have a cookie for invalid handles, and using
> the struct page pointer is good enough.
> 
> The only problem one is the RAID6 code, but that is disabled:
> 
> /* Set to 1 to use kernel-wide empty_zero_page */
> #define RAID6_USE_EMPTY_ZERO_PAGE 0
> 
> #if RAID6_USE_EMPTY_ZERO_PAGE
> # define raid6_empty_zero_page empty_zero_page
> #else
> extern const char raid6_empty_zero_page[PAGE_SIZE];
> #endif

For this I've sent a patch to remove the unused code:
https://lore.kernel.org/all/20221019160407.7550-1-giulio.benetti@benettiengineering.com/

> So, the only one that needs fixing is the SPI usage, which IMHO
> is wrong. ARM being different finds what I consider a driver bug.
> Good for 32-bit ARM. :)

Oh, I've sent a patch for substituting ZERO_PAGE(0) and it's already
been applied to spi's for-next:
https://lore.kernel.org/all/166619141690.565256.8563939546728659746.b4-ty@kernel.org/
So this doesn't break the build but there is still a bug.

Just to understand if I've understood correctly. The correct fix would
be to kzalloc() a dma_dummy_tx buffer and use it in place of
ZERO_PAGE(0), right?

Can you also please point me some link explaining the structure of this
topic? I've already started to read:
https://docs.kernel.org/core-api/index.html#memory-management

Maybe it is enough. I'd appreciate a lot any further links to get into
this if any.

Thank you

Kind regards
-- 
Giulio Benetti
CEO/CTO@...etti Engineering sas