Re: [PATCH SpectreV1+L1TF v6 3/9] x86/hvm: block speculative out-of-bound accesses

["Jan Beulich" <JBeulich@suse.com>]

>>> On 18.02.19 at 15:47, <nmanthey@amazon.de> wrote:
> On 2/15/19 12:46, Jan Beulich wrote:
>> A code change is, imo, not even worthwhile considering to be put
>> in if it is solely based on the observations made with a limited set
>> of compilers and/or options. This might indeed help you, if you
>> care only about one specific environment. But by putting this in
>> (and perhaps even backporting it) we're sort of stating that the
>> issue is under control (to the best of our abilities, and for the given
>> area of code). For everyone.
> I do not see how a fix for problems like the discussed one could enter
> the code base given the above conditions.

Well, on one hand I can understand your frustration. Otoh the
fundamental thing here is that "fix" to me means something that
actually fixes an issues independent of "weather conditions". I'd
at best call it a workaround here, yet even then I question its
usefulness given the limitations.

But please don't forget - I'm not the only one who can potentially
approve of changes which are proposed only in the hope that
they may help, without any guarantees. If other maintainers
think we should take such changes, I won't veto them going in
as long as it is made crystal clear that the same underlying issue
may re-surface at any time, for code that was supposedly "fixed"
already. It's just that I'm not going to ack anything like this myself.

> However, for this very
> specific fix, there fortunately is a comparison wrt a constant, and
> there are many instructions until the potential speculative out-of-bound
> access might happen, so that not fixing the two above access is fine for
> me. While I cannot guarantee that it is not possible, we did not manage
> to come up with a PoC for these two places with the effort we put into this.

Okay, thanks for letting us know.

>> So, to answer your question: From what we know, we simply
>> can't take a decision, at least not between the two proposed
>> variants of how to change the code. If there was a variant that
>> firmly worked, then there would not even be a need for any
>> discussion. And again from what we know, there is one
>> requirement that need to be fulfilled for a change to be
>> considered "firmly working": The index needs to be in a register.
>> There must not be a way for the compiler to undermine this,
>> be it by CSE or any other means.
>>
>> Considering changes done elsewhere, of course this may be
>> taken with a grain of salt. In other places we also expect the
>> compiler to not emit unreasonable code (e.g. needlessly
>> spilling registers to memory just to then reload them). But
>> while that's (imo) a fine expectation to have when an array
>> index is used just once, it is unavoidably more complicated in
>> the case here as well as in the grant table one.
> 
> Unless you outline a path forward to fix the above two gadgets, I will
> not include the above hunks in the next version of the series.

I would be more than happy to outline a path, but I simply see
none which would provide guarantees.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel