Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 25 Mar 2019 23:55:14 +0100 (CET)
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Ralph Campbell <rcampbell@nvidia.com>
cc:     linux-kernel@vger.kernel.org, Craig Bergstrom <craigb@google.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        Fengguang Wu <fengguang.wu@intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Hans Verkuil <hans.verkuil@cisco.com>,
        Mauro Carvalho Chehab <mchehab@s-opensource.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Sander Eikelenboom <linux@eikelenboom.it>,
        Sean Young <sean@mess.org>, Ingo Molnar <mingo@kernel.org>
Subject: Re: [PATCH 1/1] x86/mm: Fix limit mmap() of /dev/mem to valid physical
 addresses
In-Reply-To: <39b65bda-cef5-4ea2-ddaa-418e8e71e306@nvidia.com>
Message-ID: <alpine.DEB.2.21.1903252346310.1789@nanos.tec.linutronix.de>
References: <20190318224653.26549-1-rcampbell@nvidia.com> <20190318224653.26549-2-rcampbell@nvidia.com> <alpine.DEB.2.21.1903231840100.1798@nanos.tec.linutronix.de> <39b65bda-cef5-4ea2-ddaa-418e8e71e306@nvidia.com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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