Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 05/11] common/grant_table: block speculative out-of-bound accesses

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.01.19 at 14:47, <nmanthey@xxxxxxxxx> wrote:
> On 1/29/19 10:46, Jan Beulich wrote:
>>>>> Norbert Manthey <nmanthey@xxxxxxxxx> 01/29/19 9:35 AM >>>
>>> I am aware that both version use the same base array, and access it via
>>> different macros, which essentially partition the array based on the
>>> size of the respective struct. The underlying raw array has the same
>>> size for both version.
>> And this is the problem afaics: If a guest has requested its grant table to
>> be sized as a single page, this page can fit twice as many entries for
>> v1 than it can fit for v2. Hence the v1 grant reference pointing at the last
>> entry would point at the last entry in the (not mapped) second page for v2.
> 
> I might understand the code wrong, but a guest would ask to get at most
> N grant frames, and this number cannot be increased afterwards, i.e. the
> field gt->max_grant_frames is written exactly once. Furthermore, the
> void** shared_raw array is allocated and written exactly once with
> sufficient pointers for, namely gt->max_grant_frames many in function
> grant_table_init. Hence, independently of the version being used, at
> least the shared_raw array cannot be used for out-of-bound accesses
> during speculation with my above evaluate_nospec.

I'm afraid I'm still not following: A give number of pages is worth
twice as many grants in v1 than it is in v2. Therefore a v1 grant
reference to a grant entry tracked in the second half of the
first page would cause a speculative access to anywhere in the
second page when wrongly interpreted as a v2 ref.

> That being said, let's assume we have a v1 grant table, and speculation
> uses the v2 accesses. In that case, an existing and zero-initialized
> entry of shared_raw might be used in the first part of the
> shared_entry_v2 macro, and even if that pointer would be non-NULL, the
> page it would point to would have been cleared when growing the grant
> table in function gnttab_grow_table.

Not if the v1 ref is no smaller than half the maximum number of
v1 refs. In that case, if taken as a v2 ref, ->shared_raw[]
would need to be twice as big to cope with the larger index
(resulting from the smaller divisor in shared_entry_v2()
compared to shared_entry_v1()) in order to not be overrun.

Let's look at an example: gref 256 points into the middle of
the first page when using v1 calculations, but at the start
of the second page when using v2 calculations. Hence, if the
maximum number of grant frames was 1, we'd overrun the
array, consisting of just a single element (256 is valid as a
v1 gref in that case, but just out of bounds as a v2 one).

Furthermore, even if ->shared_raw[] itself could not be overrun,
an entry of it being NULL could be a problem with PV guests, who
can install a translation for the first page of the address space,
and thus perhaps partly control subsequent speculative execution.

Jan



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