All of lore.kernel.org
 help / color / mirror / Atom feed
From: grant.likely@secretlab.ca (Grant Likely)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: kernel: fix nr_cpu_ids check in DT logical map init
Date: Thu, 22 Nov 2012 15:33:36 +0000	[thread overview]
Message-ID: <20121122153337.083D73E129E@localhost> (raw)
In-Reply-To: <20121122120843.GA16972@e102568-lin.cambridge.arm.com>

On Thu, 22 Nov 2012 12:08:43 +0000, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote:
> Hi Russell,
> 
> On Wed, Nov 21, 2012 at 04:42:56PM +0000, Lorenzo Pieralisi wrote:
> > If a kernel is configured with a DT containing more /cpu nodes than
> > nr_cpu_ids, the number of cpus must be capped in the DT parsing
> > code. Current code carries out the check, but fails to cap the
> > value and the check is executed after the cpu logical index is used,
> > which can lead to memory corruption due to index overflow.
> > 
> > This patch refactors the check against nr_cpu_ids and move it before
> > any computed index is used in the parsing code.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Reported-by: Mark Rutland <mark.rutland@arm.com>
> > ---
> > Russell,
> > 
> > while refactoring the DT loop over nodes, I unfortunately missed this niggle
> > in the parsing loop that Mark reported. Here is the fix, sorry for the
> > additional commit, if it is ok for you I will add it to your patch system.
> > 
> > Apologies and thanks,
> > Lorenzo
> > 
> >  arch/arm/kernel/devtree.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> > index aaf9add..70f1bde 100644
> > --- a/arch/arm/kernel/devtree.c
> > +++ b/arch/arm/kernel/devtree.c
> > @@ -139,10 +139,14 @@ void __init arm_dt_init_cpu_maps(void)
> >  			i = cpuidx++;
> >  		}
> >  
> > -		tmp_map[i] = hwid;
> > -
> > -		if (cpuidx > nr_cpu_ids)
> > +		if (WARN(cpuidx > nr_cpu_ids, "DT /cpu %u nodes greater than "
> > +					       "max cores %u, capping them\n",
> > +					       cpuidx, nr_cpu_ids)) {
> > +			cpuidx = nr_cpu_ids;
> >  			break;
> > +		}
> > +
> > +		tmp_map[i] = hwid;
> >  	}
> >  
> >  	if (WARN(!bootcpu_valid, "DT missing boot CPU MPIDR[23:0], "
> 
> If it looks fine to you, can I queue this simple fix in your patch
> system please ?
> 
> Thanks and apologies for the extra commit,
> Lorenzo

Acked-by: Grant Likely <grant.likely@secretlab.ca>

WARNING: multiple messages have this Message-ID (diff)
From: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
To: Lorenzo Pieralisi
	<lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>,
	"linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org"
	<linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: "devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org"
	<devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: [PATCH] ARM: kernel: fix nr_cpu_ids check in DT logical map init
Date: Thu, 22 Nov 2012 15:33:36 +0000	[thread overview]
Message-ID: <20121122153337.083D73E129E@localhost> (raw)
In-Reply-To: <20121122120843.GA16972-7AyDDHkRsp3ZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>

On Thu, 22 Nov 2012 12:08:43 +0000, Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org> wrote:
> Hi Russell,
> 
> On Wed, Nov 21, 2012 at 04:42:56PM +0000, Lorenzo Pieralisi wrote:
> > If a kernel is configured with a DT containing more /cpu nodes than
> > nr_cpu_ids, the number of cpus must be capped in the DT parsing
> > code. Current code carries out the check, but fails to cap the
> > value and the check is executed after the cpu logical index is used,
> > which can lead to memory corruption due to index overflow.
> > 
> > This patch refactors the check against nr_cpu_ids and move it before
> > any computed index is used in the parsing code.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi-5wv7dgnIgG8@public.gmane.org>
> > Reported-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
> > ---
> > Russell,
> > 
> > while refactoring the DT loop over nodes, I unfortunately missed this niggle
> > in the parsing loop that Mark reported. Here is the fix, sorry for the
> > additional commit, if it is ok for you I will add it to your patch system.
> > 
> > Apologies and thanks,
> > Lorenzo
> > 
> >  arch/arm/kernel/devtree.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> > index aaf9add..70f1bde 100644
> > --- a/arch/arm/kernel/devtree.c
> > +++ b/arch/arm/kernel/devtree.c
> > @@ -139,10 +139,14 @@ void __init arm_dt_init_cpu_maps(void)
> >  			i = cpuidx++;
> >  		}
> >  
> > -		tmp_map[i] = hwid;
> > -
> > -		if (cpuidx > nr_cpu_ids)
> > +		if (WARN(cpuidx > nr_cpu_ids, "DT /cpu %u nodes greater than "
> > +					       "max cores %u, capping them\n",
> > +					       cpuidx, nr_cpu_ids)) {
> > +			cpuidx = nr_cpu_ids;
> >  			break;
> > +		}
> > +
> > +		tmp_map[i] = hwid;
> >  	}
> >  
> >  	if (WARN(!bootcpu_valid, "DT missing boot CPU MPIDR[23:0], "
> 
> If it looks fine to you, can I queue this simple fix in your patch
> system please ?
> 
> Thanks and apologies for the extra commit,
> Lorenzo

Acked-by: Grant Likely <grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>

  reply	other threads:[~2012-11-22 15:33 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-21 16:42 [PATCH] ARM: kernel: fix nr_cpu_ids check in DT logical map init Lorenzo Pieralisi
2012-11-21 16:42 ` Lorenzo Pieralisi
2012-11-22 12:08 ` Lorenzo Pieralisi
2012-11-22 12:08   ` Lorenzo Pieralisi
2012-11-22 15:33   ` Grant Likely [this message]
2012-11-22 15:33     ` Grant Likely
2012-11-22 17:05     ` Lorenzo Pieralisi
2012-11-22 17:05       ` Lorenzo Pieralisi

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=20121122153337.083D73E129E@localhost \
    --to=grant.likely@secretlab.ca \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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.