Re: [PATCH HVM v2 1/1] hvm: refactor set param

[Norbert Manthey <nmanthey@amazon.de>]

On 2/12/21 11:04 AM, Jan Beulich wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> On 11.02.2021 21:46, Norbert Manthey wrote:
>> I agree with the symmetry for get and set. This is what I'd aim for:
>>
>>  1. hvmop_set_param and hvmop_get_param (static) both check for the
>> index, and afterwards use the is_hvm_domain(d) function with its barrier
>>  2. hvm_set_param (static) and hvm_get_param both call their allow
>> helper function, evaluate the return code, and afterwards block speculation.
>>  2.1. hvm_get_param is declared in a public header, and cannot be turned
>> into a static function, hence needs the index check
> But both further call sites are in bounded loops, with the bounds not
> guest controlled. It can rely on the callers just as much as ...
Okay, so I will not add the check there either. I thought about future
modifications that allow to call that function from other places, or
modified call environments with eventually guest control - but I am fine
to not consider these.
>
>>  2.2. hvm_set_param is only called from hvmop_set_param, and index is
>> already checked there, hence, do not add check
> ... this.
>
>>  3. hvm_allow_set_param (static) and hvm_allow_get_param (static) do not
>> validate the index parameter
>>  3.1. hvm_allow_set_param blocks speculative execution with a barrier
>> after domain permissions have been evaluated, before accessing the
>> parameters of the domain. hvm_allow_get_param does not access the params
>> member of the domain, and hence does not require additional protection.
>>
>> To simplify the code, I propose to furthermore make the hvmop_set_param
>> function static as well.
> Yes - this not being so already is likely simply an oversight,
> supported by the fact that there's no declaration in any header.

Okay.

Best,
Norbert




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879