Re: [Xen-devel] [PATCH] lock down hypercall continuation encoding masks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.12.14 at 15:36, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 05/12/14 11:31, Jan Beulich wrote:
>> Andrew validly points out that even if these masks aren't a formal part
>> of the hypercall interface, we aren't free to change them: A guest
>> suspended for migration in the middle of a continuation would fail to
>> work if resumed on a hypervisor using a different value. Hence add
>> respective comments to their definitions.
>>
>> Additionally, to help future extensibility as well as in the spirit of
>> reducing undefined behavior as much as possible, refuse hypercalls made
>> with the respective bits non-zero when the respective sub-ops don't
>> make use of those bits.
>>
>> Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> General principle looks good.  A couple of issues.
> 
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -4661,9 +4661,8 @@ int xenmem_add_to_physmap_one(
>>  long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>>  {
>>      int rc;
>> -    int op = cmd & MEMOP_CMD_MASK;
> 
> This needs a blanket start_iter check, as do_memory_op() has not done so.

Not sure what you're asking for - why is removing the masking not
sufficient?

> The ARM code also needs one, as the caller has applied partial checks.

The ARM code never applied a mask.

>> --- a/xen/common/memory.c
>> +++ b/xen/common/memory.c
>> @@ -977,6 +992,9 @@ long do_memory_op(unsigned long cmd, XEN
>>          unsigned int dom_vnodes, dom_vranges, dom_vcpus;
>>          struct vnuma_info tmp;
>>  
>> +        if ( unlikely(start_extent) )
>> +            return -ENOSYS;
>> +
>>          /*
>>           * Guest passes nr_vnodes, number of regions and nr_vcpus thus
>>           * we know how much memory guest has allocated.
> 
> XENMEM_get_vnumainfo needs a guard.

Again - I don't understand what you're asking for: The hunk above
is modifying the XENMEM_get_vnumainfo case.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel