[PATCH 1/2] perf_event: Fix incorrect range check on cpu number

[PATCH 1/2] perf_event: Fix incorrect range check on cpu number

[Paul Mackerras]

It is quite legitimate for CPUs to be numbered sparsely, meaning that
it possible for an online CPU to have a number which is greater than
the total count of possible CPUs.

Currently find_get_context() has a sanity check on the cpu number
where it checks it against num_possible_cpus().  This test can fail
for a legitimate cpu number if the cpu_possible_mask is sparsely
populated.

This fixes the problem by checking the CPU number against
nr_cpumask_bits instead, since that is the appropriate check to ensure
that the cpu number is same to pass to cpu_isset() subsequently.

Reported-by: Michael Neuling <mikey@neuling.org>
Tested-by: Michael Neuling <mikey@neuling.org>
Cc: stable@kernel.org
Signed-off-by: Paul Mackerras <paulus@samba.org>
---
 kernel/perf_event.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index 6b7ddba..78551b3 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -1604,7 +1604,7 @@ static struct perf_event_context *find_get_context(pid_t pid, int cpu)
 		if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
 			return ERR_PTR(-EACCES);
 
-		if (cpu < 0 || cpu > num_possible_cpus())
+		if (cpu < 0 || cpu >= nr_cpumask_bits)
 			return ERR_PTR(-EINVAL);
 
 		/*

Re: [PATCH 1/2] perf_event: Fix incorrect range check on cpu number

[Peter Zijlstra]

On Tue, 2009-12-15 at 19:40 +1100, Paul Mackerras wrote:
> It is quite legitimate for CPUs to be numbered sparsely, meaning that
> it possible for an online CPU to have a number which is greater than
> the total count of possible CPUs.
> 
> Currently find_get_context() has a sanity check on the cpu number
> where it checks it against num_possible_cpus().  This test can fail
> for a legitimate cpu number if the cpu_possible_mask is sparsely
> populated.
> 
> This fixes the problem by checking the CPU number against
> nr_cpumask_bits instead, since that is the appropriate check to ensure
> that the cpu number is same to pass to cpu_isset() subsequently.

Cute, do you actually have hardware that does this?

> Reported-by: Michael Neuling <mikey@neuling.org>
> Tested-by: Michael Neuling <mikey@neuling.org>
> Cc: stable@kernel.org
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
>  kernel/perf_event.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/perf_event.c b/kernel/perf_event.c
> index 6b7ddba..78551b3 100644
> --- a/kernel/perf_event.c
> +++ b/kernel/perf_event.c
> @@ -1604,7 +1604,7 @@ static struct perf_event_context *find_get_context(pid_t pid, int cpu)
>  		if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
>  			return ERR_PTR(-EACCES);
>  
> -		if (cpu < 0 || cpu > num_possible_cpus())
> +		if (cpu < 0 || cpu >= nr_cpumask_bits)
>  			return ERR_PTR(-EINVAL);
>  
>  		/*

Re: [PATCH 1/2] perf_event: Fix incorrect range check on cpu number

[Paul Mackerras]

On Tue, Dec 15, 2009 at 11:31:32AM +0100, Peter Zijlstra wrote:

> On Tue, 2009-12-15 at 19:40 +1100, Paul Mackerras wrote:
> > It is quite legitimate for CPUs to be numbered sparsely, meaning that
> > it possible for an online CPU to have a number which is greater than
> > the total count of possible CPUs.
> > 
> > Currently find_get_context() has a sanity check on the cpu number
> > where it checks it against num_possible_cpus().  This test can fail
> > for a legitimate cpu number if the cpu_possible_mask is sparsely
> > populated.
> > 
> > This fixes the problem by checking the CPU number against
> > nr_cpumask_bits instead, since that is the appropriate check to ensure
> > that the cpu number is same to pass to cpu_isset() subsequently.
> 
> Cute, do you actually have hardware that does this?

Yeah, Mikey ran across this on a POWER7 box here.

Paul.

Re: [PATCH 1/2] perf_event: Fix incorrect range check on cpu number

[Corey Ashford]

Paul Mackerras wrote:
> On Tue, Dec 15, 2009 at 11:31:32AM +0100, Peter Zijlstra wrote:
> 
>> On Tue, 2009-12-15 at 19:40 +1100, Paul Mackerras wrote:
>>> It is quite legitimate for CPUs to be numbered sparsely, meaning that
>>> it possible for an online CPU to have a number which is greater than
>>> the total count of possible CPUs.
>>>
>>> Currently find_get_context() has a sanity check on the cpu number
>>> where it checks it against num_possible_cpus().  This test can fail
>>> for a legitimate cpu number if the cpu_possible_mask is sparsely
>>> populated.
>>>
>>> This fixes the problem by checking the CPU number against
>>> nr_cpumask_bits instead, since that is the appropriate check to ensure
>>> that the cpu number is same to pass to cpu_isset() subsequently.
>> Cute, do you actually have hardware that does this?
> 
> Yeah, Mikey ran across this on a POWER7 box here.

Does the perf tool need to be fixed too?  The "perf stat" tool, at least, has a 
"-a" switch that tells the tool to count the event on all cpus, and it does this 
by iterating over the number of cpus, 0..n, assuming they are all contiguous.

- Corey

Re: [PATCH 1/2] perf_event: Fix incorrect range check on cpu number

[Ingo Molnar]

* Corey Ashford <cjashfor@linux.vnet.ibm.com> wrote:

> 
> Paul Mackerras wrote:
> >On Tue, Dec 15, 2009 at 11:31:32AM +0100, Peter Zijlstra wrote:
> >
> >>On Tue, 2009-12-15 at 19:40 +1100, Paul Mackerras wrote:
> >>>It is quite legitimate for CPUs to be numbered sparsely, meaning that
> >>>it possible for an online CPU to have a number which is greater than
> >>>the total count of possible CPUs.
> >>>
> >>>Currently find_get_context() has a sanity check on the cpu number
> >>>where it checks it against num_possible_cpus().  This test can fail
> >>>for a legitimate cpu number if the cpu_possible_mask is sparsely
> >>>populated.
> >>>
> >>>This fixes the problem by checking the CPU number against
> >>>nr_cpumask_bits instead, since that is the appropriate check to ensure
> >>>that the cpu number is same to pass to cpu_isset() subsequently.
> >>Cute, do you actually have hardware that does this?
> >
> >Yeah, Mikey ran across this on a POWER7 box here.
> 
> Does the perf tool need to be fixed too?  The "perf stat" tool, at
> least, has a "-a" switch that tells the tool to count the event on
> all cpus, and it does this by iterating over the number of cpus,
> 0..n, assuming they are all contiguous.

Yes, see patch 2/2 of this series.

	Ingo

Re: [PATCH 1/2] perf_event: Fix incorrect range check on cpu number

[Corey Ashford]

Ingo Molnar wrote:
> * Corey Ashford <cjashfor@linux.vnet.ibm.com> wrote:
> 
>> Paul Mackerras wrote:
>>> On Tue, Dec 15, 2009 at 11:31:32AM +0100, Peter Zijlstra wrote:
>>>
>>>> On Tue, 2009-12-15 at 19:40 +1100, Paul Mackerras wrote:
>>>>> It is quite legitimate for CPUs to be numbered sparsely, meaning that
>>>>> it possible for an online CPU to have a number which is greater than
>>>>> the total count of possible CPUs.
>>>>>
>>>>> Currently find_get_context() has a sanity check on the cpu number
>>>>> where it checks it against num_possible_cpus().  This test can fail
>>>>> for a legitimate cpu number if the cpu_possible_mask is sparsely
>>>>> populated.
>>>>>
>>>>> This fixes the problem by checking the CPU number against
>>>>> nr_cpumask_bits instead, since that is the appropriate check to ensure
>>>>> that the cpu number is same to pass to cpu_isset() subsequently.
>>>> Cute, do you actually have hardware that does this?
>>> Yeah, Mikey ran across this on a POWER7 box here.
>> Does the perf tool need to be fixed too?  The "perf stat" tool, at
>> least, has a "-a" switch that tells the tool to count the event on
>> all cpus, and it does this by iterating over the number of cpus,
>> 0..n, assuming they are all contiguous.
> 
> Yes, see patch 2/2 of this series.
> 
> 	Ingo

Oops! missed that.  Thanks!

- Corey

[tip:perf/urgent] perf_event: Fix incorrect range check on cpu number

[tip-bot for Paul Mackerras]

Commit-ID:  0f624e7e5625f4c30c836b7a5decfe2553582391
Gitweb:     http://git.kernel.org/tip/0f624e7e5625f4c30c836b7a5decfe2553582391
Author:     Paul Mackerras <paulus@samba.org>
AuthorDate: Tue, 15 Dec 2009 19:40:32 +1100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 15 Dec 2009 13:09:55 +0100

perf_event: Fix incorrect range check on cpu number

It is quite legitimate for CPUs to be numbered sparsely, meaning
that it possible for an online CPU to have a number which is
greater than the total count of possible CPUs.

Currently find_get_context() has a sanity check on the cpu
number where it checks it against num_possible_cpus().  This
test can fail for a legitimate cpu number if the
cpu_possible_mask is sparsely populated.

This fixes the problem by checking the CPU number against
nr_cpumask_bits instead, since that is the appropriate check to
ensure that the cpu number is same to pass to cpu_isset()
subsequently.

Reported-by: Michael Neuling <mikey@neuling.org>
Signed-off-by: Paul Mackerras <paulus@samba.org>
Tested-by: Michael Neuling <mikey@neuling.org>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Cc: <stable@kernel.org>
LKML-Reference: <20091215084032.GA18661@brick.ozlabs.ibm.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/perf_event.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/perf_event.c b/kernel/perf_event.c
index d891ec4..8823b08 100644
--- a/kernel/perf_event.c
+++ b/kernel/perf_event.c
@@ -1603,7 +1603,7 @@ static struct perf_event_context *find_get_context(pid_t pid, int cpu)
 		if (perf_paranoid_cpu() && !capable(CAP_SYS_ADMIN))
 			return ERR_PTR(-EACCES);
 
-		if (cpu < 0 || cpu > num_possible_cpus())
+		if (cpu < 0 || cpu >= nr_cpumask_bits)
 			return ERR_PTR(-EINVAL);
 
 		/*