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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Mon, May 25, 2020 at 8:14 AM Tamas K Lengyel <tamas@xxxxxxxxxxxxx> wrote:
>
> On Mon, May 25, 2020 at 8:06 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >
> > On 25.05.2020 15:46, Tamas K Lengyel wrote:
> > > On Mon, May 25, 2020 at 7:06 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> > >>
> > >> On 25.05.2020 14:18, Tamas K Lengyel wrote:
> > >>> 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.
> > >>
> > >> p2m-pt.c:p2m_type_to_flags() has a similar case label.
> > >
> > > It doesn't do anything though, does it? For mem_sharing to work you
> > > actively have to restrict the memory permissions on the shared entries
> > > to be read/execute only. That's only done for EPT.
> >
> > Does it not? I seems to me that it does, seeing the case sits
> > together with the p2m_ram_ro and p2m_ram_logdirty ones:
> >
> >     case p2m_ram_ro:
> >     case p2m_ram_logdirty:
> >     case p2m_ram_shared:
> >         return flags | P2M_BASE_FLAGS;
> >
> > >> And I can't
> > >> spot a respective restriction in mem_sharing_memop(), i.e. it looks
> > >> to me as if enabling mem-sharing on NPT (to satisfy hap_enabled()
> > >> in mem_sharing_control()) would be possible.
> > >
> > > If you are looking for an explicit gate like that, then you are right,
> > > there isn't one. You can ask the original authors of this subsystem
> > > why that is. If you feel like adding an extra gate, I wouldn't object.
> >
> > Well, the question here isn't about gating - that's an independent
> > bug if it's indeed missing. The question is whether SVM code also
> > needs touching, as was previously requested. You tried to address
> > this by stating an Intel-only limitation, which I couldn't find
> > proof for (so far).
>
> Well, as far as I'm concerned VM forking is for Intel hardware only.
> If mem_sharing seems to work for non-Intel hw - I was unaware of that
> - than I'll just add an extra check for the VM fork hypercall that
> gates it. It may be possible for technically be made available for
> other hw as well, but at this time that's completely out-of-scope.

Actually, I'm going to just add that gate completely for mem_sharing.
Even if it at some time worked on other architectures (doubtful) at
this time its a usecase that's completely abandoned and forgotten and
as far as I'm concerned unmaintained with no plans from my side to
ever maintain it.

Tamas