2018-11-30 15:12:53

by Anders Roxell

[permalink] [raw]
Subject: [PATCH 1/3] stackleak: mark stackleak_track_stack() as notrace

Function graph tracing recurses into itself when stackleak is enabled,
causing the ftrace graph selftest to run for up to 90 seconds and
trigger the softlockup watchdog.

Breakpoint 2, ftrace_graph_caller () at ../arch/arm64/kernel/entry-ftrace.S:200
200 mcount_get_lr_addr x0 // pointer to function's saved lr
(gdb) bt
\#0 ftrace_graph_caller () at ../arch/arm64/kernel/entry-ftrace.S:200
\#1 0xffffff80081d5280 in ftrace_caller () at ../arch/arm64/kernel/entry-ftrace.S:153
\#2 0xffffff8008555484 in stackleak_track_stack () at ../kernel/stackleak.c:106
\#3 0xffffff8008421ff8 in ftrace_ops_test (ops=0xffffff8009eaa840 <graph_ops>, ip=18446743524091297036, regs=<optimized out>) at ../kernel/trace/ftrace.c:1507
\#4 0xffffff8008428770 in __ftrace_ops_list_func (regs=<optimized out>, ignored=<optimized out>, parent_ip=<optimized out>, ip=<optimized out>) at ../kernel/trace/ftrace.c:6286
\#5 ftrace_ops_no_ops (ip=18446743524091297036, parent_ip=18446743524091242824) at ../kernel/trace/ftrace.c:6321
\#6 0xffffff80081d5280 in ftrace_caller () at ../arch/arm64/kernel/entry-ftrace.S:153
\#7 0xffffff800832fd10 in irq_find_mapping (domain=0xffffffc03fc4bc80, hwirq=27) at ../kernel/irq/irqdomain.c:876
\#8 0xffffff800832294c in __handle_domain_irq (domain=0xffffffc03fc4bc80, hwirq=27, lookup=true, regs=0xffffff800814b840) at ../kernel/irq/irqdesc.c:650
\#9 0xffffff80081d52b4 in ftrace_graph_caller () at ../arch/arm64/kernel/entry-ftrace.S:205

Rework so we mark stackleak_track_stack as notrace

Co-developed-by: Arnd Bergmann <[email protected]>
Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Anders Roxell <[email protected]>
---
kernel/stackleak.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/stackleak.c b/kernel/stackleak.c
index e42892926244..5de3bf596dd7 100644
--- a/kernel/stackleak.c
+++ b/kernel/stackleak.c
@@ -102,7 +102,7 @@ asmlinkage void stackleak_erase(void)
current->lowest_stack = current_top_of_stack() - THREAD_SIZE/64;
}

-void __used stackleak_track_stack(void)
+void __used notrace stackleak_track_stack(void)
{
/*
* N.B. stackleak_erase() fills the kernel stack with the poison value,
--
2.19.2



2018-11-30 15:23:41

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 1/3] stackleak: mark stackleak_track_stack() as notrace

On Fri, 30 Nov 2018 16:08:59 +0100
Anders Roxell <[email protected]> wrote:

> Function graph tracing recurses into itself when stackleak is enabled,
> causing the ftrace graph selftest to run for up to 90 seconds and
> trigger the softlockup watchdog.
>
> Breakpoint 2, ftrace_graph_caller () at ../arch/arm64/kernel/entry-ftrace.S:200
> 200 mcount_get_lr_addr x0 // pointer to function's saved lr
> (gdb) bt
> \#0 ftrace_graph_caller () at ../arch/arm64/kernel/entry-ftrace.S:200
> \#1 0xffffff80081d5280 in ftrace_caller () at ../arch/arm64/kernel/entry-ftrace.S:153
> \#2 0xffffff8008555484 in stackleak_track_stack () at ../kernel/stackleak.c:106
> \#3 0xffffff8008421ff8 in ftrace_ops_test (ops=0xffffff8009eaa840 <graph_ops>, ip=18446743524091297036, regs=<optimized out>) at ../kernel/trace/ftrace.c:1507
> \#4 0xffffff8008428770 in __ftrace_ops_list_func (regs=<optimized out>, ignored=<optimized out>, parent_ip=<optimized out>, ip=<optimized out>) at ../kernel/trace/ftrace.c:6286
> \#5 ftrace_ops_no_ops (ip=18446743524091297036, parent_ip=18446743524091242824) at ../kernel/trace/ftrace.c:6321
> \#6 0xffffff80081d5280 in ftrace_caller () at ../arch/arm64/kernel/entry-ftrace.S:153
> \#7 0xffffff800832fd10 in irq_find_mapping (domain=0xffffffc03fc4bc80, hwirq=27) at ../kernel/irq/irqdomain.c:876
> \#8 0xffffff800832294c in __handle_domain_irq (domain=0xffffffc03fc4bc80, hwirq=27, lookup=true, regs=0xffffff800814b840) at ../kernel/irq/irqdesc.c:650
> \#9 0xffffff80081d52b4 in ftrace_graph_caller () at ../arch/arm64/kernel/entry-ftrace.S:205
>
> Rework so we mark stackleak_track_stack as notrace
>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Anders Roxell <[email protected]>

Acked-by: Steven Rostedt (VMware) <[email protected]>

-- Steve

> ---
> kernel/stackleak.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/stackleak.c b/kernel/stackleak.c
> index e42892926244..5de3bf596dd7 100644
> --- a/kernel/stackleak.c
> +++ b/kernel/stackleak.c
> @@ -102,7 +102,7 @@ asmlinkage void stackleak_erase(void)
> current->lowest_stack = current_top_of_stack() - THREAD_SIZE/64;
> }
>
> -void __used stackleak_track_stack(void)
> +void __used notrace stackleak_track_stack(void)
> {
> /*
> * N.B. stackleak_erase() fills the kernel stack with the poison value,


2018-12-06 01:09:49

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 1/3] stackleak: mark stackleak_track_stack() as notrace

On Fri, Nov 30, 2018 at 7:09 AM Anders Roxell <[email protected]> wrote:
>
> Function graph tracing recurses into itself when stackleak is enabled,
> causing the ftrace graph selftest to run for up to 90 seconds and
> trigger the softlockup watchdog.
>
> Breakpoint 2, ftrace_graph_caller () at ../arch/arm64/kernel/entry-ftrace.S:200
> 200 mcount_get_lr_addr x0 // pointer to function's saved lr
> (gdb) bt
> \#0 ftrace_graph_caller () at ../arch/arm64/kernel/entry-ftrace.S:200
> \#1 0xffffff80081d5280 in ftrace_caller () at ../arch/arm64/kernel/entry-ftrace.S:153
> \#2 0xffffff8008555484 in stackleak_track_stack () at ../kernel/stackleak.c:106
> \#3 0xffffff8008421ff8 in ftrace_ops_test (ops=0xffffff8009eaa840 <graph_ops>, ip=18446743524091297036, regs=<optimized out>) at ../kernel/trace/ftrace.c:1507
> \#4 0xffffff8008428770 in __ftrace_ops_list_func (regs=<optimized out>, ignored=<optimized out>, parent_ip=<optimized out>, ip=<optimized out>) at ../kernel/trace/ftrace.c:6286
> \#5 ftrace_ops_no_ops (ip=18446743524091297036, parent_ip=18446743524091242824) at ../kernel/trace/ftrace.c:6321
> \#6 0xffffff80081d5280 in ftrace_caller () at ../arch/arm64/kernel/entry-ftrace.S:153
> \#7 0xffffff800832fd10 in irq_find_mapping (domain=0xffffffc03fc4bc80, hwirq=27) at ../kernel/irq/irqdomain.c:876
> \#8 0xffffff800832294c in __handle_domain_irq (domain=0xffffffc03fc4bc80, hwirq=27, lookup=true, regs=0xffffff800814b840) at ../kernel/irq/irqdesc.c:650
> \#9 0xffffff80081d52b4 in ftrace_graph_caller () at ../arch/arm64/kernel/entry-ftrace.S:205
>
> Rework so we mark stackleak_track_stack as notrace
>
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Anders Roxell <[email protected]>
> ---
> kernel/stackleak.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/stackleak.c b/kernel/stackleak.c
> index e42892926244..5de3bf596dd7 100644
> --- a/kernel/stackleak.c
> +++ b/kernel/stackleak.c
> @@ -102,7 +102,7 @@ asmlinkage void stackleak_erase(void)
> current->lowest_stack = current_top_of_stack() - THREAD_SIZE/64;
> }
>
> -void __used stackleak_track_stack(void)
> +void __used notrace stackleak_track_stack(void)
> {
> /*
> * N.B. stackleak_erase() fills the kernel stack with the poison value,
> --
> 2.19.2
>

Acked-by: Kees Cook <[email protected]>

Steven, I assume this series going via your tree?

--
Kees Cook

2018-12-06 02:27:59

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 1/3] stackleak: mark stackleak_track_stack() as notrace

On Wed, 5 Dec 2018 17:08:34 -0800
Kees Cook <[email protected]> wrote:

> > diff --git a/kernel/stackleak.c b/kernel/stackleak.c
> > index e42892926244..5de3bf596dd7 100644
> > --- a/kernel/stackleak.c
> > +++ b/kernel/stackleak.c
> > @@ -102,7 +102,7 @@ asmlinkage void stackleak_erase(void)
> > current->lowest_stack = current_top_of_stack() - THREAD_SIZE/64;
> > }
> >
> > -void __used stackleak_track_stack(void)
> > +void __used notrace stackleak_track_stack(void)
> > {
> > /*
> > * N.B. stackleak_erase() fills the kernel stack with the poison value,
> > --
> > 2.19.2
> >
>
> Acked-by: Kees Cook <[email protected]>
>
> Steven, I assume this series going via your tree?

??

A notrace addition doesn't make it mine.

I added changes for the cond_resched in a different patch series that
I'll pull in (they are independent from this). I'll Ack the Makefile
change in the tracing directory, but the rest belongs to others.

-- Steve

2018-12-06 02:30:40

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 1/3] stackleak: mark stackleak_track_stack() as notrace

On Wed, 5 Dec 2018 21:26:51 -0500
Steven Rostedt <[email protected]> wrote:

> On Wed, 5 Dec 2018 17:08:34 -0800
> Kees Cook <[email protected]> wrote:
>

> I'll Ack the Makefile
> change in the tracing directory, but the rest belongs to others.
>

I see I already acked that patch. BTW, when sending a patch series, you
really need a 0/3 patch as a header and the rest be threaded. I had a
hard time finding that patch in the sea of my INBOX.

If I was the one to pull it in, I wouldn't do it if the series was
unthreaded like this.

-- Steve

2018-12-06 03:30:15

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 1/3] stackleak: mark stackleak_track_stack() as notrace

On Wed, Dec 5, 2018 at 6:29 PM Steven Rostedt <[email protected]> wrote:
>
> On Wed, 5 Dec 2018 21:26:51 -0500
> Steven Rostedt <[email protected]> wrote:
>
> > On Wed, 5 Dec 2018 17:08:34 -0800
> > Kees Cook <[email protected]> wrote:
> >
>
> > I'll Ack the Makefile
> > change in the tracing directory, but the rest belongs to others.

Okay, I wasn't sure. Anders's patch was marked "1/3" so I thought it
was directed at you. :)

I'll grab this one in the gcc-plugins tree.

-Kees

--
Kees Cook

2018-12-06 03:44:51

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 1/3] stackleak: mark stackleak_track_stack() as notrace

On Wed, 5 Dec 2018 19:29:11 -0800
Kees Cook <[email protected]> wrote:

> On Wed, Dec 5, 2018 at 6:29 PM Steven Rostedt <[email protected]> wrote:
> >
> > On Wed, 5 Dec 2018 21:26:51 -0500
> > Steven Rostedt <[email protected]> wrote:
> >
> > > On Wed, 5 Dec 2018 17:08:34 -0800
> > > Kees Cook <[email protected]> wrote:
> > >
> >
> > > I'll Ack the Makefile
> > > change in the tracing directory, but the rest belongs to others.
>
> Okay, I wasn't sure. Anders's patch was marked "1/3" so I thought it
> was directed at you. :)
>
> I'll grab this one in the gcc-plugins tree.

Should I just take patch 2 then? I'm thinking it's independent too.

I'm collecting patches for the next merge window right now so it wont
really be an issue if I do.

-- Steve

2018-12-06 03:57:46

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 1/3] stackleak: mark stackleak_track_stack() as notrace

On Wed, Dec 5, 2018 at 7:43 PM Steven Rostedt <[email protected]> wrote:
>
> On Wed, 5 Dec 2018 19:29:11 -0800
> Kees Cook <[email protected]> wrote:
>
> > On Wed, Dec 5, 2018 at 6:29 PM Steven Rostedt <[email protected]> wrote:
> > >
> > > On Wed, 5 Dec 2018 21:26:51 -0500
> > > Steven Rostedt <[email protected]> wrote:
> > >
> > > > On Wed, 5 Dec 2018 17:08:34 -0800
> > > > Kees Cook <[email protected]> wrote:
> > > >
> > >
> > > > I'll Ack the Makefile
> > > > change in the tracing directory, but the rest belongs to others.
> >
> > Okay, I wasn't sure. Anders's patch was marked "1/3" so I thought it
> > was directed at you. :)
> >
> > I'll grab this one in the gcc-plugins tree.
>
> Should I just take patch 2 then? I'm thinking it's independent too.
>
> I'm collecting patches for the next merge window right now so it wont
> really be an issue if I do.

Sure! I'm not sure what Anders's intention was, but yeah. :)

--
Kees Cook