lkml.org 
[lkml]   [2021]   [Jun]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch V2 13/52] x86/fpu: Clean up fpregs_set()
    On Mon, Jun 14, 2021 at 05:44:21PM +0200, Thomas Gleixner wrote:
    > From: Andy Lutomirski <luto@kernel.org>
    >
    > fpregs_set() had unnecessary complexity to support short or nonzero-offset
    > writes and to handle the case in which a copy from userspace overwrites
    > some of the target buffer and then fails. Support for partial writes is
    > useless -- just require that the write have offset 0 and the correct size,
    > and copy into a temporary kernel buffer to avoid clobbering the state if
    > the user access fails.
    >
    > Signed-off-by: Andy Lutomirski <luto@kernel.org>
    > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    > ---
    > V2: New patch. Picked up from Andy
    > ---
    > arch/x86/kernel/fpu/regset.c | 27 ++++++++++++++-------------
    > 1 file changed, 14 insertions(+), 13 deletions(-)
    > ---
    > --- a/arch/x86/kernel/fpu/regset.c
    > +++ b/arch/x86/kernel/fpu/regset.c
    > @@ -305,31 +305,32 @@ int fpregs_set(struct task_struct *targe
    > struct user_i387_ia32_struct env;
    > int ret;
    >
    > - fpu__prepare_write(fpu);
    > - fpstate_sanitize_xstate(fpu);
    > + /* No funny business with partial or oversized writes is permitted. */
    > + if (pos != 0 || count != sizeof(struct user_i387_ia32_struct))
    > + return -EINVAL;
    >
    > if (!boot_cpu_has(X86_FEATURE_FPU))

    cpu_feature_enabled(), and below too, while you're at it.

    > return fpregs_soft_set(target, regset, pos, count, kbuf, ubuf);
    >
    > - if (!boot_cpu_has(X86_FEATURE_FXSR))
    > - return user_regset_copyin(&pos, &count, &kbuf, &ubuf,
    > - &fpu->state.fsave, 0,
    > - -1);
    > + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &env, 0, -1);
    > + if (ret)
    > + return ret;
    >
    > - if (pos > 0 || count < sizeof(env))
    > - convert_from_fxsr(&env, target);
    > + fpu__prepare_write(fpu);
    >
    > - ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &env, 0, -1);
    > - if (!ret)
    > + if (static_cpu_has(X86_FEATURE_FXSR))
    > convert_to_fxsr(&target->thread.fpu.state.fxsave, &env);
    > + else
    > + memcpy(&target->thread.fpu.state.fsave, &env, sizeof(env));
    >
    > /*
    > - * update the header bit in the xsave header, indicating the
    > + * Update the header bit in the xsave header, indicating the
    > * presence of FP.
    > */
    > - if (boot_cpu_has(X86_FEATURE_XSAVE))
    > + if (static_cpu_has(X86_FEATURE_XSAVE))
    > fpu->state.xsave.header.xfeatures |= XFEATURE_MASK_FP;
    > - return ret;
    > +
    > + return 0;
    > }

    With that addressed:

    Reviewed-by: Borislav Petkov <bp@suse.de>

    Thx.

    --
    Regards/Gruss,
    Boris.

    SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg

    \
     
     \ /
      Last update: 2021-06-16 17:45    [W:5.197 / U:0.960 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site