linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/1] Add bounds check for Hotplugged memory
@ 2019-10-01  0:46 Alastair D'Silva
  2019-10-01  0:46 ` [PATCH v7 1/1] memory_hotplug: Add a bounds check to __add_pages Alastair D'Silva
  0 siblings, 1 reply; 6+ messages in thread
From: Alastair D'Silva @ 2019-10-01  0:46 UTC (permalink / raw)
  To: alastair
  Cc: Andrew Morton, Oscar Salvador, Michal Hocko, David Hildenbrand,
	Pavel Tatashin, Dan Williams, linux-mm, linux-kernel

From: Alastair D'Silva <alastair@d-silva.org>

This series adds bounds checks for hotplugged memory, ensuring that
it is within the physically addressable range (for platforms that
define MAX_(POSSIBLE_)PHYSMEM_BITS.

This allows for early failure, rather than attempting to access
bogus section numbers.

Changelog:
 V7:
   - Cast PFN_PHYS as u64 since it's type is platform dependant
 V6:
   - Fix printf formats
 V5:
   - Factor out calculation into max_allowed var
   - Declare unchanging vars as const
   - Use PFN_PHYS macro instead of shifting by PAGE_SHIFT
 V4:
   - Relocate call to __add_pages
   - Add a warning when the addressable check fails
 V3:
   - Perform the addressable check before we take the hotplug lock
 V2:
   - Don't use MAX_POSSIBLE_PHYSMEM_BITS as it's wider that what
     may be available

Alastair D'Silva (1):
  memory_hotplug: Add a bounds check to __add_pages

 mm/memory_hotplug.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

-- 
2.21.0



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v7 1/1] memory_hotplug: Add a bounds check to __add_pages
  2019-10-01  0:46 [PATCH v7 0/1] Add bounds check for Hotplugged memory Alastair D'Silva
@ 2019-10-01  0:46 ` Alastair D'Silva
  2019-10-01  9:49   ` David Hildenbrand
  2019-10-04 12:08   ` Michal Hocko
  0 siblings, 2 replies; 6+ messages in thread
From: Alastair D'Silva @ 2019-10-01  0:46 UTC (permalink / raw)
  To: alastair
  Cc: Andrew Morton, Oscar Salvador, Michal Hocko, David Hildenbrand,
	Pavel Tatashin, Dan Williams, linux-mm, linux-kernel

From: Alastair D'Silva <alastair@d-silva.org>

On PowerPC, the address ranges allocated to OpenCAPI LPC memory
are allocated from firmware. These address ranges may be higher
than what older kernels permit, as we increased the maximum
permissable address in commit 4ffe713b7587
("powerpc/mm: Increase the max addressable memory to 2PB"). It is
possible that the addressable range may change again in the
future.

In this scenario, we end up with a bogus section returned from
__section_nr (see the discussion on the thread "mm: Trigger bug on
if a section is not found in __section_nr").

Adding a check here means that we fail early and have an
opportunity to handle the error gracefully, rather than rumbling
on and potentially accessing an incorrect section.

Further discussion is also on the thread ("powerpc: Perform a bounds
check in arch_add_memory")
http://lkml.kernel.org/r/20190827052047.31547-1-alastair@au1.ibm.com

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 mm/memory_hotplug.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index c73f09913165..5af9f4466ad1 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -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
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v7 1/1] memory_hotplug: Add a bounds check to __add_pages
  2019-10-01  0:46 ` [PATCH v7 1/1] memory_hotplug: Add a bounds check to __add_pages Alastair D'Silva
@ 2019-10-01  9:49   ` David Hildenbrand
  2019-10-02 22:14     ` Andrew Morton
  2019-10-04 12:08   ` Michal Hocko
  1 sibling, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2019-10-01  9:49 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Andrew Morton, Oscar Salvador, Michal Hocko, Pavel Tatashin,
	Dan Williams, linux-mm, linux-kernel

On 01.10.19 02:46, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> On PowerPC, the address ranges allocated to OpenCAPI LPC memory
> are allocated from firmware. These address ranges may be higher
> than what older kernels permit, as we increased the maximum
> permissable address in commit 4ffe713b7587
> ("powerpc/mm: Increase the max addressable memory to 2PB"). It is
> possible that the addressable range may change again in the
> future.
> 
> In this scenario, we end up with a bogus section returned from
> __section_nr (see the discussion on the thread "mm: Trigger bug on
> if a section is not found in __section_nr").
> 
> Adding a check here means that we fail early and have an
> opportunity to handle the error gracefully, rather than rumbling
> on and potentially accessing an incorrect section.
> 
> Further discussion is also on the thread ("powerpc: Perform a bounds
> check in arch_add_memory")
> http://lkml.kernel.org/r/20190827052047.31547-1-alastair@au1.ibm.com
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>  mm/memory_hotplug.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index c73f09913165..5af9f4466ad1 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -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 :)

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v7 1/1] memory_hotplug: Add a bounds check to __add_pages
  2019-10-01  9:49   ` David Hildenbrand
@ 2019-10-02 22:14     ` Andrew Morton
  2019-10-02 23:27       ` Alastair D'Silva
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2019-10-02 22:14 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Alastair D'Silva, alastair, Oscar Salvador, Michal Hocko,
	Pavel Tatashin, Dan Williams, linux-mm, linux-kernel

On Tue, 1 Oct 2019 11:49:47 +0200 David Hildenbrand <david@redhat.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@d-silva.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@d-silva.org>
Acked-by: David Hildenbrand <david@redhat.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Logan Gunthorpe <logang@deltatee.com>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Oscar Salvador <osalvador@suse.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: Qian Cai <cai@lca.pw>
Cc: Wei Yang <richard.weiyang@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-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)) {
_



^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH v7 1/1] memory_hotplug: Add a bounds check to __add_pages
  2019-10-02 22:14     ` Andrew Morton
@ 2019-10-02 23:27       ` Alastair D'Silva
  0 siblings, 0 replies; 6+ messages in thread
From: Alastair D'Silva @ 2019-10-02 23:27 UTC (permalink / raw)
  To: Andrew Morton, David Hildenbrand
  Cc: Oscar Salvador, Michal Hocko, Pavel Tatashin, Dan Williams,
	linux-mm, linux-kernel

On Wed, 2019-10-02 at 15:14 -0700, Andrew Morton wrote:
> On Tue, 1 Oct 2019 11:49:47 +0200 David Hildenbrand <david@redhat.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.
> 

Yes, please ignore that whole series.

> From: Alastair D'Silva <alastair@d-silva.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: 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lkml.kernel.org_r_20190917010752.28395-2D3-2Dalastair-40au1.ibm.com&d=DwICAg&c=jf_iaSHvJObTbx-siA1ZOg&r=cT4tgeEQ0Ll3SIlZDHE5AEXyKy6uKADMtf9_Eb7-vec&m=pVid6q3tQNfU2PQborLw8oYmNm9naF133dZ8AJ5lW9A&s=51ZuQa-kwRu8vW9vt5OgxjaIMWm4_n-aqp5xMSdkI4k&e=
>  
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Acked-by: David Hildenbrand <david@redhat.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Oscar Salvador <osalvador@suse.com>
> Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
> Cc: Qian Cai <cai@lca.pw>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Signed-off-by: Andrew Morton <akpm@linux-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)) {
> _
> 
-- 
Alastair D'Silva
Open Source Developer
Linux Technology Centre, IBM Australia
mob: 0423 762 819



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v7 1/1] memory_hotplug: Add a bounds check to __add_pages
  2019-10-01  0:46 ` [PATCH v7 1/1] memory_hotplug: Add a bounds check to __add_pages Alastair D'Silva
  2019-10-01  9:49   ` David Hildenbrand
@ 2019-10-04 12:08   ` Michal Hocko
  1 sibling, 0 replies; 6+ messages in thread
From: Michal Hocko @ 2019-10-04 12:08 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: alastair, Andrew Morton, Oscar Salvador, David Hildenbrand,
	Pavel Tatashin, Dan Williams, linux-mm, linux-kernel

On Tue 01-10-19 10:46:15, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> On PowerPC, the address ranges allocated to OpenCAPI LPC memory
> are allocated from firmware. These address ranges may be higher
> than what older kernels permit, as we increased the maximum
> permissable address in commit 4ffe713b7587
> ("powerpc/mm: Increase the max addressable memory to 2PB"). It is
> possible that the addressable range may change again in the
> future.
> 
> In this scenario, we end up with a bogus section returned from
> __section_nr (see the discussion on the thread "mm: Trigger bug on
> if a section is not found in __section_nr").
> 
> Adding a check here means that we fail early and have an
> opportunity to handle the error gracefully, rather than rumbling
> on and potentially accessing an incorrect section.
> 
> Further discussion is also on the thread ("powerpc: Perform a bounds
> check in arch_add_memory")
> http://lkml.kernel.org/r/20190827052047.31547-1-alastair@au1.ibm.com
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>

I am sorry to come late to the party. This looks better.
Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!
> ---
>  mm/memory_hotplug.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index c73f09913165..5af9f4466ad1 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -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
> -- 
> 2.21.0

-- 
Michal Hocko
SUSE Labs


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-10-04 12:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-01  0:46 [PATCH v7 0/1] Add bounds check for Hotplugged memory Alastair D'Silva
2019-10-01  0:46 ` [PATCH v7 1/1] memory_hotplug: Add a bounds check to __add_pages Alastair D'Silva
2019-10-01  9:49   ` David Hildenbrand
2019-10-02 22:14     ` Andrew Morton
2019-10-02 23:27       ` Alastair D'Silva
2019-10-04 12:08   ` Michal Hocko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).