Message-ID: <CAJrWOzCKVDqWPmMLzq6nU-ebehN70j6F+1+Pk3ff_k3sj2vqMw@mail.gmail.com>
Date:   Fri, 16 Jun 2017 10:44:21 +0200
From:   Roman Penyaev <roman.penyaev@...fitbricks.com>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Mikhail Sennikovskii <mikhail.sennikovskii@...fitbricks.com>,
        Gleb Natapov <gleb@...nel.org>, kvm list <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] KVM: SVM: do not drop VMCB CPL to 0 if SS is not present

On Thu, Jun 15, 2017 at 11:44 PM, Andy Lutomirski <luto@...nel.org> wrote:
> On Tue, May 30, 2017 at 9:05 AM, Paolo Bonzini <pbonzini@...hat.com> wrote:
>>
>>
>> On 30/05/2017 17:58, Roman Penyaev wrote:
>>> Indeed, what is left is eventually take it from SS.RPL. J.
>>
>> Ahah! :)  But I only suggested that in specific cases.
>>
>>> But jokes aside,  with your last patch you seems fixed a race problem
>>> when "CS.RPL is not equal to the CPL in the few instructions between
>>> setting CR0.PE and reloading CS".
>>
>> Yes, exactly.  The symptom was a crash (triple fault) when you kept
>> interrupting with "info cpus" a guest that repeatedly went to protected
>> mode and back to real mode.
>>
>>> We will have CPL in var->dpl, and it seems ok.  All we need is not
>>> to lose it on the way kernel->userspace->kernel.
>>
>> You're right.  So what do you think of the other suggestion (svm.c
>> doesn't clear attributes for unusable registers, QEMU only clears P for
>> unusable registers)?
>
> AMD CPUs really allow setting RPL in MSR_*STAR to something other than
> 3 and then blindly copy the result to SS.DPL when SYSRET happens?
> Ugh!

Hm, MSR_*START ? I don't know.  The original problem was that AMD CPU
allows you to set any CPL in VMCB.CPL, which can be totally different
from CS.RPL.  And VMCB.CPL was changed from QEMU userspace side.

> I wonder if we can sweep that particular problem under the rug by
> saying that, as a KVM guest, you can't program STAR.RPL != 3?  Or
> would that require us to set an intercept that we don't want to set?

Couple of weeks ago I sent modified patches to kvm/svm and to QEMU
as well.  Sorry, I forgot you to add to CC, so here are the links:

https://patchwork.kernel.org/patch/9758889/
https://www.mail-archive.com/qemu-devel@nongnu.org/msg454368.html

(who knows why it is so difficult to find cached patch in those
 mail archives? always different resources)

> Alternatively, is there ever a case where CPL == 3, SS.DPL != 3 and
> non-root code can observe the fact that SS.DPL != 3?  If not, maybe
> KVM could just change SS.DPL to 3 whenever it reads out SS if CPL ==
> 3.  Then CPL really could live in the SS state even on SVM.  In other
> words, if a weird guest forces SS.RPL ! = 3 by programming garbage
> into *STAR and doing SYSRET, could that guest tell the difference if
> we non-deterministically changed SS.DPL back to 3 out from under it?
> Or is there some nasty case in which SS.DPL == 0, CPL == 3, SS is
> valid and you're in compat mode, and you expect stack access to fail
> because SS.DPL < CPL?

The idea of those patches above is simple: CPL is fetched from VMCB.CPL
and then the value is stored in SS.DPL field, then is sent to userspace
side.  Userspace side (QEMU) does not touch SS.DPL even if segment is
not present.  So in few words: do not touch, corrupt, spoil CPL on the
way kernel->userspace->kernel.  That is symmetric and guarantees us
that VMCB.CPL will be correctly restored from SS.DPL.

--
Roman

Powered by blists - more mailing lists