On Fri, 13 Mar 2020 at 01:45, Guenter Roeck <li...@roeck-us.net> wrote: > > Add basic USB PHY support as implemented in i.MX23, i.MX28, i.MX6, > and i.MX7 SoCs. > > The only support really needed - at least to boot Linux - is support > for soft reset, which needs to reset various registers to their initial > value. Otherwise, just record register values. > > Reviewed-by: Peter Maydell <peter.mayd...@linaro.org> > Signed-off-by: Guenter Roeck <li...@roeck-us.net>
Hi Guenter; we've had a fuzzer report that this device model accesses off the end of the usbphy[] array: https://gitlab.com/qemu-project/qemu/-/issues/1408 > +static uint64_t imx_usbphy_read(void *opaque, hwaddr offset, unsigned size) > +{ > + IMXUSBPHYState *s = (IMXUSBPHYState *)opaque; > + uint32_t index = offset >> 2; > + uint32_t value; > + default: > + value = s->usbphy[index]; No bounds check in the default case (or ditto in the write function)... > + break; > + } > + return (uint64_t)value; > +} > +static void imx_usbphy_realize(DeviceState *dev, Error **errp) > +{ > + IMXUSBPHYState *s = IMX_USBPHY(dev); > + > + memory_region_init_io(&s->iomem, OBJECT(s), &imx_usbphy_ops, s, > + "imx-usbphy", 0x1000); ...and the memory region is created at size 0x1000 so the read/write fns can be called with offsets up to that size... > + sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->iomem); > +} > +enum IMXUsbPhyRegisters { > + USBPHY_PWD, > + USBPHY_PWD_SET, > + USBPHY_PWD_CLR, > + USBPHY_PWD_TOG, > + USBPHY_TX, > + USBPHY_TX_SET, > + USBPHY_TX_CLR, > + USBPHY_TX_TOG, > + USBPHY_RX, > + USBPHY_RX_SET, > + USBPHY_RX_CLR, > + USBPHY_RX_TOG, > + USBPHY_CTRL, > + USBPHY_CTRL_SET, > + USBPHY_CTRL_CLR, > + USBPHY_CTRL_TOG, > + USBPHY_STATUS, > + USBPHY_DEBUG = 0x14, > + USBPHY_DEBUG_SET, > + USBPHY_DEBUG_CLR, > + USBPHY_DEBUG_TOG, > + USBPHY_DEBUG0_STATUS, > + USBPHY_DEBUG1 = 0x1c, > + USBPHY_DEBUG1_SET, > + USBPHY_DEBUG1_CLR, > + USBPHY_DEBUG1_TOG, > + USBPHY_VERSION, > + USBPHY_MAX > +}; > + > +#define USBPHY_CTRL_SFTRST BIT(31) > + > +#define TYPE_IMX_USBPHY "imx.usbphy" > +#define IMX_USBPHY(obj) OBJECT_CHECK(IMXUSBPHYState, (obj), TYPE_IMX_USBPHY) > + > +typedef struct IMXUSBPHYState { > + /* <private> */ > + SysBusDevice parent_obj; > + > + /* <public> */ > + MemoryRegion iomem; > + > + uint32_t usbphy[USBPHY_MAX]; ...but the array is only created with USBPHY_MAX entries. Do you know what the device is supposed to do with these off-the-end acceses? We could either reduce the memory region size or bounds check and RAZ/WI the out-of-range accesses. thanks -- PMM
