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: <0d717394-eaf0-4d29-1aae-218ffcc8f06b@linux.intel.com>
Date:   Mon, 23 Jan 2017 16:53:25 -0800
From:   Dave Hansen <dave.hansen@...ux.intel.com>
To:     Kevin Hao <haokexin@...il.com>, Yu-cheng Yu <yu-cheng.yu@...el.com>
Cc:     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

>> The fix I am proposing is...
>>
>> 	state->xsave.header.xcomp_bv = XCOMP_BV_COMPACTED_FORMAT |
>> 				       xfeatures_mask;
> 
> Actually I thought about this change before I made this patch, but I don't this
> is the right fix. It is always error prone to init the xcomp_bv to all the
> supported feature. In case like copyin_to_xsaves(), it is possible that the
> features which should be set in xcomp_bv do not equal to all the supported
> features. Please see the following codes in copyin_to_xsaves():
> 	/*
> 	 * The state that came in from userspace was user-state only.
> 	 * Mask all the user states out of 'xfeatures':
> 	 */
> 	xsave->header.xfeatures &= XFEATURE_MASK_SUPERVISOR;
> 
> 	/*
> 	 * Add back in the features that came in from userspace:
> 	 */
> 	xsave->header.xfeatures |= xfeatures;

Hi Kevin,

I think you may be confusing 'xfeatures' with 'xcomp_bv'.  xfeatures
tells you what features are present in the buffer while xcomp_bv tells
you what *format* the buffer is in.

Userspace never dictates the *format* of the kernel buffer, only the
contents of each state.  So, it makes sense that the copyin code would
not (and should not) modify xcomp_bv.

We ensure that xcomp_bv has all supported states set all the time, or
we're *supposed* to.