Re: Device model operation hypercall (DMOP, re qemu depriv)

From: David Vrabel <david.vrabel@citrix.com>
To: Jan Beulich <JBeulich@suse.com>, Ian Jackson <ian.jackson@eu.citrix.com>
Cc: StefanoStabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Tim Deegan <tim@xen.org>, GeorgeDunlap <george.dunlap@citrix.com>,
	Anthony Perard <anthony.perard@citrix.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	dgdegra@tycho.nsa.gov
Subject: Re: Device model operation hypercall (DMOP, re qemu depriv)
Date: Mon, 15 Aug 2016 15:50:30 +0100	[thread overview]
Message-ID: <57B1D6B6.5050804@citrix.com> (raw)
In-Reply-To: <57A9CCA202000078001042A0@prv-mh.provo.novell.com>

On 09/08/16 11:29, Jan Beulich wrote:
>>>> On 08.08.16 at 15:46, <ian.jackson@eu.citrix.com> wrote:
>> Jan Beulich writes ("Re: Device model operation hypercall (DMOP, re qemu 
>> depriv)"):
>>> On 05.08.16 at 18:28, <ian.jackson@eu.citrix.com> wrote:
>>>> That is, a bug of class 2 would allow the unprivileged qemu process in
>>>> dom0 to cause damage to other parts of dom0.
>> ...
>>> Ah, okay, I think I finally understand. [...]
>>>
>>> I'm having, however, a hard time imagining a class 2 bug for any
>>> of the hvmop-s that are being converted by the hvmctl series:
>>> These act on the target domain, so would not touch the calling
>>> ones state other than for copying argument structures to/from
>>> guest memory (and I don't view mixing up domain pointers as
>>> a likely source of problems).
>>
>> Right.  AIUI all the hypercall arguments are passed using
>> calling-guest-virtual addresses, which the dom0 privcmd arrangements
>> are capable of ensuring are sane.
> 
> Actually, having thought about this some more, and taking this
> together with the expectations to the privcmd driver previously
> outlined, I think this part is problematic: If all the driver is to know
> is the position (within the interface structure) of the target domain
> ID, then any guest handles embedded in the interface structure
> (XEN_HVMCTL_track_dirty_vram only for now) couldn't get
> validated, and hence user mode code would have a way to access
> or modify kernel memory.

It seems simpler to me to have in the privcmd driver:

    if (op == HVMCTL_track_dirty_vram)
        ret = access_ok(...);

It looks simpler to me to fix the ABI and add the checking in the
privcmd driver than add one of the proposed mechanisms to allow the
hypervisor to do the checking.

To avoid the issues with having to update the kernel in lock-step with
the hypervisor (if new checks are needed in privcmd), we can (in the
common case) do version the checking in the driver.

i.e., if the privcmd drivers knows about hypervisor ABI V but qemu needs
V+1 then it can choose to disable the depriv and thus continue to work
(with reduced defense in depth).

David

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel