[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] lock down hypercall continuation encoding masks
On 05/12/14 15:18, Jan Beulich wrote: >>>> On 05.12.14 at 16:01, <andrew.cooper3@xxxxxxxxxx> wrote: >> On 05/12/14 14:47, Jan Beulich wrote: >>>>>> 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? >> There is no check to ensure that a non-preemptible arch_memoy_op is not >> called with a non-zero start_iter. >> >> This location needs something like >> >> if ( cmd & ~MEMOP_CMD_MASK ) >> return -ENOSYS; > I'm sorry - the default case of sub_arch_memory_op() will ensure > this. Ah - I see now. That is subtle. Better remember to double check the first patch which needs to add a preemptible subop. Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |