* 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;
> }
>
--
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* 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