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

Bin Meng
2022-03-25T07:51:23Z

On Fri, Mar 25, 2022 at 3:03 PM Andrew Scull <ascull@google.com> wrote:
>
> On Fri, 25 Mar 2022 at 04:31, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Sun, Mar 20, 2022 at 7:42 PM Andrew Scull <ascull@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?

Regards,
Bin