linux-kernel - Re: [tip:x86/urgent] x86/fpu: Set the xcomp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170124051833.GA10737@pek-khao-d1>
Date:   Tue, 24 Jan 2017 13:18:33 +0800
From:   Kevin Hao <haokexin@...il.com>
To:     Dave Hansen <dave.hansen@...ux.intel.com>
Cc:     Yu-cheng Yu <yu-cheng.yu@...el.com>, fenghua.yu@...el.com,
        dvlasenk@...hat.com, peterz@...radead.org, oleg@...hat.com,
        mingo@...nel.org, linux-kernel@...r.kernel.org, brgerst@...il.com,
        luto@...nel.org, bp@...en8.de, jpoimboe@...hat.com, hpa@...or.com,
        quentin.casasnovas@...cle.com, tglx@...utronix.de,
        torvalds@...ux-foundation.org, riel@...hat.com,
        linux-tip-commits@...r.kernel.org
Subject: Re: [tip:x86/urgent] x86/fpu: Set the xcomp_bv when we fake up a
 XSAVES area

On Mon, Jan 23, 2017 at 06:38:42PM -0800, Dave Hansen wrote:
> On 01/23/2017 06:09 PM, Kevin Hao wrote:
> > On Mon, Jan 23, 2017 at 06:01:10PM -0800, Dave Hansen wrote:
> >> On 01/23/2017 05:50 PM, Kevin Hao wrote:
> >>> According to the ISA manual, XSAVES also set the XCOMP_BV[62:0]. My code only
> >>> try to be compatible with what the cpu does when excuting XSAVES. The following
> >>> is quoted from 325462-sdm-vol-1-2abcd-3abcd.pdf.
> >>>   The XSAVES instructions sets bit 63 of the XCOMP_BV field of the XSAVE header while writing RFBM[62:0] to
> >>>   XCOMP_BV[62:0]. The XSAVES instruction does not write any part of the XSAVE header other than the XSTATE_BV
> >>>   and XCOMP_BV fields.
> >> What purpose does it serve to make copyin_to_xsaves() set that bit,
> > We try to fake up a memory area which is supposed to be composed by XSAVES
> > instruction. My code is just trying to do what the XSAVES do.
> 
> No. copyin_to_xsaves() copies data into an *existing* XSAVES-formatted
> buffer.  If you want to change what it does, fine, but that's not what
> it does or tries to do today.

No, I didn't change what the function copyin_to_xsaves() does. I just tried to
fix the bug in it.

> 
> >> other than helping to hide bugs?
> > Why do you think it hide the bug? In contrast, I think my patch fixes what the
> > bug really is. The memory area we fake up is bug, we should fix it there.
> 
> Yu-cheng found the bug.  That bug will probably manifest in other code
> paths than copyin_to_xsaves().  If we did your patch, it would hide the
> bug in those other code paths.

The XCOMP_BV[62:0] is supposed to be updated by XSAVEC/XSAVES instructions.
It should not be touched by the software in theory except some special cases
like what we do in copyin_to_xsaves(). Trying to init the XCOMP_BV[62:0] to some
assuming values in fpstate_init() should be error prone and just paper over the
real bug.

Thanks,
Kevin

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)