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

Andrew Scull ascull at google.com
Mon Mar 28 16:28:26 CEST 2022


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.


More information about the U-Boot mailing list