Re: [Qemu-ppc] [PATCH 1/2] pseries: Synchronize qemu and KVM state on hypercalls

[David Gibson]

On Thu, Sep 20, 2012 at 02:44:26PM +0200, Alexander Graf wrote:
> 
> On 20.09.2012, at 13:53, David Gibson wrote:
> 
> > On Thu, Sep 20, 2012 at 09:38:58AM +0200, Alexander Graf wrote:
> >> 
> >> On 20.09.2012, at 09:08, David Gibson wrote:
> >> 
> >>> Currently the KVM exit path for PAPR hypercalls does not synchronize the
> >>> qemu cpu state with the KVM state.  Mostly this works, because the actual
> >>> hypercall arguments and return values are explicitly passed through the
> >>> kvm_run structure.  However, the hypercall path includes a privilege 
> >>> check,
> >>> to ensure that only the guest kernel can invoke hypercalls, not the guest
> >>> userspace.  Because of the lack of sync, this privilege check will use an
> >>> out of date copy of the MSR, which could lead either to guest userspace
> >>> being able to invoke hypercalls (a security hole for the guest) or to the
> >>> guest kernel being incorrectly refused privilege leading to various other
> >>> failures.
> >>> 
> >>> This patch fixes the bug by forcing a synchronization on the hypercall 
> >>> exit
> >>> path.  This does mean we have a potentially quite expensive get and set of
> >>> the state, however performance critical hypercalls are generally already
> >>> implemented inside KVM so this probably won't matter.  If it is a
> >>> performance problem we can optimize it later by having the kernel perform
> >>> the privilege check.  That will need a new capability, however, since qemu
> >>> will still need the privilege check for older kernels.
> >>> 
> >>> Signed-off-by: David Gibson <address@hidden>
> >> 
> >> I would actually prefer to see that one fixed in kernel space.
> > 
> > That's a better fix, but we can't fix it purely in the kernel, because
> > there are existing released kernels that don't do the privilege check.
> 
> There are security flaws fixed through -stable updates in the kernel
> any day, why should this one be handled differently?

>From the kernel's point of view, this is not obviously a security bug
- it passes a hypercall it doesn't know how to handle to qemu, qemu
handles it incorrectly.

And in any case, even if you do consider it a kernel security bug,
there's no reason that qemu should just allow that bug to appear when
it's capable of working around buggy kernels in a way that closes the
security hole.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson