[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 for-4.14 1/2] x86/mem_sharing: block interrupt injection for forks



On Mon, May 25, 2020 at 12:06 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 22.05.2020 18:33, Tamas K Lengyel wrote:
> > When running shallow forks without device models it may be undesirable for 
> > Xen
> > to inject interrupts. With Windows forks we have observed the kernel going 
> > into
> > infinite loops when trying to process such interrupts, likely because it 
> > attempts
> > to interact with devices that are not responding without QEMU running. By
> > disabling interrupt injection the fuzzer can exercise the target code 
> > without
> > interference.
> >
> > Forks & memory sharing are only available on Intel CPUs so this only applies
> > to vmx.
>
> Looking at e.g. mem_sharing_control() I can't seem to be able to confirm
> this. Would you mind pointing me at where this restriction is coming from?

Both mem_access and mem_sharing are only implemented for EPT:
http://xenbits.xen.org/hg/xen-unstable.hg/file/5eadf9363c25/xen/arch/x86/mm/p2m-ept.c#l126.

>
> > --- a/xen/arch/x86/hvm/vmx/intr.c
> > +++ b/xen/arch/x86/hvm/vmx/intr.c
> > @@ -256,6 +256,12 @@ void vmx_intr_assist(void)
> >      if ( unlikely(v->arch.vm_event) && v->arch.vm_event->sync_event )
> >          return;
> >
> > +#ifdef CONFIG_MEM_SHARING
> > +    /* Block event injection for VM fork if requested */
> > +    if ( unlikely(v->domain->arch.hvm.mem_sharing.block_interrupts) )
> > +        return;
> > +#endif
>
> The two earlier returns are temporary as far as the guest is concerned,
> i.e. eventually the interrupt(s) will get delivered. The one you add
> looks as if it is a permanent thing, i.e. interrupt requests will pile
> up and potentially confuse a guest down the road. This _may_ be okay
> for your short-lived-shallow-fork scenario, but then wants at least
> calling out in the public header by a comment (and I think the same
> goes for XENMEM_FORK_WITH_IOMMU_ALLOWED that's already there).

This is indeed only for the short-lived forks, that's why this is an
optional flag that can be enabled when creating forks and it's not on
by default. In that use-case the VM executes for fractions of a second
and we want to only executes very specific code-segments with
absolutely no interference. Interrupts in that case are just a
nuisance that in the best case slow the fuzzing process down but as we
observed in the worst case complete stall it.

>
> > --- a/xen/include/asm-x86/hvm/domain.h
> > +++ b/xen/include/asm-x86/hvm/domain.h
> > @@ -74,6 +74,8 @@ struct mem_sharing_domain
> >       * to resume the search.
> >       */
> >      unsigned long next_shared_gfn_to_relinquish;
> > +
> > +    bool block_interrupts;
> >  };
>
> Please can you avoid unnecessary growth of the structure by inserting
> next to the pre-existing bool rather than at the end?

Sure. Do you want me to resend the patch for that?

Tamas



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.