All of lore.kernel.org
 help / color / mirror / Atom feed
From: Baoquan He <bhe@redhat.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Cc: Sachin Sant <sachinp@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	akpm@linux-foundation.org, linuxppc-dev@lists.ozlabs.org,
	dan.j.williams@intel.com
Subject: Re: [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check
Date: Wed, 25 Mar 2020 15:37:07 +0800	[thread overview]
Message-ID: <20200325073707.GI3039@MiWiFi-R3L-srv> (raw)
In-Reply-To: <20200325070643.GH3039@MiWiFi-R3L-srv>

On 03/25/20 at 03:06pm, Baoquan He wrote:
> On 03/25/20 at 08:49am, Aneesh Kumar K.V wrote:

> >  mm/sparse.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/mm/sparse.c b/mm/sparse.c
> > index aadb7298dcef..3012d1f3771a 100644
> > --- a/mm/sparse.c
> > +++ b/mm/sparse.c
> > @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> >  			ms->usage = NULL;
> >  		}
> >  		memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> > +		/* Mark the section invalid */
> > +		ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
> 
> Not sure if we should add checking in valid_section() or pfn_valid(),
> e.g check ms->usage validation too. Otherwise, this fix looks good to
> me.

With SPASEMEM_VMEMAP enabled, we should do validation check on ms->usage
before checking any subsection is valid. Since now we do have case
in which ms->usage is released, people still try to check it.

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index f0a2c184eb9a..d79bd938852e 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1306,6 +1306,8 @@ static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
 {
 	int idx = subsection_map_index(pfn);
 
+	if (!ms->usage)
+		return 0;
 	return test_bit(idx, ms->usage->subsection_map);
 }
 #else


WARNING: multiple messages have this Message-ID (diff)
From: Baoquan He <bhe@redhat.com>
To: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Cc: linux-mm@kvack.org, akpm@linux-foundation.org,
	linux-kernel@vger.kernel.org, mpe@ellerman.id.au,
	linuxppc-dev@lists.ozlabs.org,
	Sachin Sant <sachinp@linux.vnet.ibm.com>,
	dan.j.williams@intel.com
Subject: Re: [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check
Date: Wed, 25 Mar 2020 15:37:07 +0800	[thread overview]
Message-ID: <20200325073707.GI3039@MiWiFi-R3L-srv> (raw)
In-Reply-To: <20200325070643.GH3039@MiWiFi-R3L-srv>

On 03/25/20 at 03:06pm, Baoquan He wrote:
> On 03/25/20 at 08:49am, Aneesh Kumar K.V wrote:

> >  mm/sparse.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/mm/sparse.c b/mm/sparse.c
> > index aadb7298dcef..3012d1f3771a 100644
> > --- a/mm/sparse.c
> > +++ b/mm/sparse.c
> > @@ -781,6 +781,8 @@ static void section_deactivate(unsigned long pfn, unsigned long nr_pages,
> >  			ms->usage = NULL;
> >  		}
> >  		memmap = sparse_decode_mem_map(ms->section_mem_map, section_nr);
> > +		/* Mark the section invalid */
> > +		ms->section_mem_map &= ~SECTION_HAS_MEM_MAP;
> 
> Not sure if we should add checking in valid_section() or pfn_valid(),
> e.g check ms->usage validation too. Otherwise, this fix looks good to
> me.

With SPASEMEM_VMEMAP enabled, we should do validation check on ms->usage
before checking any subsection is valid. Since now we do have case
in which ms->usage is released, people still try to check it.

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index f0a2c184eb9a..d79bd938852e 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1306,6 +1306,8 @@ static inline int pfn_section_valid(struct mem_section *ms, unsigned long pfn)
 {
 	int idx = subsection_map_index(pfn);
 
+	if (!ms->usage)
+		return 0;
 	return test_bit(idx, ms->usage->subsection_map);
 }
 #else



  reply	other threads:[~2020-03-25  7:39 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25  3:19 [PATCH] mm/sparse: Fix kernel crash with pfn_section_valid check Aneesh Kumar K.V
2020-03-25  3:19 ` Aneesh Kumar K.V
2020-03-25  6:50 ` Aneesh Kumar K.V
2020-03-25  7:06 ` Baoquan He
2020-03-25  7:06   ` Baoquan He
2020-03-25  7:37   ` Baoquan He [this message]
2020-03-25  7:37     ` Baoquan He
2020-03-25  8:12     ` Aneesh Kumar K.V
2020-03-25  8:12       ` Aneesh Kumar K.V
2020-03-25  8:36       ` Baoquan He
2020-03-25  8:36         ` Baoquan He
2020-03-26  0:38 ` Andrew Morton
2020-03-26  0:38   ` Andrew Morton
2020-03-26  9:40 ` Michal Hocko
2020-03-26  9:40   ` Michal Hocko
2020-03-26  9:56   ` Aneesh Kumar K.V
2020-03-26  9:56     ` Aneesh Kumar K.V
2020-03-26 10:16     ` Michal Hocko
2020-03-26 10:16       ` Michal Hocko
2020-03-26 10:50       ` Michal Hocko
2020-03-26 10:50         ` Michal Hocko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200325073707.GI3039@MiWiFi-R3L-srv \
    --to=bhe@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=aneesh.kumar@linux.ibm.com \
    --cc=dan.j.williams@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=sachinp@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.