On Wed, Dec 26, 2018 at 02:37:02AM +0000, Taylor R Campbell wrote:
> > +static int
> > +vioif_alloc_queues(struct vioif_softc *sc)
> > +{
> > + int nvq_pairs = sc->sc_max_nvq_pairs;
> > + int nvqs = nvq_pairs * 2;
> > + int i;
> > +
> > + sc->sc_rxq = kmem_zalloc(sizeof(sc->sc_rxq[0]) * nvq_pairs,
> > + KM_NOSLEEP);
> > + if (sc->sc_rxq == NULL)
> > + return -1;
> > +
> > + sc->sc_txq = kmem_zalloc(sizeof(sc->sc_txq[0]) * nvq_pairs,
> > + KM_NOSLEEP);
> > + if (sc->sc_txq == NULL)
> > + return -1;
>
> Check to avoid arithmetic overflow here:
>
> if (nvq_pairs > INT_MAX/2 - 1 ||
> nvq_pairs > SIZE_MAX/sizeof(sc->sc_rxq[0]))
> return -1;
> nvqs = nvq_pairs * 2;
> if (...) nvqs++;
> sc->sc_rxq = kmem_zalloc(sizeof(sc->sc_rxq[0]) * nvq_pairs, ...);
>
> Same in all the other allocations like this. (We should have a
> kmem_allocarray -- I have a draft somewhere.)
The limit should just be sanely enforced much, much earlier. The code
in ioif_attach already does that, so why bother.
> > diff --git a/sys/dev/pci/virtio_pci.c b/sys/dev/pci/virtio_pci.c
> > index 65c5222b774..bb972997be2 100644
> > --- a/sys/dev/pci/virtio_pci.c
> > +++ b/sys/dev/pci/virtio_pci.c
> > @@ -604,8 +677,14 @@ virtio_pci_setup_interrupts(struct virtio_softc *sc)
> > [...]
> > if (pci_intr_type(pc, psc->sc_ihp[0]) == PCI_INTR_TYPE_MSIX) {
> > - psc->sc_ihs = kmem_alloc(sizeof(*psc->sc_ihs) * 2,
> > + psc->sc_ihs = kmem_alloc(sizeof(*psc->sc_ihs) * nmsix,
> > KM_SLEEP);
>
> Check for arithmetic overflow here.
No point here either.
Joerg
