All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
	juri.lelli@redhat.com, vincent.guittot@linaro.org,
	dietmar.eggemann@arm.com, rostedt@goodmis.org,
	bsegall@google.com, mgorman@suse.de, bristot@redhat.com,
	akpm@linux-foundation.org, mark.rutland@arm.com,
	zhengqi.arch@bytedance.com, linux@armlinux.org.uk,
	catalin.marinas@arm.com, will@kernel.org, mpe@ellerman.id.au,
	paul.walmsley@sifive.com, palmer@dabbelt.com, hca@linux.ibm.com,
	gor@linux.ibm.com, borntraeger@de.ibm.com,
	linux-arch@vger.kernel.org, ardb@kernel.org
Subject: Re: [PATCH 4/7] arch: Make ARCH_STACKWALK independent of STACKTRACE
Date: Fri, 22 Oct 2021 09:18:05 -0700	[thread overview]
Message-ID: <202110220917.AACE11A@keescook> (raw)
In-Reply-To: <20211022152104.356586621@infradead.org>

On Fri, Oct 22, 2021 at 05:09:37PM +0200, Peter Zijlstra wrote:
> Make arch_stack_walk() available for ARCH_STACKWALK architectures
> without it being entangled in STACKTRACE.

Which CONFIG/arch combos did you build test with this? It looks good,
but I always expect things like this to end up landing in corner cases.
:)

Reviewed-by: Kees Cook <keescook@chromium.org>

> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
>  arch/arm/kernel/stacktrace.c   |    2 --
>  arch/arm64/kernel/stacktrace.c |    4 ----
>  arch/powerpc/kernel/Makefile   |    3 +--
>  arch/riscv/kernel/stacktrace.c |    4 ----
>  arch/s390/kernel/Makefile      |    3 +--
>  arch/x86/kernel/Makefile       |    2 +-
>  include/linux/stacktrace.h     |   33 +++++++++++++++++----------------
>  7 files changed, 20 insertions(+), 31 deletions(-)
> 
> --- a/arch/arm/kernel/stacktrace.c
> +++ b/arch/arm/kernel/stacktrace.c
> @@ -87,7 +87,6 @@ void notrace walk_stackframe(struct stac
>  }
>  EXPORT_SYMBOL(walk_stackframe);
>  
> -#ifdef CONFIG_STACKTRACE
>  noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry,
>  				      void *cookie, struct task_struct *task,
>  				      struct pt_regs *regs)
> @@ -113,4 +112,3 @@ noinline notrace void arch_stack_walk(st
>  			break;
>  	}
>  }
> -#endif
> --- a/arch/arm64/kernel/stacktrace.c
> +++ b/arch/arm64/kernel/stacktrace.c
> @@ -216,8 +216,6 @@ void show_stack(struct task_struct *tsk,
>  	barrier();
>  }
>  
> -#ifdef CONFIG_STACKTRACE
> -
>  noinline notrace void arch_stack_walk(stack_trace_consume_fn consume_entry,
>  			      void *cookie, struct task_struct *task,
>  			      struct pt_regs *regs)
> @@ -236,5 +234,3 @@ noinline notrace void arch_stack_walk(st
>  
>  	walk_stackframe(task, &frame, consume_entry, cookie);
>  }
> -
> -#endif
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -47,7 +47,7 @@ obj-y				:= cputable.o syscalls.o \
>  				   udbg.o misc.o io.o misc_$(BITS).o \
>  				   of_platform.o prom_parse.o firmware.o \
>  				   hw_breakpoint_constraints.o interrupt.o \
> -				   kdebugfs.o
> +				   kdebugfs.o stacktrace.o
>  obj-y				+= ptrace/
>  obj-$(CONFIG_PPC64)		+= setup_64.o \
>  				   paca.o nvram_64.o note.o
> @@ -116,7 +116,6 @@ obj-$(CONFIG_OPTPROBES)		+= optprobes.o
>  obj-$(CONFIG_KPROBES_ON_FTRACE)	+= kprobes-ftrace.o
>  obj-$(CONFIG_UPROBES)		+= uprobes.o
>  obj-$(CONFIG_PPC_UDBG_16550)	+= legacy_serial.o udbg_16550.o
> -obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
>  obj-$(CONFIG_SWIOTLB)		+= dma-swiotlb.o
>  obj-$(CONFIG_ARCH_HAS_DMA_SET_MASK) += dma-mask.o
>  
> --- a/arch/riscv/kernel/stacktrace.c
> +++ b/arch/riscv/kernel/stacktrace.c
> @@ -139,12 +139,8 @@ unsigned long __get_wchan(struct task_st
>  	return pc;
>  }
>  
> -#ifdef CONFIG_STACKTRACE
> -
>  noinline void arch_stack_walk(stack_trace_consume_fn consume_entry, void *cookie,
>  		     struct task_struct *task, struct pt_regs *regs)
>  {
>  	walk_stackframe(task, regs, consume_entry, cookie);
>  }
> -
> -#endif /* CONFIG_STACKTRACE */
> --- a/arch/s390/kernel/Makefile
> +++ b/arch/s390/kernel/Makefile
> @@ -40,7 +40,7 @@ obj-y	+= sysinfo.o lgr.o os_info.o machi
>  obj-y	+= runtime_instr.o cache.o fpu.o dumpstack.o guarded_storage.o sthyi.o
>  obj-y	+= entry.o reipl.o relocate_kernel.o kdebugfs.o alternative.o
>  obj-y	+= nospec-branch.o ipl_vmparm.o machine_kexec_reloc.o unwind_bc.o
> -obj-y	+= smp.o text_amode31.o
> +obj-y	+= smp.o text_amode31.o stacktrace.o
>  
>  extra-y				+= head64.o vmlinux.lds
>  
> @@ -55,7 +55,6 @@ compat-obj-$(CONFIG_AUDIT)	+= compat_aud
>  obj-$(CONFIG_COMPAT)		+= compat_linux.o compat_signal.o
>  obj-$(CONFIG_COMPAT)		+= $(compat-obj-y)
>  obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
> -obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
>  obj-$(CONFIG_KPROBES)		+= kprobes.o
>  obj-$(CONFIG_KPROBES)		+= kprobes_insn_page.o
>  obj-$(CONFIG_FUNCTION_TRACER)	+= mcount.o ftrace.o
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -84,7 +84,7 @@ obj-$(CONFIG_IA32_EMULATION)	+= tls.o
>  obj-y				+= step.o
>  obj-$(CONFIG_INTEL_TXT)		+= tboot.o
>  obj-$(CONFIG_ISA_DMA_API)	+= i8237.o
> -obj-$(CONFIG_STACKTRACE)	+= stacktrace.o
> +obj-y				+= stacktrace.o
>  obj-y				+= cpu/
>  obj-y				+= acpi/
>  obj-y				+= reboot.o
> --- a/include/linux/stacktrace.h
> +++ b/include/linux/stacktrace.h
> @@ -8,21 +8,6 @@
>  struct task_struct;
>  struct pt_regs;
>  
> -#ifdef CONFIG_STACKTRACE
> -void stack_trace_print(const unsigned long *trace, unsigned int nr_entries,
> -		       int spaces);
> -int stack_trace_snprint(char *buf, size_t size, const unsigned long *entries,
> -			unsigned int nr_entries, int spaces);
> -unsigned int stack_trace_save(unsigned long *store, unsigned int size,
> -			      unsigned int skipnr);
> -unsigned int stack_trace_save_tsk(struct task_struct *task,
> -				  unsigned long *store, unsigned int size,
> -				  unsigned int skipnr);
> -unsigned int stack_trace_save_regs(struct pt_regs *regs, unsigned long *store,
> -				   unsigned int size, unsigned int skipnr);
> -unsigned int stack_trace_save_user(unsigned long *store, unsigned int size);
> -
> -/* Internal interfaces. Do not use in generic code */
>  #ifdef CONFIG_ARCH_STACKWALK
>  
>  /**
> @@ -75,8 +60,24 @@ int arch_stack_walk_reliable(stack_trace
>  
>  void arch_stack_walk_user(stack_trace_consume_fn consume_entry, void *cookie,
>  			  const struct pt_regs *regs);
> +#endif /* CONFIG_ARCH_STACKWALK */
>  
> -#else /* CONFIG_ARCH_STACKWALK */
> +#ifdef CONFIG_STACKTRACE
> +void stack_trace_print(const unsigned long *trace, unsigned int nr_entries,
> +		       int spaces);
> +int stack_trace_snprint(char *buf, size_t size, const unsigned long *entries,
> +			unsigned int nr_entries, int spaces);
> +unsigned int stack_trace_save(unsigned long *store, unsigned int size,
> +			      unsigned int skipnr);
> +unsigned int stack_trace_save_tsk(struct task_struct *task,
> +				  unsigned long *store, unsigned int size,
> +				  unsigned int skipnr);
> +unsigned int stack_trace_save_regs(struct pt_regs *regs, unsigned long *store,
> +				   unsigned int size, unsigned int skipnr);
> +unsigned int stack_trace_save_user(unsigned long *store, unsigned int size);
> +
> +#ifndef CONFIG_ARCH_STACKWALK
> +/* Internal interfaces. Do not use in generic code */
>  struct stack_trace {
>  	unsigned int nr_entries, max_entries;
>  	unsigned long *entries;
> 
> 

-- 
Kees Cook

  reply	other threads:[~2021-10-22 16:18 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-22 15:09 [PATCH 0/7] arch: More wchan fixes Peter Zijlstra
2021-10-22 15:09 ` [PATCH 1/7] x86: Fix __get_wchan() for !STACKTRACE Peter Zijlstra
2021-10-22 16:25   ` Kees Cook
2021-10-26 19:16   ` [tip: sched/core] " tip-bot2 for Peter Zijlstra
2021-10-22 15:09 ` [PATCH 2/7] stacktrace,sched: Make stack_trace_save_tsk() more robust Peter Zijlstra
2021-10-22 16:25   ` Kees Cook
2021-10-22 16:45     ` Peter Zijlstra
2021-10-22 16:57       ` Mark Rutland
2021-10-22 16:54     ` Mark Rutland
2021-10-22 17:01       ` Peter Zijlstra
2021-10-25 20:38         ` Peter Zijlstra
2021-10-25 20:52           ` Kees Cook
2021-10-26  9:33           ` Mark Rutland
2021-10-25 16:27   ` Peter Zijlstra
2021-10-22 15:09 ` [PATCH 3/7] ARM: implement ARCH_STACKWALK Peter Zijlstra
2021-10-22 16:18   ` Kees Cook
2021-10-22 15:09 ` [PATCH 4/7] arch: Make ARCH_STACKWALK independent of STACKTRACE Peter Zijlstra
2021-10-22 16:18   ` Kees Cook [this message]
2021-10-22 16:36     ` Peter Zijlstra
2021-10-22 17:06   ` Mark Rutland
2021-10-22 15:09 ` [PATCH 5/7] powerpc, arm64: Mark __switch_to() as __sched Peter Zijlstra
2021-10-22 16:15   ` Kees Cook
2021-10-22 17:40   ` Mark Rutland
2021-10-22 15:09 ` [PATCH 6/7] arch: __get_wchan() || ARCH_STACKWALK Peter Zijlstra
2021-10-22 16:13   ` Kees Cook
2021-10-22 17:52   ` Mark Rutland
2021-10-22 15:09 ` [PATCH 7/7] selftests: proc: Make sure wchan works when it exists Peter Zijlstra
2021-10-22 15:27 ` [PATCH 0/7] arch: More wchan fixes Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202110220917.AACE11A@keescook \
    --to=keescook@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=ardb@kernel.org \
    --cc=borntraeger@de.ibm.com \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=catalin.marinas@arm.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=mark.rutland@arm.com \
    --cc=mgorman@suse.de \
    --cc=mpe@ellerman.id.au \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=vincent.guittot@linaro.org \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    --cc=zhengqi.arch@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.