Subject: Re: [RFC PATCH 14/19] powerpc: split validate_sp into two functions

From: "Nicholas Piggin" <[email-masked]>
Date: Mon, 07 Nov 2022 22:29:31 +1000

On Mon Nov 7, 2022 at 10:58 AM AEST, Russell Currey wrote:
> On Mon, 2022-10-31 at 15:54 +1000, Nicholas Piggin wrote:
> > Most callers just want to validate an arbitrary kernel stack pointer,
> > some need a particular size. Make the size case the exceptional one
> > with an extra function.
> > 
> > Signed-off-by: Nicholas Piggin <[email-masked]>
> > ---
> >  arch/powerpc/include/asm/processor.h | 15 ++++++++++++---
> >  arch/powerpc/kernel/process.c        | 23 ++++++++++++++---------
> >  arch/powerpc/kernel/stacktrace.c     |  2 +-
> >  arch/powerpc/perf/callchain.c        |  6 +++---
> >  4 files changed, 30 insertions(+), 16 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/processor.h
> > b/arch/powerpc/include/asm/processor.h
> > index 631802999d59..e96c9b8c2a60 100644
> > --- a/arch/powerpc/include/asm/processor.h
> > +++ b/arch/powerpc/include/asm/processor.h
> > @@ -374,9 +374,18 @@ static inline unsigned long __pack_fe01(unsigned
> > int fpmode)
> >  
> >  #endif
> >  
> > -/* Check that a certain kernel stack pointer is valid in task_struct
> > p */
> > -int validate_sp(unsigned long sp, struct task_struct *p,
> > -                       unsigned long nbytes);
> > +/*
> > + * Check that a certain kernel stack pointer is a valid (minimum
> > sized)
> > + * stack frame in task_struct p.
> > + */
> > +int validate_sp(unsigned long sp, struct task_struct *p);
> > +
> > +/*
> > + * validate the stack frame of a particular minimum size, used for
> > when we are
> > + * looking at a certain object in the stack beyond the minimum.
> > + */
> > +int validate_sp_size(unsigned long sp, struct task_struct *p,
> > +                    unsigned long nbytes);
> >  
> >  /*
> >   * Prefetch macros.
> > diff --git a/arch/powerpc/kernel/process.c
> > b/arch/powerpc/kernel/process.c
> > index 6cb3982a11ef..b5defea32e75 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -2128,9 +2128,12 @@ static inline int
> > valid_emergency_stack(unsigned long sp, struct task_struct *p,
> >         return 0;
> >  }
> >  
> > -
> > -int validate_sp(unsigned long sp, struct task_struct *p,
> > -                      unsigned long nbytes)
> > +/*
> > + * validate the stack frame of a particular minimum size, used for
> > when we are
> > + * looking at a certain object in the stack beyond the minimum.
> > + */
> > +int validate_sp_size(unsigned long sp, struct task_struct *p,
> > +                    unsigned long nbytes)
> >  {
> >         unsigned long stack_page = (unsigned long)task_stack_page(p);
> >  
> > @@ -2146,7 +2149,10 @@ int validate_sp(unsigned long sp, struct
> > task_struct *p,
> >         return valid_emergency_stack(sp, p, nbytes);
> >  }
> >  
> > -EXPORT_SYMBOL(validate_sp);
> > +int validate_sp(unsigned long sp, struct task_struct *p)
> > +{
> > +       return validate_sp(sp, p, STACK_FRAME_OVERHEAD);
>
> Hi Nick, I assume this supposed to be validate_sp_size()?  Did you get
> this to compile?

Oops, yeah I think I sent a slightly stale version of the series. I
did fix that one.

Thanks,
Nick