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

[Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>]

On 07.01.2020 15:55, Jan Beulich wrote:
> 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.
> 

So the way to go is to have:

altp2m_idx = array_index_nospec(altp2m_idx, MAX_ALTP2M);
ap2m = d->arch.altp2m_p2m[altp2m_idx];


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