Re: [patch V4 09/65] x86/fpu: Sanitize xstateregs_set()

From: Dave Hansen
Date: Mon Jul 25 2022 - 17:26:09 EST


On 7/25/22 10:57, Andrei Vagin wrote:
> On Mon, Jul 25, 2022 at 10:47 AM Dave Hansen <dave.hansen@xxxxxxxxx> wrote:
>> On 7/13/22 21:04, Andrei Vagin wrote:
>>>> - /*
>>>> - * mxcsr reserved bits must be masked to zero for security reasons.
>>>> - */
>>>> - xsave->i387.mxcsr &= mxcsr_feature_mask;
>>>> -
>>>> - /*
>>>> - * In case of failure, mark all states as init:
>>>> - */
>>>> - if (ret)
>>>> - fpstate_init(&fpu->state);
>>>> + fpu__prepare_write(fpu);
>>>> + ret = copy_kernel_to_xstate(&fpu->state.xsave, kbuf ?: tmpbuf);
>>> This change breaks gVisor. Now, when we set a new fpu state without
>>> fp,sse and ymm via ptrace, mxcsr isn't reset to the default value.
>>> The issue is reproduced only on hosts without xsaves. On hosts with
>>> xsaves, it works as expected.
>> Is gVisor some out-of-tree kernel code or is it just an proprietary KVM
>> user? In other words, is this an issue with the upstream kernel itself
>> or does it require kernel modification?
> This is on the upstream kernel without any modifications. And this is the case
> when gVisor uses ptrace to trap syscalls. KVM isn't used in this case.

Do you happen to have a quick reproducer for this, or at least the
contents of the buffer that you are trying to restore?

I'm struggling to if we broke a meaningful part of the ABI, or if this
is fallout from the kernel being more hesitant to zap the whole buffer
when it has problems.