* Re: [RESEND PATCH] ARM: prevent tracing IPI_CPU_BACKTRACE
2018-09-27 3:41 [RESEND PATCH] ARM: prevent tracing IPI_CPU_BACKTRACE Chunyan Zhang
@ 2018-10-09 2:20 ` Chunyan Zhang
2018-10-18 3:32 ` Chunyan Zhang
2018-10-18 8:23 ` [PATCH] " Chunyan Zhang
2 siblings, 0 replies; 5+ messages in thread
From: Chunyan Zhang @ 2018-10-09 2:20 UTC (permalink / raw)
To: Chunyan Zhang
Cc: Russell King - ARM Linux, Marc Zyngier, Arnd Bergmann,
Linux Kernel Mailing List, Linux ARM
Hi All,
If there's no comments, should I submit this patch on RMK's Patch system?
Thanks,
Chunyan
On 27 September 2018 at 11:41, Chunyan Zhang <chunyan.zhang@unisoc.com> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> When function tracing for IPIs is enabled, we get a warning for an
> overflow of the ipi_types array with the IPI_CPU_BACKTRACE type
> as triggered by raise_nmi():
>
> arch/arm/kernel/smp.c: In function 'raise_nmi':
> arch/arm/kernel/smp.c:489:2: error: array subscript is above array bounds [-Werror=array-bounds]
> trace_ipi_raise(target, ipi_types[ipinr]);
>
> This is a correct warning as we actually overflow the array here.
>
> This patch raise_nmi() to call __smp_cross_call() instead of
> smp_cross_call(), to avoid calling into ftrace. For clarification,
> I'm also adding a two new code comments describing how this one
> is special.
>
> The warning appears to have shown up after patch e7273ff49acf
> ("ARM: 8488/1: Make IPI_CPU_BACKTRACE a "non-secure" SGI"), which
> changed the number assignment from '15' to '8', but as far as I can
> tell has existed since the IPI tracepoints were first introduced.
> If we decide to backport this patch to stable kernels, we probably
> need to backport e7273ff49acf as well.
>
> Resubmiting this patch is because that I found coverity is complaining
> the issue this patch fixed, and also I got the traces like below:
> "ipi_raise: target_mask=00000001 (machine_suspend)" which actually was
> the TPS of suspend_resume[1] rather than ipi_raise.
>
> [1]
> https://elixir.bootlin.com/linux/latest/source/kernel/power/suspend.c#L80
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: e7273ff49acf ("ARM: 8488/1: Make IPI_CPU_BACKTRACE a "non-secure" SGI")
> Fixes: 365ec7b17327 ("ARM: add IPI tracepoints") # v3.17
> Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> ---
> arch/arm/include/asm/hardirq.h | 1 +
> arch/arm/kernel/smp.c | 6 +++++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
> index cba23ea..7a88f16 100644
> --- a/arch/arm/include/asm/hardirq.h
> +++ b/arch/arm/include/asm/hardirq.h
> @@ -6,6 +6,7 @@
> #include <linux/threads.h>
> #include <asm/irq.h>
>
> +/* number of IPIS _not_ including IPI_CPU_BACKTRACE */
> #define NR_IPI 7
>
> typedef struct {
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 0978282..123be77 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -75,6 +75,10 @@ enum ipi_msg_type {
> IPI_CPU_STOP,
> IPI_IRQ_WORK,
> IPI_COMPLETION,
> + /*
> + * CPU_BACKTRACE is special and not included in NR_IPI
> + * or tracable with trace_ipi_*
> + */
> IPI_CPU_BACKTRACE,
> /*
> * SGI8-15 can be reserved by secure firmware, and thus may
> @@ -755,7 +759,7 @@ core_initcall(register_cpufreq_notifier);
>
> static void raise_nmi(cpumask_t *mask)
> {
> - smp_cross_call(mask, IPI_CPU_BACKTRACE);
> + _smp_cross_call(mask, IPI_CPU_BACKTRACE);
> }
>
> void arch_trigger_cpumask_backtrace(const cpumask_t *mask, bool exclude_self)
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [RESEND PATCH] ARM: prevent tracing IPI_CPU_BACKTRACE
2018-09-27 3:41 [RESEND PATCH] ARM: prevent tracing IPI_CPU_BACKTRACE Chunyan Zhang
2018-10-09 2:20 ` Chunyan Zhang
@ 2018-10-18 3:32 ` Chunyan Zhang
2018-10-18 8:23 ` [PATCH] " Chunyan Zhang
2 siblings, 0 replies; 5+ messages in thread
From: Chunyan Zhang @ 2018-10-18 3:32 UTC (permalink / raw)
To: Chunyan Zhang
Cc: Russell King - ARM Linux, Marc Zyngier, Arnd Bergmann,
Linux Kernel Mailing List, Linux ARM
On Thu, 27 Sep 2018 at 11:42, Chunyan Zhang <chunyan.zhang@unisoc.com> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> When function tracing for IPIs is enabled, we get a warning for an
> overflow of the ipi_types array with the IPI_CPU_BACKTRACE type
> as triggered by raise_nmi():
>
> arch/arm/kernel/smp.c: In function 'raise_nmi':
> arch/arm/kernel/smp.c:489:2: error: array subscript is above array bounds [-Werror=array-bounds]
> trace_ipi_raise(target, ipi_types[ipinr]);
>
> This is a correct warning as we actually overflow the array here.
>
> This patch raise_nmi() to call __smp_cross_call() instead of
> smp_cross_call(), to avoid calling into ftrace. For clarification,
> I'm also adding a two new code comments describing how this one
> is special.
>
> The warning appears to have shown up after patch e7273ff49acf
> ("ARM: 8488/1: Make IPI_CPU_BACKTRACE a "non-secure" SGI"), which
> changed the number assignment from '15' to '8', but as far as I can
> tell has existed since the IPI tracepoints were first introduced.
> If we decide to backport this patch to stable kernels, we probably
> need to backport e7273ff49acf as well.
>
> Resubmiting this patch is because that I found coverity is complaining
> the issue this patch fixed, and also I got the traces like below:
> "ipi_raise: target_mask=00000001 (machine_suspend)" which actually was
> the TPS of suspend_resume[1] rather than ipi_raise.
>
> [1]
> https://elixir.bootlin.com/linux/latest/source/kernel/power/suspend.c#L80
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: e7273ff49acf ("ARM: 8488/1: Make IPI_CPU_BACKTRACE a "non-secure" SGI")
> Fixes: 365ec7b17327 ("ARM: add IPI tracepoints") # v3.17
> Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> ---
> arch/arm/include/asm/hardirq.h | 1 +
> arch/arm/kernel/smp.c | 6 +++++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
> index cba23ea..7a88f16 100644
> --- a/arch/arm/include/asm/hardirq.h
> +++ b/arch/arm/include/asm/hardirq.h
> @@ -6,6 +6,7 @@
> #include <linux/threads.h>
> #include <asm/irq.h>
>
> +/* number of IPIS _not_ including IPI_CPU_BACKTRACE */
> #define NR_IPI 7
>
> typedef struct {
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 0978282..123be77 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -75,6 +75,10 @@ enum ipi_msg_type {
> IPI_CPU_STOP,
> IPI_IRQ_WORK,
> IPI_COMPLETION,
> + /*
> + * CPU_BACKTRACE is special and not included in NR_IPI
> + * or tracable with trace_ipi_*
> + */
> IPI_CPU_BACKTRACE,
> /*
> * SGI8-15 can be reserved by secure firmware, and thus may
> @@ -755,7 +759,7 @@ core_initcall(register_cpufreq_notifier);
>
> static void raise_nmi(cpumask_t *mask)
> {
> - smp_cross_call(mask, IPI_CPU_BACKTRACE);
> + _smp_cross_call(mask, IPI_CPU_BACKTRACE);
Here should be double underline, i.e.
> + __smp_cross_call(mask, IPI_CPU_BACKTRACE);
> }
>
> void arch_trigger_cpumask_backtrace(const cpumask_t *mask, bool exclude_self)
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] ARM: prevent tracing IPI_CPU_BACKTRACE
2018-09-27 3:41 [RESEND PATCH] ARM: prevent tracing IPI_CPU_BACKTRACE Chunyan Zhang
2018-10-09 2:20 ` Chunyan Zhang
2018-10-18 3:32 ` Chunyan Zhang
@ 2018-10-18 8:23 ` Chunyan Zhang
2019-01-23 16:49 ` Julien Thierry
2 siblings, 1 reply; 5+ messages in thread
From: Chunyan Zhang @ 2018-10-18 8:23 UTC (permalink / raw)
To: Russell King, Marc Zyngier
Cc: Arnd Bergmann, linux-kernel, linux-arm-kernel, Chunyan Zhang
From: Arnd Bergmann <arnd@arndb.de>
When function tracing for IPIs is enabled, we get a warning for an
overflow of the ipi_types array with the IPI_CPU_BACKTRACE type
as triggered by raise_nmi():
arch/arm/kernel/smp.c: In function 'raise_nmi':
arch/arm/kernel/smp.c:489:2: error: array subscript is above array bounds [-Werror=array-bounds]
trace_ipi_raise(target, ipi_types[ipinr]);
This is a correct warning as we actually overflow the array here.
This patch raise_nmi() to call __smp_cross_call() instead of
smp_cross_call(), to avoid calling into ftrace. For clarification,
I'm also adding a two new code comments describing how this one
is special.
The warning appears to have shown up after patch e7273ff49acf
("ARM: 8488/1: Make IPI_CPU_BACKTRACE a "non-secure" SGI"), which
changed the number assignment from '15' to '8', but as far as I can
tell has existed since the IPI tracepoints were first introduced.
If we decide to backport this patch to stable kernels, we probably
need to backport e7273ff49acf as well.
Resubmitting this patch is because that I found coverity is complaining
the issue this patch fixed, and also I got the traces like below:
"ipi_raise: target_mask=00000001 (machine_suspend)" which actually was
the TPS of suspend_resume[1] rather that ipi_raise.
[1]
https://elixir.bootlin.com/linux/latest/source/kernel/power/suspend.c#L80
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Fixes: e7273ff49acf ("ARM: 8488/1: Make IPI_CPU_BACKTRACE a "non-secure" SGI")
Fixes: 365ec7b17327 ("ARM: add IPI tracepoints") # v3.17
Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
---
arch/arm/include/asm/hardirq.h | 1 +
arch/arm/kernel/smp.c | 6 +++++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
index cba23ea..7a88f16 100644
--- a/arch/arm/include/asm/hardirq.h
+++ b/arch/arm/include/asm/hardirq.h
@@ -6,6 +6,7 @@
#include <linux/threads.h>
#include <asm/irq.h>
+/* number of IPIS _not_ including IPI_CPU_BACKTRACE */
#define NR_IPI 7
typedef struct {
diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
index 0978282..ddd48e2 100644
--- a/arch/arm/kernel/smp.c
+++ b/arch/arm/kernel/smp.c
@@ -75,6 +75,10 @@ enum ipi_msg_type {
IPI_CPU_STOP,
IPI_IRQ_WORK,
IPI_COMPLETION,
+ /*
+ * CPU_BACKTRACE is special and not included in NR_IPI
+ * or tracable with trace_ipi_*
+ */
IPI_CPU_BACKTRACE,
/*
* SGI8-15 can be reserved by secure firmware, and thus may
@@ -755,7 +759,7 @@ core_initcall(register_cpufreq_notifier);
static void raise_nmi(cpumask_t *mask)
{
- smp_cross_call(mask, IPI_CPU_BACKTRACE);
+ __smp_cross_call(mask, IPI_CPU_BACKTRACE);
}
void arch_trigger_cpumask_backtrace(const cpumask_t *mask, bool exclude_self)
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] ARM: prevent tracing IPI_CPU_BACKTRACE
2018-10-18 8:23 ` [PATCH] " Chunyan Zhang
@ 2019-01-23 16:49 ` Julien Thierry
0 siblings, 0 replies; 5+ messages in thread
From: Julien Thierry @ 2019-01-23 16:49 UTC (permalink / raw)
To: Chunyan Zhang, Russell King, Marc Zyngier
Cc: Arnd Bergmann, linux-kernel, linux-arm-kernel, Chunyan Zhang
Hi,
On 18/10/2018 09:23, Chunyan Zhang wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> When function tracing for IPIs is enabled, we get a warning for an
> overflow of the ipi_types array with the IPI_CPU_BACKTRACE type
> as triggered by raise_nmi():
>
> arch/arm/kernel/smp.c: In function 'raise_nmi':
> arch/arm/kernel/smp.c:489:2: error: array subscript is above array bounds [-Werror=array-bounds]
> trace_ipi_raise(target, ipi_types[ipinr]);
>
> This is a correct warning as we actually overflow the array here.
>
> This patch raise_nmi() to call __smp_cross_call() instead of
> smp_cross_call(), to avoid calling into ftrace. For clarification,
> I'm also adding a two new code comments describing how this one
> is special.
>
> The warning appears to have shown up after patch e7273ff49acf
> ("ARM: 8488/1: Make IPI_CPU_BACKTRACE a "non-secure" SGI"), which
> changed the number assignment from '15' to '8', but as far as I can
> tell has existed since the IPI tracepoints were first introduced.
> If we decide to backport this patch to stable kernels, we probably
> need to backport e7273ff49acf as well.
>
> Resubmitting this patch is because that I found coverity is complaining
> the issue this patch fixed, and also I got the traces like below:
> "ipi_raise: target_mask=00000001 (machine_suspend)" which actually was
> the TPS of suspend_resume[1] rather that ipi_raise.
>
What's the status on this patch? it does not seem to be in v5.0-rc3 nor
linux-next and looks like an actual issue.
Also, I'm assuming the patch is avoiding the tracing of
IPI_CPU_BACKTRACE because the tracing is not NMI safe. It would be good
to have a comment about that either near NR_IPI or ipi_types stating why
this IPI is excluded.
Thanks,
> [1]
> https://elixir.bootlin.com/linux/latest/source/kernel/power/suspend.c#L80
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: e7273ff49acf ("ARM: 8488/1: Make IPI_CPU_BACKTRACE a "non-secure" SGI")
> Fixes: 365ec7b17327 ("ARM: add IPI tracepoints") # v3.17
> Signed-off-by: Chunyan Zhang <zhang.chunyan@linaro.org>
> ---
> arch/arm/include/asm/hardirq.h | 1 +
> arch/arm/kernel/smp.c | 6 +++++-
> 2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
> index cba23ea..7a88f16 100644
> --- a/arch/arm/include/asm/hardirq.h
> +++ b/arch/arm/include/asm/hardirq.h
> @@ -6,6 +6,7 @@
> #include <linux/threads.h>
> #include <asm/irq.h>
>
> +/* number of IPIS _not_ including IPI_CPU_BACKTRACE */
> #define NR_IPI 7
>
> typedef struct {
> diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c
> index 0978282..ddd48e2 100644
> --- a/arch/arm/kernel/smp.c
> +++ b/arch/arm/kernel/smp.c
> @@ -75,6 +75,10 @@ enum ipi_msg_type {
> IPI_CPU_STOP,
> IPI_IRQ_WORK,
> IPI_COMPLETION,
> + /*
> + * CPU_BACKTRACE is special and not included in NR_IPI
> + * or tracable with trace_ipi_*
> + */
> IPI_CPU_BACKTRACE,
> /*
> * SGI8-15 can be reserved by secure firmware, and thus may
> @@ -755,7 +759,7 @@ core_initcall(register_cpufreq_notifier);
>
> static void raise_nmi(cpumask_t *mask)
> {
> - smp_cross_call(mask, IPI_CPU_BACKTRACE);
> + __smp_cross_call(mask, IPI_CPU_BACKTRACE);
> }
>
> void arch_trigger_cpumask_backtrace(const cpumask_t *mask, bool exclude_self)
>
--
Julien Thierry
^ permalink raw reply [flat|nested] 5+ messages in thread