On 2/15/19 09:55, Jan Beulich wrote: >>>> On 15.02.19 at 09:05, <nmant...@amazon.de> wrote: >> On 2/12/19 15:14, Jan Beulich wrote: >>>>>> On 12.02.19 at 15:05, <nmant...@amazon.de> wrote: >>>> On 2/12/19 14:25, Jan Beulich wrote: >>>>>>>> On 08.02.19 at 14:44, <nmant...@amazon.de> wrote: >>>>>> @@ -4104,6 +4108,12 @@ static int hvmop_set_param( >>>>>> if ( a.index >= HVM_NR_PARAMS ) >>>>>> return -EINVAL; >>>>>> >>>>>> + /* >>>>>> + * Make sure the guest controlled value a.index is bounded even >>>>>> during >>>>>> + * speculative execution. >>>>>> + */ >>>>>> + a.index = array_index_nospec(a.index, HVM_NR_PARAMS); >>>>>> + >>>>>> d = rcu_lock_domain_by_any_id(a.domid); >>>>>> if ( d == NULL ) >>>>>> return -ESRCH; >>>>>> @@ -4370,6 +4380,12 @@ static int hvmop_get_param( >>>>>> if ( a.index >= HVM_NR_PARAMS ) >>>>>> return -EINVAL; >>>>>> >>>>>> + /* >>>>>> + * Make sure the guest controlled value a.index is bounded even >>>>>> during >>>>>> + * speculative execution. >>>>>> + */ >>>>>> + a.index = array_index_nospec(a.index, HVM_NR_PARAMS); >>>>> ... the usefulness of these two. To make forward progress it may >>>>> be worthwhile to split off these two changes into a separate patch. >>>>> If you're fine with this, I could strip these two before committing, >>>>> in which case the remaining change is >>>>> Reviewed-by: Jan Beulich <jbeul...@suse.com> >>>> Taking apart the commit is fine with me. I will submit a follow up >>>> change that does not update the values but fixes the reads. >>> As pointed out during the v5 discussion, I'm unconvinced that if >>> you do so the compiler can't re-introduce the issue via CSE. I'd >>> really like a reliable solution to be determined first. >> I cannot give a guarantee what future compilers might do. Furthermore, I >> do not want to wait until all/most compilers ship with such a >> controllable guarantee. > Guarantee? Future compilers are (hopefully) going to get better at > optimizing, and hence are (again hopefully) going to find more > opportunities for CSE. So the problem is going to get worse rather > than better, and the changes you're proposing to re-instate are > therefore more like false promises.
I do not want to dive into compilers future here. I would like to fix the issue for todays compilers now and not wait until compilers evolved one way or another. For this patch, the relevant information is whether it should go in like this, or whether you want me to protect all the reads instead. Is there more data I shall provide to help make this decision? Best, Norbert > > Jan > > Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich Ust-ID: DE 289 237 879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
