All of lore.kernel.org
 help / color / mirror / Atom feed
* acpi: Fix broken error check in map_processor()
@ 2016-09-23 15:08 ` Thomas Gleixner
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Gleixner @ 2016-09-23 15:08 UTC (permalink / raw)
  To: Dou Liyang
  Cc: cl, tj, mika.j.penttila, mingo, akpm, rjw, hpa, yasu.isimatu,
	isimatu.yasuaki, kamezawa.hiroyu, izumi.taku, gongzhaogang,
	len.brown, lenb, chen.tang, rafael, x86, linux-acpi,
	linux-kernel, linux-mm, Gu Zheng, Tang Chen, Zhu Guihua

map_processor() checks the cpuid value returned by acpi_map_cpuid() for -1
but acpi_map_cpuid() returns -EINVAL in case of error.

As a consequence the error is ignored and the following access into percpu
data with that negative cpuid results in a boot crash.

This happens always when NR_CPUS/nr_cpu_ids is smaller than the number of
processors listed in the ACPI tables.

Use a proper error check for id < 0 so the function returns instead of
trying to map CPU#(-EINVAL).

Reported-by: Ingo Molnar <mingo@kernel.org>
Fixes: dc6db24d2476 ("x86/acpi: Set persistent cpuid <-> nodeid mapping when booting")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/acpi/processor_core.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -284,7 +284,7 @@ EXPORT_SYMBOL_GPL(acpi_get_cpuid);
 static bool __init
 map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid)
 {
-	int type;
+	int type, id;
 	u32 acpi_id;
 	acpi_status status;
 	acpi_object_type acpi_type;
@@ -320,10 +320,11 @@ map_processor(acpi_handle handle, phys_c
 	type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
 
 	*phys_id = __acpi_get_phys_id(handle, type, acpi_id, false);
-	*cpuid = acpi_map_cpuid(*phys_id, acpi_id);
-	if (*cpuid == -1)
-		return false;
+	id = acpi_map_cpuid(*phys_id, acpi_id);
 
+	if (id < 0)
+		return false;
+	*cpuid = id;
 	return true;
 }
 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* acpi: Fix broken error check in map_processor()
@ 2016-09-23 15:08 ` Thomas Gleixner
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Gleixner @ 2016-09-23 15:08 UTC (permalink / raw)
  To: Dou Liyang
  Cc: cl, tj, mika.j.penttila, mingo, akpm, rjw, hpa, yasu.isimatu,
	isimatu.yasuaki, kamezawa.hiroyu, izumi.taku, gongzhaogang,
	len.brown, lenb, chen.tang, rafael, x86, linux-acpi,
	linux-kernel, linux-mm, Gu Zheng, Tang Chen, Zhu Guihua

map_processor() checks the cpuid value returned by acpi_map_cpuid() for -1
but acpi_map_cpuid() returns -EINVAL in case of error.

As a consequence the error is ignored and the following access into percpu
data with that negative cpuid results in a boot crash.

This happens always when NR_CPUS/nr_cpu_ids is smaller than the number of
processors listed in the ACPI tables.

Use a proper error check for id < 0 so the function returns instead of
trying to map CPU#(-EINVAL).

Reported-by: Ingo Molnar <mingo@kernel.org>
Fixes: dc6db24d2476 ("x86/acpi: Set persistent cpuid <-> nodeid mapping when booting")
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/acpi/processor_core.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -284,7 +284,7 @@ EXPORT_SYMBOL_GPL(acpi_get_cpuid);
 static bool __init
 map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid)
 {
-	int type;
+	int type, id;
 	u32 acpi_id;
 	acpi_status status;
 	acpi_object_type acpi_type;
@@ -320,10 +320,11 @@ map_processor(acpi_handle handle, phys_c
 	type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
 
 	*phys_id = __acpi_get_phys_id(handle, type, acpi_id, false);
-	*cpuid = acpi_map_cpuid(*phys_id, acpi_id);
-	if (*cpuid == -1)
-		return false;
+	id = acpi_map_cpuid(*phys_id, acpi_id);
 
+	if (id < 0)
+		return false;
+	*cpuid = id;
 	return true;
 }
 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [tip:x86/apic] acpi: Fix broken error check in map_processor()
  2016-09-23 15:08 ` Thomas Gleixner
  (?)
@ 2016-09-23 18:54 ` tip-bot for Thomas Gleixner
  -1 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Thomas Gleixner @ 2016-09-23 18:54 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: zhugh.fnst, peterz, tglx, tangchen, torvalds, linux-kernel, hpa,
	mingo, guz.fnst, douly.fnst

Commit-ID:  c183a603e8d8a5a189729b77d0c623a3d5950e5f
Gitweb:     http://git.kernel.org/tip/c183a603e8d8a5a189729b77d0c623a3d5950e5f
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Fri, 23 Sep 2016 17:08:04 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 23 Sep 2016 18:04:56 +0200

acpi: Fix broken error check in map_processor()

map_processor() checks the cpuid value returned by acpi_map_cpuid() for -1
but acpi_map_cpuid() returns -EINVAL in case of error.

As a consequence the error is ignored and the following access into percpu
data with that negative cpuid results in a boot crash.

This happens always when NR_CPUS/nr_cpu_ids is smaller than the number of
processors listed in the ACPI tables.

Use a proper error check for id < 0 so the function returns instead of
trying to map CPU#(-EINVAL).

Reported-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Dou Liyang <douly.fnst@cn.fujitsu.com>
Cc: Gu Zheng <guz.fnst@cn.fujitsu.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tang Chen <tangchen@cn.fujitsu.com>
Cc: Zhu Guihua <zhugh.fnst@cn.fujitsu.com>
Cc: akpm@linux-foundation.org
Cc: chen.tang@easystack.cn
Cc: cl@linux.com
Cc: gongzhaogang@inspur.com
Cc: isimatu.yasuaki@jp.fujitsu.com
Cc: izumi.taku@jp.fujitsu.com
Cc: kamezawa.hiroyu@jp.fujitsu.com
Cc: len.brown@intel.com
Cc: lenb@kernel.org
Cc: linux-acpi@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: mika.j.penttila@gmail.com
Cc: rafael@kernel.org
Cc: rjw@rjwysocki.net
Cc: tj@kernel.org
Cc: yasu.isimatu@gmail.com
Fixes: dc6db24d2476 ("x86/acpi: Set persistent cpuid <-> nodeid mapping when booting")
Link: http://lkml.kernel.org/r/alpine.DEB.2.20.1609231705570.5640@nanos
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/acpi/processor_core.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 9ac265f..5c78ee1 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -284,7 +284,7 @@ EXPORT_SYMBOL_GPL(acpi_get_cpuid);
 static bool __init
 map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid)
 {
-	int type;
+	int type, id;
 	u32 acpi_id;
 	acpi_status status;
 	acpi_object_type acpi_type;
@@ -320,10 +320,11 @@ map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid)
 	type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
 
 	*phys_id = __acpi_get_phys_id(handle, type, acpi_id, false);
-	*cpuid = acpi_map_cpuid(*phys_id, acpi_id);
-	if (*cpuid == -1)
-		return false;
+	id = acpi_map_cpuid(*phys_id, acpi_id);
 
+	if (id < 0)
+		return false;
+	*cpuid = id;
 	return true;
 }
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: acpi: Fix broken error check in map_processor()
  2016-09-23 15:08 ` Thomas Gleixner
  (?)
@ 2016-09-24  0:06   ` Rafael J. Wysocki
  -1 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2016-09-24  0:06 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Dou Liyang, cl, Tejun Heo, mika.j.penttila, Ingo Molnar,
	Andrew Morton, Rafael J. Wysocki, H. Peter Anvin, yasu.isimatu,
	isimatu.yasuaki, kamezawa.hiroyu, izumi.taku, gongzhaogang,
	Len Brown, Len Brown, chen.tang, Rafael J. Wysocki,
	the arch/x86 maintainers, ACPI Devel Maling List,
	Linux Kernel Mailing List, Linux Memory Management List,
	Gu Zheng, Tang

On Fri, Sep 23, 2016 at 5:08 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> map_processor() checks the cpuid value returned by acpi_map_cpuid() for -1
> but acpi_map_cpuid() returns -EINVAL in case of error.
>
> As a consequence the error is ignored and the following access into percpu
> data with that negative cpuid results in a boot crash.
>
> This happens always when NR_CPUS/nr_cpu_ids is smaller than the number of
> processors listed in the ACPI tables.
>
> Use a proper error check for id < 0 so the function returns instead of
> trying to map CPU#(-EINVAL).
>
> Reported-by: Ingo Molnar <mingo@kernel.org>
> Fixes: dc6db24d2476 ("x86/acpi: Set persistent cpuid <-> nodeid mapping when booting")
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

It looks like the commit in the Fixes tag is in the tip tree now, so
the fix should better go in via tip as well IMO.

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/acpi/processor_core.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -284,7 +284,7 @@ EXPORT_SYMBOL_GPL(acpi_get_cpuid);
>  static bool __init
>  map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid)
>  {
> -       int type;
> +       int type, id;
>         u32 acpi_id;
>         acpi_status status;
>         acpi_object_type acpi_type;
> @@ -320,10 +320,11 @@ map_processor(acpi_handle handle, phys_c
>         type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
>
>         *phys_id = __acpi_get_phys_id(handle, type, acpi_id, false);
> -       *cpuid = acpi_map_cpuid(*phys_id, acpi_id);
> -       if (*cpuid == -1)
> -               return false;
> +       id = acpi_map_cpuid(*phys_id, acpi_id);
>
> +       if (id < 0)
> +               return false;
> +       *cpuid = id;
>         return true;
>  }
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: acpi: Fix broken error check in map_processor()
@ 2016-09-24  0:06   ` Rafael J. Wysocki
  0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2016-09-24  0:06 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Dou Liyang, cl, Tejun Heo, mika.j.penttila, Ingo Molnar,
	Andrew Morton, Rafael J. Wysocki, H. Peter Anvin, yasu.isimatu,
	isimatu.yasuaki, kamezawa.hiroyu, izumi.taku, gongzhaogang,
	Len Brown, Len Brown, chen.tang, Rafael J. Wysocki,
	the arch/x86 maintainers, ACPI Devel Maling List,
	Linux Kernel Mailing List, Linux Memory Management List,
	Gu Zheng, Tang Chen, Zhu Guihua

On Fri, Sep 23, 2016 at 5:08 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> map_processor() checks the cpuid value returned by acpi_map_cpuid() for -1
> but acpi_map_cpuid() returns -EINVAL in case of error.
>
> As a consequence the error is ignored and the following access into percpu
> data with that negative cpuid results in a boot crash.
>
> This happens always when NR_CPUS/nr_cpu_ids is smaller than the number of
> processors listed in the ACPI tables.
>
> Use a proper error check for id < 0 so the function returns instead of
> trying to map CPU#(-EINVAL).
>
> Reported-by: Ingo Molnar <mingo@kernel.org>
> Fixes: dc6db24d2476 ("x86/acpi: Set persistent cpuid <-> nodeid mapping when booting")
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

It looks like the commit in the Fixes tag is in the tip tree now, so
the fix should better go in via tip as well IMO.

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/acpi/processor_core.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -284,7 +284,7 @@ EXPORT_SYMBOL_GPL(acpi_get_cpuid);
>  static bool __init
>  map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid)
>  {
> -       int type;
> +       int type, id;
>         u32 acpi_id;
>         acpi_status status;
>         acpi_object_type acpi_type;
> @@ -320,10 +320,11 @@ map_processor(acpi_handle handle, phys_c
>         type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
>
>         *phys_id = __acpi_get_phys_id(handle, type, acpi_id, false);
> -       *cpuid = acpi_map_cpuid(*phys_id, acpi_id);
> -       if (*cpuid == -1)
> -               return false;
> +       id = acpi_map_cpuid(*phys_id, acpi_id);
>
> +       if (id < 0)
> +               return false;
> +       *cpuid = id;
>         return true;
>  }
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: acpi: Fix broken error check in map_processor()
@ 2016-09-24  0:06   ` Rafael J. Wysocki
  0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2016-09-24  0:06 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Dou Liyang, cl, Tejun Heo, mika.j.penttila, Ingo Molnar,
	Andrew Morton, Rafael J. Wysocki, H. Peter Anvin, yasu.isimatu,
	isimatu.yasuaki, kamezawa.hiroyu, izumi.taku, gongzhaogang,
	Len Brown, Len Brown, chen.tang, Rafael J. Wysocki,
	the arch/x86 maintainers, ACPI Devel Maling List,
	Linux Kernel Mailing List, Linux Memory Management List,
	Gu Zheng, Tang Chen, Zhu Guihua

On Fri, Sep 23, 2016 at 5:08 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> map_processor() checks the cpuid value returned by acpi_map_cpuid() for -1
> but acpi_map_cpuid() returns -EINVAL in case of error.
>
> As a consequence the error is ignored and the following access into percpu
> data with that negative cpuid results in a boot crash.
>
> This happens always when NR_CPUS/nr_cpu_ids is smaller than the number of
> processors listed in the ACPI tables.
>
> Use a proper error check for id < 0 so the function returns instead of
> trying to map CPU#(-EINVAL).
>
> Reported-by: Ingo Molnar <mingo@kernel.org>
> Fixes: dc6db24d2476 ("x86/acpi: Set persistent cpuid <-> nodeid mapping when booting")
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

It looks like the commit in the Fixes tag is in the tip tree now, so
the fix should better go in via tip as well IMO.

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/acpi/processor_core.c |    9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> --- a/drivers/acpi/processor_core.c
> +++ b/drivers/acpi/processor_core.c
> @@ -284,7 +284,7 @@ EXPORT_SYMBOL_GPL(acpi_get_cpuid);
>  static bool __init
>  map_processor(acpi_handle handle, phys_cpuid_t *phys_id, int *cpuid)
>  {
> -       int type;
> +       int type, id;
>         u32 acpi_id;
>         acpi_status status;
>         acpi_object_type acpi_type;
> @@ -320,10 +320,11 @@ map_processor(acpi_handle handle, phys_c
>         type = (acpi_type == ACPI_TYPE_DEVICE) ? 1 : 0;
>
>         *phys_id = __acpi_get_phys_id(handle, type, acpi_id, false);
> -       *cpuid = acpi_map_cpuid(*phys_id, acpi_id);
> -       if (*cpuid == -1)
> -               return false;
> +       id = acpi_map_cpuid(*phys_id, acpi_id);
>
> +       if (id < 0)
> +               return false;
> +       *cpuid = id;
>         return true;
>  }
>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-24  0:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-23 15:08 acpi: Fix broken error check in map_processor() Thomas Gleixner
2016-09-23 15:08 ` Thomas Gleixner
2016-09-23 18:54 ` [tip:x86/apic] " tip-bot for Thomas Gleixner
2016-09-24  0:06 ` Rafael J. Wysocki
2016-09-24  0:06   ` Rafael J. Wysocki
2016-09-24  0:06   ` Rafael J. Wysocki

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.