linux-kernel - Re: [PATCH v7 1/1] memory_hotplug: Add a bounds check to __add

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: <20191002151416.42bc2e8228fdefc6eb802abc@linux-foundation.org>
Date:   Wed, 2 Oct 2019 15:14:16 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     David Hildenbrand <david@...hat.com>
Cc:     "Alastair D'Silva" <alastair@....ibm.com>, alastair@...ilva.org,
        Oscar Salvador <osalvador@...e.de>,
        Michal Hocko <mhocko@...e.com>,
        Pavel Tatashin <pasha.tatashin@...een.com>,
        Dan Williams <dan.j.williams@...el.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 1/1] memory_hotplug: Add a bounds check to
 __add_pages

On Tue, 1 Oct 2019 11:49:47 +0200 David Hildenbrand <david@...hat.com> wrote:

> > @@ -278,6 +278,22 @@ static int check_pfn_span(unsigned long pfn, unsigned long nr_pages,
> >  	return 0;
> >  }
> >  
> > +static int check_hotplug_memory_addressable(unsigned long pfn,
> > +					    unsigned long nr_pages)
> > +{
> > +	const u64 max_addr = PFN_PHYS(pfn + nr_pages) - 1;
> > +
> > +	if (max_addr >> MAX_PHYSMEM_BITS) {
> > +		const u64 max_allowed = (1ull << (MAX_PHYSMEM_BITS + 1)) - 1;
> > +		WARN(1,
> > +		     "Hotplugged memory exceeds maximum addressable address, range=%#llx-%#llx, maximum=%#llx\n",
> > +		     (u64)PFN_PHYS(pfn), max_addr, max_allowed);
> > +		return -E2BIG;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  /*
> >   * Reasonably generic function for adding memory.  It is
> >   * expected that archs that support memory hotplug will
> > @@ -291,6 +307,10 @@ int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
> >  	unsigned long nr, start_sec, end_sec;
> >  	struct vmem_altmap *altmap = restrictions->altmap;
> >  
> > +	err = check_hotplug_memory_addressable(pfn, nr_pages);
> > +	if (err)
> > +		return err;
> > +
> >  	if (altmap) {
> >  		/*
> >  		 * Validate altmap is within bounds of the total request
> > 
> 
> I actually wanted to give my RB to v7, not v6 :)
>

Given that check_hotplug_memory_addressable() is now static, I'll
assume that the old [2/2]
mm-add-a-bounds-check-in-devm_memremap_pages.patch is now obsolete.

From: Alastair D'Silva <alastair@...ilva.org>
Subject: mm/memremap.c: add a bounds check in devm_memremap_pages()

The call to check_hotplug_memory_addressable() validates that the memory
is fully addressable.

Without this call, it is possible that we may remap pages that is not
physically addressable, resulting in bogus section numbers being returned
from __section_nr().

Link: http://lkml.kernel.org/r/20190917010752.28395-3-alastair@au1.ibm.com
Signed-off-by: Alastair D'Silva <alastair@...ilva.org>
Acked-by: David Hildenbrand <david@...hat.com>
Cc: Dan Williams <dan.j.williams@...el.com>
Cc: Ira Weiny <ira.weiny@...el.com>
Cc: Jason Gunthorpe <jgg@...pe.ca>
Cc: Logan Gunthorpe <logang@...tatee.com>
Cc: Michal Hocko <mhocko@...e.com>
Cc: Oscar Salvador <osalvador@...e.com>
Cc: Pavel Tatashin <pasha.tatashin@...een.com>
Cc: Qian Cai <cai@....pw>
Cc: Wei Yang <richard.weiyang@...il.com>
Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
---

 mm/memremap.c |    5 +++++
 1 file changed, 5 insertions(+)

--- a/mm/memremap.c~mm-add-a-bounds-check-in-devm_memremap_pages
+++ a/mm/memremap.c
@@ -185,6 +185,11 @@ void *memremap_pages(struct dev_pagemap
 	int error, is_ram;
 	bool need_devmap_managed = true;
 
+	error = check_hotplug_memory_addressable(res->start,
+						 resource_size(res));
+	if (error)
+		return ERR_PTR(error);
+
 	switch (pgmap->type) {
 	case MEMORY_DEVICE_PRIVATE:
 		if (!IS_ENABLED(CONFIG_DEVICE_PRIVATE)) {
_