Re: [PATCH v12 12/36] phy: rockchip: usbdp: Limit DP lane count to muxed lanes

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing lock / data race on shared driver state during PHY configuration
- [High] Incomplete fix: DP lane bounds check is bypassed during voltage-only configurations
--

--- Patch [12]: [PATCH v12 12/36] phy: rockchip: usbdp: Limit DP lane count to muxed lanes ---
commit f343affd6c4a3599ea8955fa27e07174fef51eb5
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Limit DP lane count to muxed lanes

This patch adds a check to ensure the DP controller does not request more
lanes than the PHY is configured to support. This protects against USB3
usage corruption.

Link: https://lore.kernel.org/linux-phy/20260612165546.98E1F1F000E9@smtp.kernel.org/

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index c4d62b234ea80..b172ce6a722ee 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1099,6 +1099,9 @@ static int rk_udphy_dp_phy_verify_link_rate(struct rk_udphy *udphy,
>  static int rk_udphy_dp_phy_verify_lanes(struct rk_udphy *udphy,
>  					struct phy_configure_opts_dp *dp)
>  {
> +	if (dp->lanes > udphy->dp_lanes)
> +		return -EINVAL;
> +

[Severity: High]
This is a pre-existing issue, but is there a data race when accessing
udphy->dp_lanes here?

It appears this lockless read creates a TOCTOU vulnerability against
concurrent Type-C mode switches. The dw-dp driver invokes link training
concurrently with TCPM Type-C events invoking rk_udphy_typec_mux_set().

Since rk_udphy_dp_phy_configure() and rk_udphy_dp_phy_verify_lanes()
read and write shared variables without acquiring udphy->mutex (only the
generic phy->mutex is held):

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
        ...
        if (dp->set_voltages) {
                for (i = 0; i < udphy->lanes; i++) {
                        lane = udphy->dp_lane_sel[i];
        ...
}

Can this lead to hardware PHY corruption if DRM and TCPM use independent
workqueues with no common lock?

[Severity: High]
This isn't a bug introduced by this patch, but does this new check fully
protect the voltage configuration path?

If the dw-dp driver splits link training and first calls phy_configure()
with set_lanes = true, it caches dp->lanes into udphy->lanes. 

If it later calls it with set_voltages = true and set_lanes = false, this
bounds check in rk_udphy_dp_phy_verify_lanes() is bypassed.

If a Type-C event reduces the available udphy->dp_lanes before the second
call, the set_voltages block iterates up to the cached udphy->lanes:

drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
        ...
        if (dp->set_voltages) {
                for (i = 0; i < udphy->lanes; i++) {
                        lane = udphy->dp_lane_sel[i];
        ...
}

Could this override active USB3 lanes and corrupt the USB3 PHY state?

>  	switch (dp->lanes) {
>  	case 1:
>  	case 2:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-rockchip-usbdp-cleanup-v12-0-8b41a9a9bef0@collabora.com?part=12

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy