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

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

On Tue, Nov 02, 2021 at 09:07:56AM +0000, Oleksandr Andrushchenko wrote:
> 
> 
> On 02.11.21 10:48, Roger Pau Monné wrote:
> > 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.
> I do agree with this. But at the moment we only emulate some of them,
> so in the future we will need revisiting the emulation and put many
> more registers under Xen's control

Indeed. That's my main point - there's still a lot of work to do
internally in vPCI in order to be safe for unprivileged guests to
use.

Thanks, Roger.