linux-kernel - Re: [PATCH 1/1] x86/mm: Fix limit mmap() of /dev/mem to valid physical addresses

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]

Date:   Mon, 25 Mar 2019 23:55:14 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Ralph Campbell <rcampbell@...dia.com>
cc:     linux-kernel@...r.kernel.org, Craig Bergstrom <craigb@...gle.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Fengguang Wu <fengguang.wu@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Hans Verkuil <hans.verkuil@...co.com>,
        Mauro Carvalho Chehab <mchehab@...pensource.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Sander Eikelenboom <linux@...elenboom.it>,
        Sean Young <sean@...s.org>, Ingo Molnar <mingo@...nel.org>
Subject: Re: [PATCH 1/1] x86/mm: Fix limit mmap() of /dev/mem to valid physical
 addresses

Ralph,

On Mon, 25 Mar 2019, Ralph Campbell wrote:
> On 3/23/19 12:02 PM, Thomas Gleixner wrote:
> > > --- a/arch/x86/mm/mmap.c
> > > +++ b/arch/x86/mm/mmap.c
> > > @@ -230,7 +230,7 @@ bool mmap_address_hint_valid(unsigned long addr,
> > > unsigned long len)
> > >   /* Can we access it for direct reading/writing? Must be RAM: */
> > >   int valid_phys_addr_range(phys_addr_t addr, size_t count)
> > >   {
> > > -	return addr + count <= __pa(high_memory);
> > > +	return addr + count <= __pa_nodebug(high_memory);
> > 
> > This lacks a comment. Aside of that I think there is no point in using
> > __pa(high_memory) here. This is all about the physical address range. So
> > this can be simply expressed via:
> > 
> > 	return addr + count <= max_pfn * PAGE_SIZE;
> > 
> > which is much more obvious.
> 
> This looks OK to me for x86_64 but looking at arch/x86/mm/init_32.c,
> initmem_init() sets high_memory based on highstart_pfn or max_low_pfn
> depending on CONFIG_HIGHMEM. Would using max_pfn in this case work?

Aargh. There is also numa_32.c ...

So __pa_nodebug(high_memory) should be fine, but looking at the other
places which do __pa() on high_memory they all use __pa(high_memory - 1)
which is more obvious that the nodebug thing.

Thanks,

	tglx