Re: [PATCH v2 3/4] usb: chipidea: tegra: Avoid controller/PHY init if bus is externally controlled

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds I/O memory access in `tegra_ehci_hub_control()` due to unvalidated `wIndex`.
--

commit 60717c8d1b44f3d9bbb2d143c18add1a1d62f035
Author: Svyatoslav Ryhel <clamor95@gmail.com>

usb: chipidea: tegra: Avoid controller/PHY init if bus is externally controlled

This commit skips the registration of the USB controller and PHY
initialization if they are externally controlled. Since the USB PHY
in Tegra controls clock gates required by the controller itself, it
defers this configuration to the device that controls the bus.

> diff --git a/drivers/usb/chipidea/ci_hdrc_tegra.c b/drivers/usb/chipidea/ci_hdrc_tegra.c
> index 372788f0f9701..a19cb3b33bf37 100644
> --- a/drivers/usb/chipidea/ci_hdrc_tegra.c
> +++ b/drivers/usb/chipidea/ci_hdrc_tegra.c
[ ... ]
> @@ -342,12 +348,14 @@ static int tegra_usb_probe(struct platform_device *pdev)
>  	if (of_usb_get_phy_mode(pdev->dev.of_node) == USBPHY_INTERFACE_MODE_ULPI)
>  		usb->data.flags &= ~CI_HDRC_SUPPORTS_RUNTIME_PM;
>  
> -	usb->dev = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> -				      pdev->num_resources, &usb->data);

[Severity: High]
This is a pre-existing issue, but does tegra_ehci_hub_control() have an
out-of-bounds I/O memory access?

When handling root hub control requests, the core USB code in
rh_call_control() passes the raw, user-supplied wIndex (e.g., from usbfs via
the USBDEVFS_CONTROL ioctl) to the HCD's hub_control callback.

In tegra_ehci_hub_control(), which is passed in the usb->data structure here,
the pointer to the port status register is calculated without bounds checking
on wIndex:

status_reg = &ehci->regs->port_status[(wIndex & 0xff) - 1]

If wIndex & 0xff evaluates to 0, the calculation produces an index of -1
(pointing to the configured_flag register). If it is a large number like 255,
it results in an index of 254, reading far past the port_status array which
has a size of HCS_N_PORTS_MAX (15).

Could sending a SetPortFeature request with wValue = USB_PORT_FEAT_RESET
and a crafted wIndex allow a user with access to the root hub device node to
force the driver to perform multiple reads and writes at arbitrary MMIO
offsets?

> +	if (!usb->externally_controlled) {
> +		usb->dev = ci_hdrc_add_device(&pdev->dev, pdev->resource,
> +					      pdev->num_resources, &usb->data);
> +		if (IS_ERR(usb->dev)) {
> +			err = dev_err_probe(&pdev->dev, PTR_ERR(usb->dev),
> +					    "failed to add HDRC device");
> +			goto phy_shutdown;
> +		}
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260523083013.46372-1-clamor95@gmail.com?part=3