Skip to contentSkip to conversation

stackleak: mark stackleak_track_stack() as notrace

PATCH In review Anders Roxell 3 participants 8 messages Nov 30, 2018 – Dec 6, 2018

lore
Share & tools
lore
#0 lore raw

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

---
 kernel/stackleak.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/stackleak.c +1 -1
index e42892926244..5de3bf596dd7 100644
@@ -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,
Signature · 1 line
2.19.2
Acked-by on v1 1/3
#1 lore raw
Quoted 18 lines · On Fri, 30 Nov 2018 16:08:59 +0100 Anders Roxell <[email protected]>…

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
Quoted 5 lines

Rework so we mark stackleak_track_stack as notrace

-- Steve

Quoted 17 lines
---
 kernel/stackleak.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/stackleak.c +1 -1
index e42892926244..5de3bf596dd7 100644
@@ -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,
Acked-by on v1 1/3
#2 lore raw
In reply to Anders Roxell
Quoted 42 lines · Anders Roxell

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

---
 kernel/stackleak.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/stackleak.c +1 -2
index e42892926244..5de3bf596dd7 100644
@@ -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,
--
Quoted 2 lines

2.19.2

Steven, I assume this series going via your tree?

Signature · 1 line
Kees Cook
#3 lore raw
Quoted 19 lines · On Wed, 5 Dec 2018 17:08:34 -0800 Kees Cook <[email protected]> wrote:
Quoted 14 lines
kernel/stackleak.c +1 -2
index e42892926244..5de3bf596dd7 100644
@@ -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,
--
Quoted 2 lines

2.19.2

Quoted 1 line

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

#4 lore raw
Quoted 7 lines · 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

#5 lore raw
Quoted 10 lines · Steven Rostedt
Quoted 6 lines · 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

Signature · 1 line
Kees Cook
#6 lore raw
Quoted 9 lines · On Wed, 5 Dec 2018 19:29:11 -0800 Kees Cook <[email protected]> wrote:
Quoted 8 lines · Steven Rostedt
Quoted 3 lines · 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:

Quoted 7 lines
Quoted 2 lines
Quoted 2 lines

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

#7 lore raw
Quoted 19 lines · Steven Rostedt
Quoted 15 lines · On Wed, 5 Dec 2018 19:29:11 -0800 Kees Cook <[email protected]> wrote:
Quoted 10 lines · Steven Rostedt
Quoted 6 lines · 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. :)

Quoted 6 lines
Quoted 1 line

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. :)

Signature · 1 line
Kees Cook