On Tue, Jan 10, 2017 at 9:34 AM, Razvan Cojocaru <rcojoc...@bitdefender.com> wrote:
> On 01/10/2017 06:29 PM, Tamas K Lengyel wrote: > > > > > > On Tue, Jan 10, 2017 at 8:35 AM, Razvan Cojocaru > > <rcojoc...@bitdefender.com <mailto:rcojoc...@bitdefender.com>> wrote: > > > > >>> What I meant was taking out this call: > > >>> > > >>> /* Remove the ring_pfn from the guest's physmap */ > > >>> rc1 = xc_domain_decrease_reservation_exact(xch, domain_id, > 1, 0, > > >>> &ring_pfn); > > >>> if ( rc1 != 0 ) > > >>> PERROR("Failed to remove ring page from guest physmap"); > > >>> > > >>> To leave the frame in the guest physmap. The issue is > fundamentally > > >>> that after this frame has been taken out, something kicks the VM > to > > >>> realise it has an extra frame of balloonable space, which it > clearly > > >>> compensates for. > > >>> > > >>> You can work around the added attack surface by marking it RO in > > EPT; > > >>> neither Xen's nor dom0's mappings are translated via EPT, so > > they can > > >>> still make updates, but the guest won't be able to write to it. > > >>> > > >>> I should say that this is all a gross hack, and is in desperate > > need of > > >>> a proper API to make rings entirely outside of the gfn space, > > but this > > >>> hack should work for now. > > >> Thanks! So far, it seems to work like a charm like this: > > > > > > Great. > > > > > >> > > >> diff --git a/tools/libxc/xc_vm_event.c b/tools/libxc/xc_vm_event.c > > >> index 2fef96a..5dd00a6 100644 > > >> --- a/tools/libxc/xc_vm_event.c > > >> +++ b/tools/libxc/xc_vm_event.c > > >> @@ -130,9 +130,17 @@ void *xc_vm_event_enable(xc_interface *xch, > > domid_t > > >> domain_id, int param, > > >> } > > >> > > >> /* Remove the ring_pfn from the guest's physmap */ > > >> + /* > > >> rc1 = xc_domain_decrease_reservation_exact(xch, domain_id, > 1, 0, > > >> &ring_pfn); > > >> if ( rc1 != 0 ) > > >> PERROR("Failed to remove ring page from guest physmap"); > > >> + */ > > >> + > > >> + if ( xc_set_mem_access(xch, domain_id, XENMEM_access_r, > > mmap_pfn, 1) ) > > >> + { > > >> + PERROR("Could not set ring page read-only\n"); > > >> + goto out; > > >> + } > > >> > > >> out: > > >> saved_errno = errno; > > >> > > >> Should I send this as a patch for mainline as well? > > > > > > Probably a good idea, although I would include a code comment > > explaining > > > what is going on, because this is subtle if you don't know the > > context. > > > > Will do, I'll send a patch out as soon as we've done a few more > rounds > > of testing. > > > > > > (replying to all): I'm not in favor of this patch mainly because it is > > not stealthy. A malicious kernel could easily track what events are > > being sent on the ring. With DRAKVUF I could work around this using > > altp2m pfn-remapping, but for other tools this is can be a serious > > information leak. > > I understand your point, however the alternative is potential lack of > availability to monitor which is arguably a more severe problem. _Any_ > guest could choose to do what this Ubuntu 16.04 guest does, and then > connecting to the guest via vm_event can only be done once. > IMHO in that case you should implement your own internal version of this function to fall back to instead of forcing all tools to go down this path. The requirements to do that in your own tool are accessible so there is no need to push that into libxc. Tamas
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
