Re: [PATCH] xen/arm: fix SBDF calculation for vPCI MMIO handlers

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Nov 01, 2021 at 06:14:40AM +0000, Oleksandr Andrushchenko wrote:
> 
> 
> On 29.10.21 10:33, Roger Pau Monné wrote:
> > On Thu, Oct 28, 2021 at 05:55:25PM +0000, Oleksandr Andrushchenko wrote:
> >>
> >> On 28.10.21 19:03, Roger Pau Monné wrote:
> >>> On Thu, Oct 28, 2021 at 02:23:34PM +0000, Oleksandr Andrushchenko wrote:
> >>>> On 28.10.21 16:36, Roger Pau Monné wrote:
> >>>>> And for domUs you really need to fix vpci_{read,write} to not
> >>>>> passthrough accesses not explicitly handled.
> >>>> Do you mean that we need to validate SBDFs there?
> >>>> This can be tricky if we have a use-case when a PCI device being
> >>>> passed through if not put at 0000:00:0.0, but requested to be, for
> >>>> example, 0000:0d:0.0. So, we need to go over the list of virtual
> >>>> devices and see if SBDF the guest is trying to access is a valid SBDF.
> >>>> Is this what you mean?
> >>> No, you need to prevent accesses to registers not explicitly handled
> >>> by vpci. Ie: do not forward unhandled accesses to
> >>> vpci_{read,wrie}_hw).
> >> I see, so those which have no handlers are not passed to the hardware.
> >> I need to see how to do that
> > Indeed. Without fixing that passthrough to domUs is completely unsafe,
> > as you allow domUs full access to registers not explicitly handled by
> > current vPCI code.
> Well, my understanding is: we can let the guest access whatever
> registers it wants with the following exceptions:
> - "special" registers we already trap in vPCI, e.g. command, BARs
> - we must not let the guest go out of the configuration space of a
> specific PCI device, e.g. prevent it from accessing configuration
> spaces of other devices.
> The rest accesses seem to be ok to me as we do not really want:
> - have handlers and emulate all possible registers
> - we do not want the guest to fail if it accesses a valid register which
> we do not emulate.

IMO that's not good from a security PoV. Xen needs to be sure that
every registers a guest accesses is not going to cause the system to
malfunction, so Xen needs to keep a list of the registers it's safe
for a guest to access.

For example we should only expose the PCI capabilities that we know
are safe for a guest to use, ie: MSI and MSI-X initially. The rest of
the capabilities should be blocked from guest access, unless we audit
them and declare safe for a guest to access.

As a reference you might want to look at the approach currently used
by QEMU in order to do PCI passthrough. A very limited set of PCI
capabilities known to be safe for untrusted access are exposed to the
guest, and registers need to be explicitly handled or else access is
rejected. We need a fairly similar model in vPCI or else none of this
will be safe for unprivileged access.

Regards, Roger.