Re: Setting constant-time mode CPU flag

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.09.2022 08:40, Demi Marie Obenour wrote:
> On Wed, Sep 14, 2022 at 08:36:02AM +0200, Jan Beulich wrote:
>> On 13.09.2022 19:22, Demi Marie Obenour wrote:
>>> On Tue, Sep 13, 2022 at 04:47:24PM +0200, Jan Beulich wrote:
>>>> On 13.09.2022 16:22, Demi Marie Obenour wrote:
>>>>> On Tue, Sep 06, 2022 at 10:01:00AM +0000, Andrew Cooper wrote:
>>>>>> On 06/09/2022 10:52, Jan Beulich wrote:
>>>>>>> On 02.09.2022 04:05, Demi Marie Obenour wrote:
>>>>>>>> On Intel chips (Ice Lake and later) and ARM64, a bit needs to be set in
>>>>>>>> a CPU register to enforce constant-time execution.  Linux plans to set
>>>>>>>> this bit by default; Xen should do the same.  See
>>>>>>>> https://lore.kernel.org/lkml/YwgCrqutxmX0W72r@xxxxxxxxx/T/ for details.
>>>>>>>> I recommend setting the bit unconditionally and ignoring guest attempts
>>>>>>>> to change it.
>>>>>>> I don't think we ought to set it by default; I can see reasons why 
>>>>>>> kernels
>>>>>>> may want to set it by default (providing a way to turn it off). In Xen
>>>>>>> what I think we need is exposure of the bit to be guest-controllable.
>>>>>>
>>>>>> We absolutely should not have it set by default.  It's a substantial
>>>>>> overhead for something that is only applicable to code which otherwise
>>>>>> crafted to be constant-time.
>>>>>
>>>>> Either Xen needs to set the bit by default, or guests need to both know
>>>>> the bit needs to be set and be able set it.  Otherwise code that *is*
>>>>> intended to be constant-time has no way to protect itself.
>>>>>
>>>>>> As for why Xen doesn't enumerate/virtualise it, that's because
>>>>>> virtualising MSR_ARCH_CAPS for guests is still not working yet, so the
>>>>>> feature can't be enumerated yet even if we did support context switching 
>>>>>> it.
>>>>>
>>>>> Intel and ARM64 guarantee that CPUs that do not enumerate this flag
>>>>> behave as if it is set unconditionally.
>>>>
>>>> I'm not qualified to talk about the Arm side, but may I ask what you've
>>>> derived this statement from for Intel? The doc page referenced by the
>>>> link you did provide (still in context above) specifically further links
>>>> to a page listing instruction with data operand independent timing. All
>>>> other instructions, as I conclude, have variable timing unless the bit
>>>> in ARCH_CAPS enumerates DOITM and then the new MSR bit (of the same name)
>>>> is set.
>>>
>>> My understanding is that only instructions in the constant-time subset
>>> are ever guaranteed to be constant time.
>>
>> Hmm, yes, I did overlook respective wording in the doc.
>>
>>>  On architectures where DOITM
>>> is not enumerated, this guarantee is unconditional.
>>
>> I have to admit I'm suspicious of this "guarantee".
> 
> Do you mean that previous CPUs had a vulnerability that has no fix?

I'm not sure I'd call it a vulnerability, but at least if going back far
enough in history I think you'll find insns on the list which don't have
invariant timing. Like with other documentation on e.g. speculation
issues I take it that Intel simply doesn't consider sufficiently old
CPUs relevant anymore for such new documents.

>>>  On architectures
>>> where DOITM is enumerated, this guarantee only holds when DOITM is set.
>>> Therefore, it is critical that on CPUs that enumerate DOITM, Xen does
>>> one of the following:
>>>
>>> - Ensure that all vCPUs enumerate DOITM, and virtualize the DOITM MSR
>>>   bit for use by guests.
>>>
>>> - Set DOITM by default.
>>>
>>> Since Xen does not support virtualizing MSR_ARCH_CAPS, vCPUs cannot
>>> enumerate DOITM.  Therefore, the only secure option is to set DOITM by
>>> default, so that guests do not need to be aware of it.
>>
>> I can see where you're coming from, but I also agree with Andrew that
>> the resulting loss of performance is a counter-indication to making
>> this the (universal) default. What I could see us doing is make this
>> both Kconfig and command line controllable.
> 
> How large is the loss of performance?

I have no (practical) way to know.

Jan

>  Linux seems to be setting the
> flag unconditionally, so I think my point about guests needing to be
> able to ensure the flag is set stands.  The default can be changed once
> Xen gets support for virtualizing the bit properly, but until then
> unconditionally setting DOITM seems to be the only safe option.