Re: [Xen-devel] [v2][PATCH] xen/vtd/iommu: permit group devices to passthrough in relaxed mode

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Chen, Tiejun
> Sent: Friday, September 11, 2015 10:21 AM
> 
> >> >> > > However I don't think this patch is a right fix. So far 
> >> >> > > relax/strict policy
> >> >> > > is per-domain. what about one VM specifies relax while another VM
> >> >> > > specifies strict when each is assigned with a device sharing rmrr
> >> >> > > with the other? In that case it becomes a system-wide security hole.
> >> >> >
> >> >> > The one specifying "strict" won't gets its device assigned (due to
> >> >> > the code above, taking the path that was there already without
> >> >> > the patch), so I don't see the security issue.
> >> >> >
> >> >>
> >> >> Agreed. A VM can't get such device assigned in the first place, so the
> >> >> hypothetical scenario doesn't exist.
> >> >>
> >> >
> >> > Sorry it's a bad example. My actual concern is that we can't count
> >> > on this per-VM relax/strict policy to prevent group devices assigned
> >> > to different VM. In that case it's definitely a security hole since
> >> > one VM may clobber shared RMRR to impact another VM. So right
> >> > example for that scenario is both VMs specified with 'relax'.
> >>
> >> What if one of group devices is still owned by Dom0?
> >>
> >
> > It's also risky since other VM may attack Dom0 in such scenario.
> >
> 
> In my opinion, Dom0 should have a big impact...
> 
> Anyway, this always means we have to start refactoring some codes. For
> example, we are probably going to introduce some new fields in struct
> acpi_rmrr_unit, just like,
> 
> int domain_id -> Distinguish which domain owns this unit
> unsigned int flag -> Record state: XEN_DOMCTL_DEV_RDM_RELAXE or
> !XEN_DOMCTL_DEV_RDM_RELAXE
> 
> This should involve several sections, such as parsing rmrr, setup
> hwdomain and assign/remove device.
> 
> But I'm not sure if this is good to handle current problem. Actually I
> prefer to work on current patch just now, and then we can start
> discussing our final solution :)
> 

I think we are synced on final solution, that we must ensure devices
sharing RMRR are assigned together. Before the final solution is ready,
our previous agreement is to disallow such assignment to avoid 
security risk, which however caused one regression as reported here.
Now the question is whether we want to remove the security check
(as your patch) to solve the regression, or keep the security check 
which fix a security hole but causing some regression.

Also we should note that reusing same per-domain relax/strict flag
for such purpose is also risky. What about an user unaware of
the fact of devices sharing RMRR, and then he assigns devices to
different VM both w/ relax policy specified? It may expose attack
chance inadvertently. So if we really want to have a temporary
relax mode for group devices, I prefer to have a new parameter
so the user can have full awareness and control on it.

Thanks
Kevin

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