[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support
From: |
Peter Maydell |
Subject: |
Re: [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support |
Date: |
Thu, 16 Mar 2023 13:41:56 +0000 |
On Fri, 13 Mar 2020 at 01:45, Guenter Roeck <linux@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.maydell@linaro.org>
> Signed-off-by: Guenter Roeck <linux@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
- Re: [PATCH v3 1/5] hw/usb: Add basic i.MX USB Phy support,
Peter Maydell <=