Re: [Xen-devel] [PATCH L1TF v10 7/8] common/grant_table: block speculative out-of-bound accesses

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 20.05.19 at 16:27, <nmanthey@xxxxxxxxx> wrote:
> On 3/29/19 18:11, Jan Beulich wrote:
>>>>> On 14.03.19 at 13:50, <nmanthey@xxxxxxxxx> wrote:
>>> @@ -2410,9 +2448,11 @@ acquire_grant_for_copy(
>>>          PIN_FAIL(gt_unlock_out, GNTST_bad_gntref,
>>>                   "Bad grant reference %#x\n", gref);
>>>  
>>> -    act = active_entry_acquire(rgt, gref);
>>> +    /* This call ensures the above check cannot be bypassed speculatively 
>>> */
>>>      shah = shared_entry_header(rgt, gref);
>>> -    if ( rgt->gt_version == 1 )
>>> +    act = active_entry_acquire(rgt, gref);
>>> +
>>> +    if ( evaluate_nospec(rgt->gt_version == 1) )
>>>      {
>>>          sha2 = NULL;
>>>          status = &shah->flags;
>> What about the second version check further down in this function?
> That one should be fine, as it exists that function afterwards, and
> represents an abort path that is valid for both versions.

That's not obvious from looking at just the if() in question. For
example, fixup_status_for_copy_pin() gets handed "status" as
an argument, which is version dependent. It seems quite likely
indeed that no code changes are here, but this imo again needs
discussing/explaining in the commit message.

>>> @@ -3838,6 +3886,9 @@ static int gnttab_get_status_frame_mfn(struct domain 
>>> *d,
>>>              return -EINVAL;
>>>      }
>>>  
>>> +    /* Make sure idx is bounded wrt nr_status_frames */
>>> +    block_speculation();
>>> +
>>>      *mfn = _mfn(virt_to_mfn(gt->status[idx]));
>> Why not array_access_nospec()? And how is this different from
>> gnttab_get_shared_frame_mfn(), which you don't change?
> 
> This idx access is to a version dependent structure, and hence
> array_index_nospec is not good enough to catch the version difference
> case as well.

But the comment talks about the array bound only.

Jan



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