Re: [PATCH for-4.17?] x86: support data operand independent timing mode

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On Fri, Sep 30, 2022 at 11:25:12AM +0000, Andrew Cooper wrote:
> On 15/09/2022 11:04, Jan Beulich wrote:
> > [1] specifies a long list of instructions which are intended to exhibit
> > timing behavior independent of the data they operate on. On certain
> > hardware this independence is optional, controlled by a bit in a new
> > MSR. Provide a command line option to control the mode Xen and its
> > guests are to operate in, with a build time control over the default.
> > Longer term we may want to allow guests to control this.
> >
> > Since Arm64 supposedly also has such a control, put command line option
> > and Kconfig control in common files.
> >
> > [1] 
> > https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/best-practices/data-operand-independent-timing-isa-guidance.html
> >
> > Requested-by: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> This patch should not be taken; at least not in this form.  The whole
> DOITM infrastructure is currently under argument, for being impossible
> to use appropriately.
> 
> I understand why Qubes want this blanket set, but it is a steep penalty
> to pay;  It's only code which is already written trying to be constant
> time/cache which gains any security from this. 

Based on the bit description, I'd say rather "prevent _breaking_
security of the code already written". It is not setting this bit that
change behaviour on new parts, but it's not setting it that breaks
previous guarantees. It's really bizarre design choice from Intel...

>  On current parts, using
> SSBD has the same behaviour, but this isn't expected to remain true in
> the future.
> 
> Forcing it on behind the back of a VM is mutually exclusive with
> enumerating it for VMs to use at some point in the future when we have
> the capability to.  i.e. specifically, you are not able to maintain the
> ABI/API in this patch in the future.

Regarding the current behavior of the hypervisor (without this patch):
will guest see DOITM present but not set? Or will they not see it at
all?

Documentation clearly state:
    For Intel® Core™ family processors based on microarchitectures
    before Ice Lake and Intel Atom® family processors based on
    microarchitectures before Gracemont that do not enumerate
    IA32_UARCH_MISC_CTL, developers may assume that the instructions listed
    here operate as if DOITM is enabled.

So, if guests will not see the feature at all, it Xen should set it
unconditionally, to remain compliant with expected hardware behaviour.

If guests will see the thing (and see it not enabled), then indeed it's
less clear what should be done right now (but I'd still like to have an
option to enable it).

> If we do move forward with something like this (under the strict
> understanding that the behaviour is going to change in the future), then
> "DIT" is too short of an acronym to use.  Amongst other things, it's not
> "data independent timing"; it's "controls for forcing ..." which is
> important because these are going to be vendor specific, if even needed
> in the first place.
> 
> ~Andrew

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature