2017-06-15 21:45:07

by Andy Lutomirski

[permalink] [raw]
Subject: Re: [RFC] KVM: SVM: do not drop VMCB CPL to 0 if SS is not present

On Tue, May 30, 2017 at 9:05 AM, Paolo Bonzini <[email protected]> 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!

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?

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?

--Andy


2017-06-16 08:44:45

by Roman Pen

[permalink] [raw]
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 <[email protected]> wrote:
> On Tue, May 30, 2017 at 9:05 AM, Paolo Bonzini <[email protected]> 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/[email protected]/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

2017-06-16 16:40:24

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [RFC] KVM: SVM: do not drop VMCB CPL to 0 if SS is not present



On 15/06/2017 23:44, Andy Lutomirski wrote:
> On Tue, May 30, 2017 at 9:05 AM, Paolo Bonzini <[email protected]> wrote:
>> On 30/05/2017 17:58, Roman Penyaev wrote:
>>> 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!

For AMD, "a data-segment-descriptor DPL field is ignored in 64-bit mode"
(4.8.2). This is unlike Intel, where SS.DPL is the CPL.

After SYSRET, CPL is always 3, even if CS.RPL != 3.

> 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.

Currently that's almost what happens, except the "migration" of the CPL
field into SS.DPL only happens when going through QEMU.

> 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?

No, any case where STAR is programmed with RPL != 3 is garbage.

Paolo