Re: [PATCH] powerpc/64: Implement arch_within_stack_frames

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 14/12/2022 à 05:42, Nicholas Miehlbradt a écrit :
> Walks the stack when copy_{to,from}_user address is in the stack to
> ensure that the object being copied is entirely within a single stack
> frame.
> 
> Substatially similar to the x86 implementation except using the back
> chain to traverse the stack and identify stack frame boundaries.
> 
> Signed-off-by: Nicholas Miehlbradt <nicholas@linux.ibm.com>
> ---
>   arch/powerpc/Kconfig                   |  1 +
>   arch/powerpc/include/asm/thread_info.h | 38 ++++++++++++++++++++++++++
>   2 files changed, 39 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 2ca5418457ed..4c59d139ea83 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -198,6 +198,7 @@ config PPC
>   	select HAVE_ARCH_KASAN_VMALLOC		if HAVE_ARCH_KASAN
>   	select HAVE_ARCH_KFENCE			if ARCH_SUPPORTS_DEBUG_PAGEALLOC
>   	select HAVE_ARCH_RANDOMIZE_KSTACK_OFFSET
> +	select HAVE_ARCH_WITHIN_STACK_FRAMES	if PPC64

Why don't you do something that works for both PPC32 and PPC64 ?

>   	select HAVE_ARCH_KGDB
>   	select HAVE_ARCH_MMAP_RND_BITS
>   	select HAVE_ARCH_MMAP_RND_COMPAT_BITS	if COMPAT
> diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
> index af58f1ed3952..efdf39e07884 100644
> --- a/arch/powerpc/include/asm/thread_info.h
> +++ b/arch/powerpc/include/asm/thread_info.h
> @@ -186,6 +186,44 @@ static inline bool test_thread_local_flags(unsigned int flags)
>   #define is_elf2_task() (0)
>   #endif
>   
> +#ifdef CONFIG_PPC64
> +
> +#ifdef CONFIG_PPC64_ELF_ABI_V1
> +#define PARAMETER_SAVE_OFFSET 48
> +#else
> +#define PARAMETER_SAVE_OFFSET 32
> +#endif

Why not use STACK_INT_FRAME_REGS, defined in asm/ptrace.h ?

> +
> +/*
> + * Walks up the stack frames to make sure that the specified object is
> + * entirely contained by a single stack frame.
> + *
> + * Returns:
> + *	GOOD_FRAME	if within a frame
> + *	BAD_STACK	if placed across a frame boundary (or outside stack)
> + */
> +static inline int arch_within_stack_frames(const void * const stack,
> +					   const void * const stackend,
> +					   const void *obj, unsigned long len)
> +{
> +	const void *frame;
> +	const void *oldframe;
> +
> +	oldframe = (const void *)current_stack_pointer;
> +	frame = *(const void * const *)oldframe;
> +
> +	while (stack <= frame && frame < stackend) {
> +		if (obj + len <= frame)
> +			return obj >= oldframe + PARAMETER_SAVE_OFFSET ?
> +				GOOD_FRAME : BAD_STACK;
> +		oldframe = frame;
> +		frame = *(const void * const *)oldframe;
> +	}
> +
> +	return BAD_STACK;
> +}

What about:

+	const void *frame;
+	const void *params;
+
+	params = (const void *)current_stack_pointer + STACK_INT_FRAME_REGS;
+	frame = *(const void * const *)current_stack_pointer;
+
+	while (stack <= frame && frame < stackend) {
+		if (obj + len <= frame)
+			return obj >= params ? GOOD_FRAME : BAD_STACK;
+		params = frame + STACK_INT_FRAME_REGS;
+		frame = *(const void * const *)frame;
+	}
+
+	return BAD_STACK;


> +#endif /* CONFIG_PPC64 */
> +
>   #endif	/* !__ASSEMBLY__ */
>   
>   #endif /* __KERNEL__ */