[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 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. >>> The ARM code also needs one, as the caller has applied partial checks. >> The ARM code never applied a mask. > > But the common code does, so the ARM code must follow suit for consistency. > > Otherwise, we end up with ARM non-preemptible memory subops not failing > with -ENOSYS where primary memory ops would. Again, the default case results in -ENOSYS for any with the high bits set. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |