Re: [PATCH] powerpc/64: Implement arch_within_stack_frames

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

Le 14/12/2022 à 12:39, Nicholas Piggin a écrit :
> On Wed Dec 14, 2022 at 6:39 PM AEST, Christophe Leroy wrote:
>>
>>
>> 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 ?
> 
> +1
> 
>>>    	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 ?
> 
> I think use a STACK_FRAME prefixed define in asm/ptrace.h, but maybe
> avoid overloading the STACK_INT_ stuff for this.
> 

Maybe I should have written _already_ defined :

$ git grep -n STACK_INT_FRAME_REGS arch/powerpc/include/asm/
arch/powerpc/include/asm/ptrace.h:126:#define STACK_INT_FRAME_REGS 
(STACK_FRAME_MIN_SIZE + 16)
arch/powerpc/include/asm/ptrace.h:138:#define STACK_INT_FRAME_REGS 
STACK_FRAME_MIN_SIZE
arch/powerpc/include/asm/ptrace.h:155:#define STACK_INT_FRAME_REGS 
STACK_FRAME_MIN_SIZE


Christophe