Re: [PATCH v11 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] The DP lane bounds check and PHY configuration execute without holding the driver's mutex, creating a TOCTOU race condition with concurrent Type-C events.
- [High] The newly added lane count check is bypassed during standalone DP voltage updates, allowing the same USB3 register corruption the patch intended to fix.
--

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

This patch adds a check to limit the DisplayPort lane count to the muxed
lanes, preventing the DP controller from configuring 4 lanes when the PHY
is restricted to 2 lanes. This aims to protect USB3 usage from being
corrupted by incorrect voltage configurations.

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 does this bounds check need to be
protected by a lock?

If a Type-C event (like a cable flip reducing DP lanes from 4 to 2)
occurs concurrently with a DP link training operation, could the bounds
check pass while udphy->dp_lanes is 4?

If a context switch allows the Type-C handler to reduce the lane count
and change udphy->lane_mux_sel while holding udphy->mutex, when this
thread resumes and proceeds to rk_udphy_dp_phy_configure() (which is
called without holding udphy->mutex), could it overwrite hardware
registers of physical lanes now assigned to USB3:

rk_udphy_dp_phy_configure() {
    ...
    lane = udphy->dp_lane_sel[i];
    ...
    regmap_update_bits(udphy->pma_regmap,
                       TRSV_ANA_TX_CLK_OFFSET_N(lane),
                       LN_ANA_TX_SER_TXCLK_INV,
                       FIELD_PREP(LN_ANA_TX_SER_TXCLK_INV,
                       udphy->lane_mux_sel[lane]));
    ...
}

[Severity: High]
This is also a pre-existing issue, but will this check be bypassed
during standalone DP voltage updates?

If a Type-C event asynchronously reduces udphy->dp_lanes (e.g., from
4 down to 2), and the DP controller subsequently issues a standalone
voltage update (where dp->set_voltages=1 and dp->set_lanes=0) prior to
a full link retrain:

rk_udphy_dp_phy_configure() {
    ...
    if (dp->set_lanes) {
        ret = rk_udphy_dp_phy_verify_lanes(udphy, dp);
        if (ret)
            return ret;
    }
    ...
}

Would rk_udphy_dp_phy_verify_lanes() be bypassed entirely? This would
allow the configuration loop to iterate up to the stale cached
udphy->lanes limit:

rk_udphy_dp_phy_configure() {
    ...
    for (i = 0; i < udphy->lanes; i++) {
    ...

and potentially cause the out-of-bounds writes to the USB3 hardware
registers that this patch intends to prevent.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-rockchip-usbdp-cleanup-v11-0-a149ac60f76c@collabora.com?part=12