[PATCH 06/11] virtio: pci: Read entire capability into memory

[Andrew Scull]

On Fri, 25 Mar 2022 at 07:51, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> On Fri, Mar 25, 2022 at 3:03 PM Andrew Scull <ascull at google.com> wrote:
> >
> > On Fri, 25 Mar 2022 at 04:31, Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > On Sun, Mar 20, 2022 at 7:42 PM Andrew Scull <ascull at google.com> wrote:
> > > >
> > > > Read the virtio PCI capability out of the device configuration space to
> > > > a struct rather than accessing fields directly from the configuration
> > > > space as they are needed. This both makes access to the fields easier
> > > > and avoids re-reading fields.
> > > >
> > > > Re-reading fields could result in time-of-check to time-of-use problems,
> > > > should the value in the configuration space change. The range check of
> > > > the `bar` field and the later call to `dm_pci_read_bar32()` is an
> > > > example of where this could happen.
> > >
> > > I don't see the need to avoid the time-of-check to time-of-use
> > > problems, as it can only happen with the PCI configuration access
> > > capability, which U-Boot driver does not touch.
> > >
> > > Am I missing something?
> >
> > U-Boot doesn't touch the configuration space but the device could
> > have, whether that be accidently or maliciously. Linux has taken
> > similar precautions [1] to add more safety checks and I'll be looking
> > to do the same in other parts of the u-boot virtio drivers.
> >
> > [1] -- https://lwn.net/Articles/865216
> >
>
> Got it. So basically the problem is that we don't trust the host that
> implements the virtio device :)
>
> I am curious that under such a guideline, probably lots of device
> drivers need to be enhanced to do the sanity check, no?

Absolutely, they do! My focus is going to be primarily on the modern
PCI driver, vring and block driver. This will lay a foundation for the
others but they will also need to be checked over carefully before
being relied on.