Re: [PATCH v7 22/27] phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB

[sashiko-bot@kernel.org]

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

New issues:
- [High] A critical boolean logic error in `rk_udphy_typec_mux_set` incorrectly uses logical OR (`||`), causing all valid Type-C events to be ignored and breaking the state machine.

Pre-existing issues:
- [High] NULL pointer dereference when handling non-DP accessory modes (like Audio Accessory) in Type-C mux drivers.
--

commit b36f3de7b0f44156b05e5cda72d2bdd174f98299
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB

This patch handles TYPEC_STATE_SAFE and TYPEC_STATE_USB Type-C state events
so that the muxing is properly updated when exiting DP AltMode.

Fixes: 2f70bbddeb45 ("phy: rockchip: add usbdp combo phy driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/message/20260619155020.CC7361F000E9%40smtp.kernel.org

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 0bcaf54c5645d..0119b6229b0a9 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1329,11 +1329,12 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>  	struct rk_udphy *udphy = typec_mux_get_drvdata(mux);
>  
>  	/*
> -	 * Ignore mux events not involving DP AltMode, because
> -	 * the mode field is being reused, e.g. state->mode == 4
> -	 * could be either TYPEC_MODE_USB4 or TYPEC_DP_STATE_C.
> +	 * Ignore mux events not involving Safe State, USB State or DP AltMode,
> +	 * because the mode field is being reused, e.g. state->mode == 4 could
> +	 * be either TYPEC_MODE_USB4 or TYPEC_DP_STATE_C.
>  	 */
> -	if (!state->alt || state->alt->svid != USB_TYPEC_DP_SID)
> +	if (state->mode >= TYPEC_STATE_MODAL || !state->alt ||
> +	    state->alt->svid != USB_TYPEC_DP_SID)
>  		return 0;

[Severity: High]
Does this conditional unintentionally ignore all valid states by using
logical OR instead of logical AND?

If a DP AltMode event is passed, state->mode >= TYPEC_STATE_MODAL evaluates
to true, causing an early return. 

If a Safe State or USB State event is passed, state->alt is NULL, which also
causes an early return. 

This appears to short-circuit and return 0 for all expected Type-C muxing
events, breaking the state machine.


[Severity: High]
This is a pre-existing issue, but can accessory modes trigger a NULL pointer
dereference in other Type-C mux drivers?

When the Type-C port is set to an accessory mode like TYPEC_MODE_AUDIO,
state->mode >= TYPEC_STATE_MODAL evaluates to true, but state->alt is
initialized to NULL.

In drivers/usb/typec/mux/it5205.c:it5205_mux_set():

	if (state->mode >= TYPEC_STATE_MODAL &&
	    state->alt->svid != USB_TYPEC_DP_SID)
		return -EINVAL;

And similarly in drivers/usb/typec/mux/tusb1046.c:tusb1046_mux_set():

	if (state->mode >= TYPEC_STATE_MODAL &&
	    state->alt->svid != USB_TYPEC_DP_SID)
		return -EINVAL;

Does this condition dereference state->alt without checking if it is NULL
when a non-DP modal device is plugged in?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-rockchip-usbdp-cleanup-v7-0-38eb3cf654fd@collabora.com?part=22