Subject: RE: [PATCH v4 02/10] powerpc/signal: Add unsafe_copy_{vsx, fpr}_from_user()

From: "Christopher M. Riedl" <[email-masked]>
Date: Mon, 01 Feb 2021 09:55:44 -0600

On Thu Jan 28, 2021 at 4:38 AM CST, David Laight wrote:
> From: Christopher M. Riedl
> > Sent: 28 January 2021 04:04
> > 
> > Reuse the "safe" implementation from signal.c except for calling
> > unsafe_copy_from_user() to copy into a local buffer.
> > 
> > Signed-off-by: Christopher M. Riedl <[email-masked]>
> > ---
> >  arch/powerpc/kernel/signal.h | 33 +++++++++++++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> > 
> > diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
> > index 2559a681536e..c18402d625f1 100644
> > --- a/arch/powerpc/kernel/signal.h
> > +++ b/arch/powerpc/kernel/signal.h
> > @@ -53,6 +53,33 @@ unsigned long copy_ckfpr_from_user(struct task_struct *task, void __user *from);
> >  				&buf[i], label);\
> >  } while (0)
> > 
> > +#define unsafe_copy_fpr_from_user(task, from, label)	do {		\
> > +	struct task_struct *__t = task;					\
> > +	u64 __user *__f = (u64 __user *)from;				\
> > +	u64 buf[ELF_NFPREG];						\
>
> How big is that buffer?
> Isn't is likely to be reasonably large compared to a reasonable
> kernel stack frame.
> Especially since this isn't even a leaf function.
>

I think Christophe answered this - I don't really have an opinion either
way. What would be a 'reasonable' kernel stack frame for reference?

> > +	int i;								\
> > +									\
> > +	unsafe_copy_from_user(buf, __f, ELF_NFPREG * sizeof(double),	\
>
> That really ought to be sizeof(buf).
>

Agreed, I will fix this. Thanks!

> David
>
>
> > +				label);					\
> > +	for (i = 0; i < ELF_NFPREG - 1; i++)				\
> > +		__t->thread.TS_FPR(i) = buf[i];				\
> > +	__t->thread.fp_state.fpscr = buf[i];				\
> > +} while (0)
> > +
> > +#define unsafe_copy_vsx_from_user(task, from, label)	do {		\
> > +	struct task_struct *__t = task;					\
> > +	u64 __user *__f = (u64 __user *)from;				\
> > +	u64 buf[ELF_NVSRHALFREG];					\
> > +	int i;								\
> > +									\
> > +	unsafe_copy_from_user(buf, __f,					\
> > +				ELF_NVSRHALFREG * sizeof(double),	\
> > +				label);					\
> > +	for (i = 0; i < ELF_NVSRHALFREG ; i++)				\
> > +		__t->thread.fp_state.fpr[i][TS_VSRLOWOFFSET] = buf[i];	\
> > +} while (0)
> > +
> > +
> >  #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> >  #define unsafe_copy_ckfpr_to_user(to, task, label)	do {		\
> >  	struct task_struct *__t = task;					\
> > @@ -80,6 +107,10 @@ unsigned long copy_ckfpr_from_user(struct task_struct *task, void __user *from);
> >  	unsafe_copy_to_user(to, (task)->thread.fp_state.fpr,	\
> >  			    ELF_NFPREG * sizeof(double), label)
> > 
> > +#define unsafe_copy_fpr_from_user(task, from, label)			\
> > +	unsafe_copy_from_user((task)->thread.fp_state.fpr, from,	\
> > +			    ELF_NFPREG * sizeof(double), label)
> > +
> >  static inline unsigned long
> >  copy_fpr_to_user(void __user *to, struct task_struct *task)
> >  {
> > @@ -115,6 +146,8 @@ copy_ckfpr_from_user(struct task_struct *task, void __user *from)
> >  #else
> >  #define unsafe_copy_fpr_to_user(to, task, label) do { } while (0)
> > 
> > +#define unsafe_copy_fpr_from_user(task, from, label) do { } while (0)
> > +
> >  static inline unsigned long
> >  copy_fpr_to_user(void __user *to, struct task_struct *task)
> >  {
> > --
> > 2.26.1
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes,
> MK1 1PT, UK
> Registration No: 1397386 (Wales)