On Mon Feb 1, 2021 at 10:54 AM CST, Gabriel Paubert wrote:
> On Mon, Feb 01, 2021 at 09:55:44AM -0600, Christopher M. Riedl wrote:
> > 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 <c...@codefail.de>
> > > > ---
> > > > 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?
>
> See include/linux/poll.h, where the limit is of the order of 800 bytes
> and the number of entries in an on stack array is chosen at compile time
> (different between 32 and 64 bit for example).
>
> The values are used in do_sys_poll, which, with almost 1000 bytes of
> stack footprint, appears close to the top of "make checkstack". In
> addition do_sys_poll has to call the ->poll function of every file
> descriptor in its table, so it is not a tail function.
>
> This 264 bytes array looks reasonable, but please use 'make checkstack'
> to verify that the function's total stack usage stays within reason.
Neat, looks like total usage is a bit larger but still reasonable and
less than half of 800B:
0xc000000000017e900 __unsafe_restore_sigcontext.constprop.0 [vmlinux]:352
Thanks for the tip!
>
> Gabriel
>
> >
> > > > + 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)
> >
>
