Re: [Xen-devel] [PATCH V6 1/4] x86/mm: Add array_index_nospec to guest provided index values

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.01.2020 14:25, Alexandru Stefan ISAILA wrote:
> On 27.12.2019 10:01, Jan Beulich wrote:
>> On 23.12.2019 15:04, Alexandru Stefan ISAILA wrote:
>>> --- a/xen/arch/x86/mm/mem_access.c
>>> +++ b/xen/arch/x86/mm/mem_access.c
>>> @@ -366,11 +366,12 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, 
>>> uint32_t nr,
>>>   #ifdef CONFIG_HVM
>>>       if ( altp2m_idx )
>>>       {
>>> -        if ( altp2m_idx >= MAX_ALTP2M ||
>>> -             d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
>>> +        if ( altp2m_idx >=  min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) 
>>> ||
>>
>> Stray blank after >= .
>>
>>> +             d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] 
>>> ==
>>
>> I accept you can't (currently) use array_access_nospec() here,
>> but ...
>>
>>> +             mfn_x(INVALID_MFN) )
>>>               return -EINVAL;
>>>   
>>> -        ap2m = d->arch.altp2m_p2m[altp2m_idx];
>>> +        ap2m = d->arch.altp2m_p2m[array_index_nospec(altp2m_idx, 
>>> MAX_ALTP2M)];
>>
>> ... I don't see why you still effectively open-code it here.
> 
> I am not sure I follow you here, that is what we agreed in v5 
> (https://lists.xenproject.org/archives/html/xen-devel/2019-12/msg01704.html). 
> Did I miss something?

In context there (from an earlier reply of mine) you will find me
having mentioned array_access_nospec(). This wasn't invalidated or
overridden by my "Yes, that's how I think it ought to be." I didn't
say so explicitly (again) because to me it goes without saying that
open-coding _anything_ is, in the common case, bad practice.

Jan

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