On 7/30/19 15:38, Jan Beulich wrote: > 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 <nmant...@amazon.de> >> Reviewed-by: Jan Beulich <jbeul...@suse.com> >> --- >> >> 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.
I see, and am sorry for the misunderstanding. I am fine with adding the 4th condition in a weakened form (essentially modifying the commit message to the form you suggested). I wonder whether the summary when to fix a potential speculative out-of-bound access should actually be documented somewhere else than in the commit message of this (more or less random) commit. Best, Norbert > > Jan Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
