Re: [Xen-devel] [PATCH L1TF MDS GT v4 1/2] common/grant_table: harden bound accesses

[Jan Beulich <JBeulich@xxxxxxxx>]

On 30.07.2019 15:15, Norbert Manthey wrote:
> Guests can issue grant table operations and provide guest controlled
> data to them. This data is used as index for memory loads after bound
> checks have been done. To avoid speculative out-of-bound accesses, we
> use the array_index_nospec macro where applicable, or the macro
> block_speculation. Note, the block_speculation macro is used on all
> path in shared_entry_header and nr_grant_entries. This way, after a
> call to such a function, all bound checks that happened before become
> architectural visible, so that no additional protection is required
> for corresponding array accesses. As the way we introduce an lfence
> instruction might allow the compiler to reload certain values from
> memory multiple times, we try to avoid speculatively continuing
> execution with stale register data by moving relevant data into
> function local variables.
> 
> Speculative execution is not blocked in case one of the following
> properties is true:
>   - path cannot be triggered by the guest
>   - path does not return to the guest
>   - path does not result in an out-of-bound access
> 
> Only the combination of the above properties allows to actually leak
> continuous chunks of memory. Therefore, we only add the penalty of
> protective mechanisms in case a potential speculative out-of-bound
> access matches all the above properties.
> 
> This commit addresses only out-of-bound accesses whose index is
> directly controlled by the guest, and the index is checked before.
> Potential out-of-bound accesses that are caused by speculatively
> evaluating the version of the current table are not addressed in this
> commit. Hence, speculative out-of-bound accesses might still be
> possible, for example in gnttab_get_status_frame_mfn, when calling
> gnttab_grow_table, the assertion that the grant table version equals
> two might not hold under speculation.
> 
> This is part of the speculative hardening effort.
> 
> Signed-off-by: Norbert Manthey <nmanthey@xxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> 
> Notes:
>    v3:  Drop condition to not fix defects in commit message.
>         Copy in reviewed-by.

According to this (which aiui means v4) there are no code changes
compared to v3. At the risk of annoying you, this doesn't fit well
with me having said "and then perhaps make changes to a few more
paths" alongside the option of doing this removal in reply to v3.
After all you've now dropped a condition from what is covered by
"Only the combination of ...", and hence there's a wider set of
paths that would need to be fixed. It was for this reason that as
the other alternative I did suggest to simply weaken the wording
of the item you've now dropped. IOW I'm afraid my R-b is not
applicable to v4.

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