Re: [Xen-devel] [PATCH 1/5] x86/hvm: Switch hvm_allow_get_param() to use a whitelist

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.09.18 at 20:18, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 07/09/18 09:55, Jan Beulich wrote:
>>>>> On 06.09.18 at 17:21, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 06/09/18 09:56, Paul Durrant wrote:
>>>>> @@ -4390,9 +4411,6 @@ static int hvmop_get_param(
>>>>>      if ( copy_from_guest(&a, arg, 1) )
>>>>>          return -EFAULT;
>>>>>
>>>>> -    if ( a.index >= HVM_NR_PARAMS )
>>>>> -        return -EINVAL;
>>>>> -
>>>> ASSERT, just in case someone screws up the allow function in future?
>>> That's not going to help in any practical way.  This check does really
>>> exist, and is part of the switch statement.
>> Which switch() statement? The one in the allow function includes this,
>> but the one here simply has
>>
>>     default:
>>         a.value = d->arch.hvm.params[a.index];
>>         break;
> 
> A boundary check on a.index logically falls within the remit of
> hvm_allow_get_param()

Correct. Hence - due to the split between the two functions - the
desire to have a validating ASSERT() in the other function.

Jan



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