Message ID | 20dad21f9446938697573e6642db583bdb874656.1614792440.git.christophe.leroy@csgroup.eu (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [v2] powerpc: Fix save_stack_trace_regs() to have running function as first entry | expand |
Context | Check | Description |
---|---|---|
snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (626a6c3d2e20da80aaa710104f34ea6037b28b33) |
snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 42 lines checked |
snowpatch_ozlabs/needsstable | success | Patch is tagged for stable |
Christophe Leroy <christophe.leroy@csgroup.eu> writes: > It seems like other architectures, namely x86 and arm64 > at least, include the running function as top entry when saving > stack trace with save_stack_trace_regs(). Also riscv AFAICS. > Functionnalities like KFENCE expect it. > > Do the same on powerpc, it allows KFENCE to properly identify the faulting > function as depicted below. Before the patch KFENCE was identifying > finish_task_switch.isra as the faulting function. Thanks, I think this is the right approach. There's kfence but also several other users from what I can see with a quick grep. ... > > Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu> > Fixes: 35de3b1aa168 ("powerpc: Implement save_stack_trace_regs() to enable kprobe stack tracing") > Cc: stable@vger.kernel.org I'm not sure about the Cc to stable. I think we are fixing the behaviour to match the (implied) intent of the API, but that doesn't mean we won't break something by accident. I'll think about it :) cheers
diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c index b6440657ef92..a99bd3697286 100644 --- a/arch/powerpc/kernel/stacktrace.c +++ b/arch/powerpc/kernel/stacktrace.c @@ -23,6 +23,18 @@ #include <asm/paca.h> +static bool save_entry(struct stack_trace *trace, unsigned long ip, int savesched) +{ + if (savesched || !in_sched_functions(ip)) { + if (!trace->skip) + trace->entries[trace->nr_entries++] = ip; + else + trace->skip--; + } + /* Returns true when the trace is full */ + return trace->nr_entries >= trace->max_entries; +} + /* * Save stack-backtrace addresses into a stack_trace buffer. */ @@ -39,14 +51,7 @@ static void save_context_stack(struct stack_trace *trace, unsigned long sp, newsp = stack[0]; ip = stack[STACK_FRAME_LR_SAVE]; - if (savesched || !in_sched_functions(ip)) { - if (!trace->skip) - trace->entries[trace->nr_entries++] = ip; - else - trace->skip--; - } - - if (trace->nr_entries >= trace->max_entries) + if (save_entry(trace, ip, savesched)) return; sp = newsp; @@ -84,6 +89,9 @@ EXPORT_SYMBOL_GPL(save_stack_trace_tsk); void save_stack_trace_regs(struct pt_regs *regs, struct stack_trace *trace) { + if (save_entry(trace, regs->nip, 0)) + return; + save_context_stack(trace, regs->gpr[1], current, 0); } EXPORT_SYMBOL_GPL(save_stack_trace_regs);