Re: granting access to MSI-X table and pending bit array

["Jan Beulich" <JBeulich@novell.com>]

>>> On 07.07.10 at 18:49, Stefano Stabellini <stefano.stabellini@eu.citrix.com>
wrote:
> On Wed, 7 Jul 2010, Jan Beulich wrote:
>> >>> On 07.07.10 at 16:14, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>> > On Wed, Jul 07, 2010 at 11:14:04AM +0100, Jan Beulich wrote:
>> >> The original implementation (c/s 17536) disallowed access to these
>> >> after granting access to all BAR specified resources (i.e. this was
>> >> almost correct, except for a small time window during which the
>> >> memory was accessible to the guest and except for hiding the
>> >> pending bit array from the guest), but this got reverted with c/s
>> >> 20171.
>> >> 
>> >> Afaics this is a security problem, as CPU accesses to the granted
>> >> memory don't go through any IOMMU and hence there's no place
>> >> these could be filtered out even in a supposedly secure environment
>> >> (not that I think devices accesses would be filtered at present, but
>> >> for those this would at least be possible ), and such accesses could
>> >> inadvertently or maliciously unmask masked vectors or modify the
>> >> message address/data fields.
>> >> 
>> >> Imo the pending bit array must be granted read-only access to the
>> >> guest (instead of either granting full access or no access at all),
>> >> with the potential side effect of also granting read-only access to
>> >> the table. And I would even think that this shouldn't be done in the
>> >> tools, but rather in Xen itself (since it knows of all the PCI devices
>> >> and their respective eventual MSI-X address ranges), thus at once
>> >> eliminating any timing windows.
>> > 
>> > That sounds sensible. You got a patch ready?
>> 
>> Would be nice, but that's not that simple as currently there's no way
>> to distinguish r/w mmio and r/o mmio.
>> 
>> Additionally we may need to override the guest's page table
>> attributes, as generally at least Linux maps I/O memory writeable
>> no matter whether any writes will actually happen - which to me
>> seems to be an ugly hack (but I can't think of better alternatives).
>> 
>> Furthermore I'm not really clear how a HVM guest's writes to the
>> MSI-X table get processed - clearly they can't be let through to
>> the actual MMIO region. I'd guess they somehow get redirected
>> to qemu (since pciback doesn't seem to be handling this), but I
>> don't know this process very well...
>> 
> 
> the guest gets a fake MSI-X table, emulated by qemu, have a look at:
> 
> hw/pt-msi.c:pt_msi_setup
> hw/pt-msi.c:pt_msi_update
> hw/pass-through.c:pt_msgctrl_reg_write

Hmm, okay, this means that the HVM case looks safe but really isn't:
Generally, access to the underlying I/O memory range is disallowed,
but there is a (small) time window where the guest can directly see
that range. What's worse is that qemu writes the mask flag (as
coming from the guest) directly to hardware, this overriding
whatever Xen may want there.

With the revert mentioned above being about PV guests, I wonder
what problems PV guests had with not being able to access the
table and PBA ranges. Linux doesn't (so far) use the PBA at all,
and according to my analysis of drivers/pci/msi-xen.c also doesn't
access the table in any way even when being run privileged. Keir,
with the description of c/s 20171 being rather brief - what was
the problem PV guests were having? Or was that more specifically
with stubdom wanting to do above mentioned writes on behalf of
the serviced guest?

Finally, removing the rounded up to page boundaries range
completely for HVM guests will break any guest OSes that
want to make use of the PBA if, for a particular device, this
and the table (partially) share a page (which the spec explicitly
permits).

Jan