Re: [Xen-devel] [PATCH L1TF v8 9/9] common/grant_table: block speculative out-of-bound accesses

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.03.19 at 09:15, <nmanthey@xxxxxxxxx> wrote:
> On 2/28/19 11:00, Jan Beulich wrote:
>>>>> On 27.02.19 at 14:01, <nmanthey@xxxxxxxxx> wrote:
>>> On 2/25/19 17:46, Jan Beulich wrote:
>>>> I would really like to ask that I (or someone else) don't need to
>>>> go through and list remaining version checks again - after all I
>>>> had done so for v6 already, and I didn't go through all of them
>>>> again for v7 assuming that you would have worked through the
>>>> entire set.
>>> So, here is the annotation for all of them. Anyone that I did not
>>> include in the list has been fixed in previous versions, or will be
>>> fixed in the next version:
>>>
>>> git grep -np version common/grant_table.c
>>>
>>> common/grant_table.c:831:static int _set_status(unsigned gt_version,
>>> common/grant_table.c:840:    if ( gt_version == 1 )
>>>
>>> -> I do not see how out-of-bound accesses happen in the called functions
>>> there.
>> Both functions get shah passed into them, which may point to the
>> other version's layout. Earlier fences don't help speculation on this
>> conditional.
> Whenever this function is called, the shah field has the same structure
> for both versions.

But it does matter how it was obtained, doesn't it? There's
shared_raw[] involved here once again. Hence we're safe here
only if the caller has already suitably guarded against speculation.
Which may indeed be the case, but which then needs to be called
out in the description.

> The v2 grant entry has a plain header hdr, which is
> used here, and which is forwarded to the set_status method. Another
> property that we never talked about here is that we only care about
> cross cache-line accesses. As soon as any byte is touched on a cache
> line, the whole line is brought into the cache. As the code around the
> set_status method already pulls in the corresponding cache line, that
> code looks okay to me from a L1TF perspective.

So here as well as for the other points - if you decide to omit guarding,
please explain why in the commit message.

Jan



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