Re: [PATCH 0/4] XSA-52..54 follow-up

[Ben Guthro <Benjamin.Guthro@citrix.com>]

On 06/04/2013 06:32 AM, Jan Beulich wrote:
>>>> On 04.06.13 at 12:00, Keir Fraser <keir.xen@gmail.com> wrote:
>> On 04/06/2013 08:58, "Jan Beulich" <JBeulich@suse.com> wrote:
>>
>>> The first patch really isn't as much of a follow-up than what triggered
>>> the security issues to be noticed in the first place.
>>>
>>> 1: x86: preserve FPU selectors for 32-bit guest code
>>> 2: x86: fix XCR0 handling
>>> 3: x86/xsave: adjust state management
>>> 4: x86/fxsave: bring in line with recent xsave adjustments
>>>
>>> The first two I would see as candidates for 4.3 (as well as
>>> subsequent backporting, albeit I realize that especially the first
>>> one is non-trivial), while the third is code improvement only,
>>> and the fourth is really just cleanup, and hence I'd be fine with
>>> deferring them until after 4.3.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> I like the patches, 1 & 2 are good bug fixes.
>> Acked-by: Keir Fraser <keir@xen.org>
>>
>> Patch #1 is quite scary though! I wonder really whether these long-lived
>> issues must be fixed right now, let alone backported?
>
> You can also view this as "finally we get these long standing
> issues fixed" - I was aware of the selector corruption since back
> when dealing with the original FXSAVE information leak, just that
> no-one had an issue with this in practice.
>
> Ben, who originally initiated this work, might want to give an
> estimation on how important these fixes are for their practical
> purposes. With that, I'll then defer to George for whether to
> pull them in right now or only once branched.

This was more of an exercise of QA exercising of code paths uncovering a 
bug, rather than real-world usage implications.

Microsoft ships something called the "driver verifier"[1] in every OS 
after XP, that can dynamically be turned on for any driver. When enabled 
for some Citrix PV drivers, they would bug-check (occasionally) during 
QA nightly test runs on 32bit OSes on this XSAVE/XRSTOR data mismatch.

To the best of my knowledge, outside of this somewhat contrived 
scenario, we never saw this fail in any real-world usage. Therefore, I 
think the assessment of deferring until after 4.3 is prudent, given the 
release cycle stage.

With my downstream hat on, it would certainly be nice to see these 
backported to 4.2 though, once in mainline, so we don't have to carry a 
patch. However, we can always do so, if necessary.

Ben

[1] http://support.microsoft.com/kb/244617