Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Andy Lutomirski <luto@amacapital.net>
Mime-Version: 1.0 (1.0)
Subject: Re: [PATCH v4 14/22] x86/fpu/xstate: Expand the xstate buffer on the first use of dynamic user state
Date:   Wed, 24 Mar 2021 15:12:43 -0700
Message-Id: <95242657-E9AC-470C-9834-40BDA2E852B9@amacapital.net>
References: <90ea8d01-7fa5-100e-8590-d1ed80eb0d6a@intel.com>
Cc:     Len Brown <lenb@kernel.org>,
        "Liu, Jing2" <jing2.liu@linux.intel.com>,
        Andy Lutomirski <luto@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Borislav Petkov <bp@suse.de>, Ingo Molnar <mingo@kernel.org>,
        X86 ML <x86@kernel.org>, Len Brown <len.brown@intel.com>,
        "Liu, Jing2" <jing2.liu@intel.com>,
        "Ravi V. Shankar" <ravi.v.shankar@intel.com>,
        LKML <linux-kernel@vger.kernel.org>,
        "Bae, Chang Seok" <chang.seok.bae@intel.com>
In-Reply-To: <90ea8d01-7fa5-100e-8590-d1ed80eb0d6a@intel.com>
To:     Dave Hansen <dave.hansen@intel.com>
Precedence: bulk


> On Mar 24, 2021, at 2:58 PM, Dave Hansen <dave.hansen@intel.com> wrote:
>=20
> =EF=BB=BFOn 3/24/21 2:42 PM, Andy Lutomirski wrote:
>>>>> 3. user space always uses fully uncompacted XSAVE buffers.
>>>>>=20
>>>> There is no reason we have to do this for new states. Arguably we
>>>> shouldn=E2=80=99t for AMX to avoid yet another altstack explosion.
>>> The thing that's worried me is that the list of OS-enabled states is
>>> visible to apps via XGETBV.  It doesn't seem too much of a stretch to
>>> think that apps will see AMX enabled with XGETBV and them assume that
>>> it's on the signal stack.
>>>=20
>>> Please tell me I'm being too paranoid.  If we can break this
>>> assumption, it would get rid of a lot of future pain.
>> There are no AMX apps. I sure hope that there are no apps that
>> enumerate xfeatures with CPUID and try to decode the mess in the
>> signal stack.
>=20
> I don't think they quite need to decode it in order to be screwed over a
> bit.  For instance, I don't think it's too crazy if someone did:
>=20
>    xcr0 =3D xgetbv(0);
>    xrstor(xcr0, &sig_stack[something]);
>    // change some registers
>    xsave(xcr0, &sig_stack[something]);
>=20
> The XRSTOR would work fine, but the XSAVE would overflow the stack
> because it would save the AMX state.  It also *looks* awfully benign.
> This is true even if the silly signal handler didn't know about AMX at
> *ALL*.
>=20
> A good app would have checked that the xfeatures field in the header
> matched xcr0.

Ugh.

On the other hand, if we require a syscall to flip the AMX bit in XCR0, we c=
ould maybe get away with saying that programs that flip the bit and don=E2=80=
=99t understand the new ABI get to keep both pieces.

I don=E2=80=99t live futzing with the ABI like this, but AMX is really only b=
arely compatible with everything before it.=