lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Fri, 8 Oct 2021 13:52:38 +0100 From: Mark Rutland <mark.rutland@....com> To: Peter Zijlstra <peterz@...radead.org> Cc: keescook@...omium.org, jannh@...gle.com, linux-kernel@...r.kernel.org, vcaputo@...garu.com, mingo@...hat.com, juri.lelli@...hat.com, vincent.guittot@...aro.org, dietmar.eggemann@....com, rostedt@...dmis.org, bsegall@...gle.com, mgorman@...e.de, bristot@...hat.com, akpm@...ux-foundation.org, christian.brauner@...ntu.com, amistry@...gle.com, Kenta.Tada@...y.com, legion@...nel.org, michael.weiss@...ec.fraunhofer.de, mhocko@...e.com, deller@....de, zhengqi.arch@...edance.com, me@...in.cc, tycho@...ho.pizza, tglx@...utronix.de, bp@...en8.de, hpa@...or.com, axboe@...nel.dk, metze@...ba.org, laijs@...ux.alibaba.com, luto@...nel.org, dave.hansen@...ux.intel.com, ebiederm@...ssion.com, ohoono.kwon@...sung.com, kaleshsingh@...gle.com, yifeifz2@...inois.edu, jpoimboe@...hat.com, linux-hardening@...r.kernel.org, linux-arch@...r.kernel.org, vgupta@...nel.org, linux@...linux.org.uk, will@...nel.org, guoren@...nel.org, bcain@...eaurora.org, monstr@...str.eu, tsbogend@...ha.franken.de, nickhu@...estech.com, jonas@...thpole.se, mpe@...erman.id.au, paul.walmsley@...ive.com, hca@...ux.ibm.com, ysato@...rs.sourceforge.jp, davem@...emloft.net, chris@...kel.net Subject: Re: [PATCH 7/7] arch: Fix STACKTRACE_SUPPORT On Fri, Oct 08, 2021 at 01:15:34PM +0200, Peter Zijlstra wrote: > A few archs got save_stack_trace_tsk() vs in_sched_functions() wrong. As mentioned on the last patch, it's not clear to me what the intended semantic of save_stack_trace_tsk() is w.r.t. sched functions, as the naive reading is that it should report *everything* a task may return to. If it's meant to skip sched functions, I think we need some explicit documentation/commentary to that effect. In that case, there are other architectures that need a fixup (e.g. arm64). TBH, I don't think it *should* skip sched functions, and we should filter out sched functions as required at a higher level, or deprecate this interface in favour of arch_stack_walk() where it's easier to have common filter functions invoked during the walk.... Thanks, Mark. > > Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org> > --- > arch/csky/kernel/stacktrace.c | 7 ++++++- > arch/mips/kernel/stacktrace.c | 27 ++++++++++++++++----------- > arch/nds32/kernel/stacktrace.c | 21 +++++++++++---------- > 3 files changed, 33 insertions(+), 22 deletions(-) > > --- a/arch/csky/kernel/stacktrace.c > +++ b/arch/csky/kernel/stacktrace.c > @@ -122,12 +122,17 @@ static bool save_trace(unsigned long pc, > return __save_trace(pc, arg, false); > } > > +static bool save_trace_nosched(unsigned long pc, void *arg) > +{ > + return __save_trace(pc, arg, true); > +} > + > /* > * Save stack-backtrace addresses into a stack_trace buffer. > */ > void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) > { > - walk_stackframe(tsk, NULL, save_trace, trace); > + walk_stackframe(tsk, NULL, save_trace_nosched, trace); > } > EXPORT_SYMBOL_GPL(save_stack_trace_tsk); > > --- a/arch/mips/kernel/stacktrace.c > +++ b/arch/mips/kernel/stacktrace.c > @@ -66,16 +66,7 @@ static void save_context_stack(struct st > #endif > } > > -/* > - * Save stack-backtrace addresses into a stack_trace buffer. > - */ > -void save_stack_trace(struct stack_trace *trace) > -{ > - save_stack_trace_tsk(current, trace); > -} > -EXPORT_SYMBOL_GPL(save_stack_trace); > - > -void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) > +static void __save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace, bool savesched) > { > struct pt_regs dummyregs; > struct pt_regs *regs = &dummyregs; > @@ -88,6 +79,20 @@ void save_stack_trace_tsk(struct task_st > regs->cp0_epc = tsk->thread.reg31; > } else > prepare_frametrace(regs); > - save_context_stack(trace, tsk, regs, tsk == current); > + save_context_stack(trace, tsk, regs, savesched); > +} > + > +/* > + * Save stack-backtrace addresses into a stack_trace buffer. > + */ > +void save_stack_trace(struct stack_trace *trace) > +{ > + __save_stack_trace_tsk(current, trace, true); > +} > +EXPORT_SYMBOL_GPL(save_stack_trace); > + > +void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) > +{ > + __save_stack_trace_tsk(tsk, trace, false); > } > EXPORT_SYMBOL_GPL(save_stack_trace_tsk); > --- a/arch/nds32/kernel/stacktrace.c > +++ b/arch/nds32/kernel/stacktrace.c > @@ -6,25 +6,16 @@ > #include <linux/stacktrace.h> > #include <linux/ftrace.h> > > -void save_stack_trace(struct stack_trace *trace) > -{ > - save_stack_trace_tsk(current, trace); > -} > -EXPORT_SYMBOL_GPL(save_stack_trace); > - > -void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) > +static void __save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace, bool savesched) > { > unsigned long *fpn; > int skip = trace->skip; > - int savesched; > int graph_idx = 0; > > if (tsk == current) { > __asm__ __volatile__("\tori\t%0, $fp, #0\n":"=r"(fpn)); > - savesched = 1; > } else { > fpn = (unsigned long *)thread_saved_fp(tsk); > - savesched = 0; > } > > while (!kstack_end(fpn) && !((unsigned long)fpn & 0x3) > @@ -50,4 +41,14 @@ void save_stack_trace_tsk(struct task_st > fpn = (unsigned long *)fpp; > } > } > +void save_stack_trace(struct stack_trace *trace) > +{ > + __save_stack_trace_tsk(current, trace, true); > +} > +EXPORT_SYMBOL_GPL(save_stack_trace); > + > +void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) > +{ > + __save_stack_trace_tsk(tsk, trace, false); > +} > EXPORT_SYMBOL_GPL(save_stack_trace_tsk); > >
Powered by blists - more mailing lists