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

[Andrew Scull]

On Fri, 25 Mar 2022 at 10:26, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> On Fri, Mar 25, 2022 at 5:18 PM Andrew Scull <ascull at google.com> wrote:
> >
> > 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.
>
> Do you know if the Linux kernel has a plan to apply such a guideline
> to some other drivers than virtio, like usb or nvme? There are devices
> that can do bad things too so we should not trust them?

I don't know about those other subsystems, but I do know that virtio
has made assumptions about the other side being necessarily trusted
and that those assumptions are being broken by new applications, which
is leading to things being reworked.