linux-kernel - Re: [PATCH] arm: Extend the check for RAM in /dev/mem

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: <1562900298.1345.12.camel@amazon.de>
Date:   Fri, 12 Jul 2019 02:58:18 +0000
From:   "Raslan, KarimAllah" <karahmed@...zon.de>
To:     "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "anshuman.khandual@....com" <anshuman.khandual@....com>
CC:     "yaojun8558363@...il.com" <yaojun8558363@...il.com>,
        "ard.biesheuvel@...aro.org" <ard.biesheuvel@...aro.org>,
        "linux@...linux.org.uk" <linux@...linux.org.uk>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "james.morse@....com" <james.morse@....com>,
        "robin.murphy@....com" <robin.murphy@....com>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "info@...ux.net" <info@...ux.net>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "yuzhao@...gle.com" <yuzhao@...gle.com>,
        "rppt@...ux.ibm.com" <rppt@...ux.ibm.com>,
        "anders.roxell@...aro.org" <anders.roxell@...aro.org>,
        "will@...nel.org" <will@...nel.org>
Subject: Re: [PATCH] arm: Extend the check for RAM in /dev/mem

On Fri, 2019-07-12 at 08:06 +0530, Anshuman Khandual wrote:
> 
> On 07/12/2019 03:51 AM, KarimAllah Ahmed wrote:
> > 
> > Some valid RAM can live outside kernel control (e.g. using mem= kernel
> > command-line). For these regions, pfn_valid would return "false" causing
> > system RAM to be mapped as uncached. Use memblock instead to identify RAM.
> 
> Once the remaining memory is outside of the kernel (as the admin would have
> intended with mem= command line) what is the particular concern regarding
> the way those get mapped (cached or not) ? It is not to be used any way.

They can be used by user-space which might lead to them being used by the 
kernel. One use-case would be using them as guest memory for KVM as I detailed 
here:

https://lwn.net/Articles/778240/

> 
> > 
> > 
> > Cc: Russell King <linux@...linux.org.uk>
> > Cc: Catalin Marinas <catalin.marinas@....com>
> > Cc: Will Deacon <will@...nel.org>
> > Cc: Mike Rapoport <rppt@...ux.ibm.com>
> > Cc: Andrew Morton <akpm@...ux-foundation.org>
> > Cc: Anders Roxell <anders.roxell@...aro.org>
> > Cc: Enrico Weigelt <info@...ux.net>
> > Cc: Thomas Gleixner <tglx@...utronix.de>
> > Cc: KarimAllah Ahmed <karahmed@...zon.de>
> > Cc: Mark Rutland <mark.rutland@....com>
> > Cc: James Morse <james.morse@....com>
> > Cc: Anshuman Khandual <anshuman.khandual@....com>
> > Cc: Jun Yao <yaojun8558363@...il.com>
> > Cc: Yu Zhao <yuzhao@...gle.com>
> > Cc: Robin Murphy <robin.murphy@....com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@...aro.org>
> > Cc: linux-arm-kernel@...ts.infradead.org
> > Cc: linux-kernel@...r.kernel.org
> > Signed-off-by: KarimAllah Ahmed <karahmed@...zon.de>
> > ---
> >  arch/arm/mm/mmu.c   | 2 +-
> >  arch/arm64/mm/mmu.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> > index 1aa2586..492774b 100644
> > --- a/arch/arm/mm/mmu.c
> > +++ b/arch/arm/mm/mmu.c
> > @@ -705,7 +705,7 @@ static void __init build_mem_type_table(void)
> >  pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
> >  			      unsigned long size, pgprot_t vma_prot)
> >  {
> > -	if (!pfn_valid(pfn))
> > +	if (!memblock_is_memory(__pfn_to_phys(pfn)))
> >  		return pgprot_noncached(vma_prot);
> >  	else if (file->f_flags & O_SYNC)
> >  		return pgprot_writecombine(vma_prot);
> > diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> > index 3645f29..cdc3e8e 100644
> > --- a/arch/arm64/mm/mmu.c
> > +++ b/arch/arm64/mm/mmu.c
> > @@ -78,7 +78,7 @@ void set_swapper_pgd(pgd_t *pgdp, pgd_t pgd)
> >  pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
> >  			      unsigned long size, pgprot_t vma_prot)
> >  {
> > -	if (!pfn_valid(pfn))
> > +	if (!memblock_is_memory(__pfn_to_phys(pfn)))
> 
> pfn_valid() on arm64 checks if the memblock region is mapped i.e does it have
> a linear mapping or not. If a segment of RAM is outside linear mapping due to
> mem= directive and lacks a linear mapping then why should it be mapped similarly
> like system RAM on this path ?

I actually struggled a bit here because there is really no *explicit* 
documentation of what is the expected behavior here, so for me it was open to 
interpretation.

It seems like for you the deciding factor between cached and uncached is the 
existence of linear mapping. However, for me the deciding factor is whether it
is RAM or not. I choose this interpretation because it helps in the KVM
scenario that I mentioned above :)




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879