| Message ID | 20221214044252.1910657-1-nicholas@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/64: Implement arch_within_stack_frames | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
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__ */
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. > > > + > > +/* > > + * 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; This is not the same as x86, they start with the parent of the current frame. I assume because the way the caller is set up (with a noinline function from an out of line call), then there must be at least one stack frame that does not have to be checked, but if I'm wrong about that and there is some reason we need to be different it should be commented.. > > + > > + 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; What about just copying x86's implementation including using __builtin_frame_address(1/2)? Are those builtins reliable for all our targets and compiler versions? For bonus points, extract the x86 code out into asm-generic and make it usable by both - static inline int generic_within_stack_frames(unsigned int ptr_offset, unsigned int vars_offset, const void * const stack, const void * const stackend, const void *obj, unsigned long len) And our arch_within_stack_frames can just be return generic_within_stack_frames(0, STACK_FRAME_ARGS_OFFSET, stack, stackend, obj, len); Thanks, Nick
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
Le 14/12/2022 à 12:48, Christophe Leroy a écrit : > > > 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 > Looks like I misread STACK_FRAME_MIN_SIZE. So yes, we need a new macro for it. What about STACK_FRAME_PARAMS ?
On Wed, Dec 14, 2022 at 09:39:25PM +1000, Nicholas Piggin wrote: > What about just copying x86's implementation including using > __builtin_frame_address(1/2)? Are those builtins reliable for all > our targets and compiler versions? __builtin_frame_address(n) with n > 0 is specifically not supported, not on any architecture; it does not work in all situations on *any* arch, and not in *any* situation on some archs. This is clearly documented: Calling this function with a nonzero argument can have unpredictable effects, including crashing the calling program. As a result, calls that are considered unsafe are diagnosed when the '-Wframe-address' option is in effect. Such calls should only be made in debugging situations. (and that warning is enabled by -Wall). Segher
On Thu Dec 15, 2022 at 10:17 AM AEST, Segher Boessenkool wrote: > On Wed, Dec 14, 2022 at 09:39:25PM +1000, Nicholas Piggin wrote: > > What about just copying x86's implementation including using > > __builtin_frame_address(1/2)? Are those builtins reliable for all > > our targets and compiler versions? > > __builtin_frame_address(n) with n > 0 is specifically not supported, not > on any architecture; it does not work in all situations on *any* arch, > and not in *any* situation on some archs. No, but the particular case of powerpc where we have frame pointers and calling from a function where we know we have at least n + 1 frames on the stack, it would be okay, right? The code is not really different than using r1 directly and dereferencing that. Thanks, Nick
On Thu, Dec 15, 2022 at 10:52:35AM +1000, Nicholas Piggin wrote: > On Thu Dec 15, 2022 at 10:17 AM AEST, Segher Boessenkool wrote: > > On Wed, Dec 14, 2022 at 09:39:25PM +1000, Nicholas Piggin wrote: > > > What about just copying x86's implementation including using > > > __builtin_frame_address(1/2)? Are those builtins reliable for all > > > our targets and compiler versions? > > > > __builtin_frame_address(n) with n > 0 is specifically not supported, not > > on any architecture; it does not work in all situations on *any* arch, > > and not in *any* situation on some archs. > > No, but the particular case of powerpc where we have frame pointers and > calling from a function where we know we have at least n + 1 frames on > the stack, it would be okay, right? The code is not really different > than using r1 directly and dereferencing that. We do not typically have frame pointers; those make quite a bit slower code. But we do not *need* frame pointers for most purposes, perhaps that is what you were getting at? In simple cases r1 is the address of the current frame, and accessing the machine word there gets you the previous frame, etc. (until you hit a zero, there is no parent frame above that). This is the *machine* frame, not the C frame. Often they are the same. But there are complications. As long as you only use it for debug purposes, do not use it for program logic, and are sure to check any pointer for nil, things will work fine. The kernel does not do any of the more problematic cases I think, but no promises (dynamic stack alignment, recursive functions, variable size stack allocations, PIC code (the model of it used for shared libraries that is, not position-independent code in general, as PowerPC mostly is no matter what), split stack, ...) So at least do not follow a stack chain past the terminator, and do not expect there to be exactly one frame per C function (there can be zero, or more than one). There also is the complication that there is no sure-fire way to detect if a new frame has been set up for a function (yet), of course. Segher
On Thu, Dec 15, 2022 at 10:29:38AM -0600, Segher Boessenkool wrote: > The kernel does not do any of the more > problematic cases I think, but no promises (dynamic stack alignment, > recursive functions Before people get scared: I meant *nested* functions. With a static chain and all that :-) Segher
On 14/12/2022 10:39 pm, Nicholas Piggin wrote: > 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 I'm not familiar with the 32bit ABI, but from a quick glance through it seems like the only thing that would need to change is to set then PARAMETER_SAVE_OFFSET (to be renamed in the next version as per suggestions) to 8 bytes, the layout of the stack and the back chain remains the same. Is there something else that I am missing or is that it? > >>> 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. > >> >>> + >>> +/* >>> + * 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; > > This is not the same as x86, they start with the parent of the current > frame. I assume because the way the caller is set up (with a noinline > function from an out of line call), then there must be at least one > stack frame that does not have to be checked, but if I'm wrong about > that and there is some reason we need to be different it should be > commented.. > Yes, this is something that I overlooked, the current frame is created as a result of the call to copy_{to,from}_user and should therefore not contain any data being copied. >>> + >>> + 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; > > What about just copying x86's implementation including using > __builtin_frame_address(1/2)? Are those builtins reliable for all > our targets and compiler versions? > From what I found it has undefined behavior. Since x86 has it's use guarded behind CONFIG_FRAME_POINTER which I couldn't find used in the ppc code I decided it was best to avoid them. Could be wrong though. > For bonus points, extract the x86 code out into asm-generic and > make it usable by both - > > static inline int generic_within_stack_frames(unsigned int ptr_offset, > unsigned int vars_offset, > const void * const stack, > const void * const stackend, > const void *obj, unsigned long len) > > And our arch_within_stack_frames can just be > > return generic_within_stack_frames(0, STACK_FRAME_ARGS_OFFSET, > stack, stackend, obj, len); > > Thanks, > Nick
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 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 + +/* + * 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; +} +#endif /* CONFIG_PPC64 */ + #endif /* !__ASSEMBLY__ */ #endif /* __KERNEL__ */
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(+)