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

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

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
>
> Regards, Roger.