| Date | Tue, 22 Jun 2021 19:35:50 +0200 | From | Borislav Petkov <> | Subject | Re: [patch V3 59/66] x86/fpu/signal: Move initial checks into fpu__sig_restore() |
| |
On Fri, Jun 18, 2021 at 04:19:22PM +0200, Thomas Gleixner wrote:
> Subject: Re: [patch V3 59/66] x86/fpu/signal: Move initial checks into fpu__sig_restore()
fpu__restore_sig() is the name.
> __fpu_sig_restore() is convoluted and some of the basic checks can trivialy be done > in the calling function as well as the final error handling of clearing user state. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > arch/x86/kernel/fpu/signal.c | 76 +++++++++++++++++++++++-------------------- > 1 file changed, 41 insertions(+), 35 deletions(-)
...
> @@ -490,15 +464,47 @@ static inline int xstate_sigframe_size(v > */ > int fpu__restore_sig(void __user *buf, int ia32_frame) > { > + unsigned int size = xstate_sigframe_size(); > void __user *buf_fx = buf; > - int size = xstate_sigframe_size(); > + bool ia32_fxstate = false; > + int ret; > > + if (unlikely(!buf)) { > + fpu__clear_user_states(¤t->thread.fpu);
You could declare
struct fpu *fpu = &tsk->thread.fpu;
above so that it is easier to read, as this call is done twice.
Also, you can do:
int ret = 0;
if (unlikely(!buf)) goto out;
so that the exit paths converge at the end.
> + return 0; > + } > + > + ia32_frame &= (IS_ENABLED(CONFIG_X86_32) || > + IS_ENABLED(CONFIG_IA32_EMULATION)); > + > + /* > + * Only FXSR enabled systems need the FX state quirk. > + * FRSTOR does not need it and can use the fast path. > + */ > if (ia32_frame && use_fxsr()) { > buf_fx = buf + sizeof(struct fregs_state); > size += sizeof(struct fregs_state); > + ia32_fxstate = true; > + } > + > + if (!access_ok(buf, size)) { > + ret = -EACCES; > + goto out; > + } > + > + if (!IS_ENABLED(CONFIG_X86_64) && !static_cpu_has(X86_FEATURE_FPU)) {
cpu_feature_enabled()
> + return fpregs_soft_set(current, NULL, 0, > + sizeof(struct user_i387_ia32_struct), > + NULL, buf);
Err, don't you need to catch retval into ret here and goto out, like before, so that you can call fpu__clear_user_states() on error?
-- Regards/Gruss, Boris.
SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
|