Re: [PATCH v2 04/22] usb: typec: tipd: Publish CD321x partner alternate modes

[sashiko-bot@kernel.org]

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

New issues:
- [High] Probe error path causes Use-After-Free and data races when unregistering partner altmodes.

Pre-existing issues:
- [High] Uncancelled delayed works (`update_work` and `wq_poll`) in the `tps6598x_probe` error paths lead to use-after-free of the `tps` structure.
--

commit 32fed0e1c2a7fe442601560146889c74a0d8a3f2
Author: Sven Peter <sven@kernel.org>

    usb: typec: tipd: Publish CD321x partner alternate modes
    
    Register Thunderbolt partner and cable modes once their VDOs are
    available and USB4 as a partner mode once firmware has negotiated it,
    with the EUDO stored in its eudo field. The CD321X firmware still controls
    mode selection and entry itself. This just makes the negotiated modes
    available to consumers on the Type-C bus.
    
    Mark them active after the Type-C mux has been configured and deactivate
    them before the mux is switched back to its safe state to make sure any
    consuming driver is initialized and shut down in the correct order.
    
    Signed-off-by: Sven Peter <sven@kernel.org>

> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 8c7f93d4270e2..218b52b07b5a7 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
[ ... ]
> @@ -1380,6 +1565,7 @@ cd321x_unregister_port(struct tps6598x *tps)
>  {
>  	struct cd321x *cd321x = container_of(tps, struct cd321x, tps);
>  
> +	cd321x_unregister_partner_altmodes(cd321x);

[Severity: High]
Does this code introduce a use-after-free on the probe error path?

If tps6598x_probe() fails after a successful connect, it jumps to err_disconnect:

tps6598x_probe()
    ...
err_disconnect:
    tps6598x_disconnect(tps, 0);
err_unregister_port:
    tps->data->unregister_port(tps);

tps6598x_disconnect() unregisters the partner, which cascades and frees its 
children (the altmodes).

Next, tps->data->unregister_port() calls cd321x_unregister_port(), which
now calls cd321x_unregister_partner_altmodes(). 

Since the altmode pointers were never cleared to NULL on the error path, 
cd321x_unregister_partner_altmodes() will dereference the freed memory 
(e.g., cd321x->partner_altmode_tbt->active).

Could we clear the altmode pointers in tps6598x_disconnect() or check for their
validity before dereferencing?

>  	typec_mux_put(cd321x->mux);
>  	cd321x->mux = NULL;
>  	typec_unregister_altmode(cd321x->port_altmode_dp);
[ ... ]
> @@ -1802,6 +1988,14 @@ static void cd321x_remove(struct tps6598x *tps)
>  	struct cd321x *cd321x = container_of(tps, struct cd321x, tps);
>  
>  	cancel_delayed_work_sync(&cd321x->update_work);
> +
> +	guard(mutex)(&tps->lock);
> +	cd321x_deactivate_altmodes(cd321x);
> +	cd321x->state.alt = NULL;
> +	cd321x->state.mode = TYPEC_STATE_SAFE;
> +	cd321x->state.data = NULL;
> +	typec_set_mode(tps->port, TYPEC_STATE_SAFE);
> +	cd321x_unregister_partner_altmodes(cd321x);
>  }
>  
>  static int tps6598x_probe(struct i2c_client *client)

[Severity: High]
This is a pre-existing issue, but does the probe error path leak delayed works?

When tps6598x_probe() allocates `tps` via devm_kzalloc(), it queues delayed 
works. For example, cd321x_connect() schedules `update_work`, and if 
polling mode is chosen, `wq_poll` is scheduled. 

If probe fails after these works are queued (e.g., devm_request_threaded_irq 
fails), the function jumps to err_disconnect and returns an error without 
cancelling the delayed works:

tps6598x_probe() {
    ...
err_disconnect:
	tps6598x_disconnect(tps, 0);
err_unregister_port:
	tps->data->unregister_port(tps);
err_role_put:
    ...
    return ret;
}

Devres will then free the `tps` memory. When the delayed works execute 
later, they will dereference the freed `tps` structure, causing a panic.

Could we add cancel_delayed_work_sync() for the scheduled works in the probe 
error paths?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-b4-apple-soc-tbt-v2-0-1f80085f93fb@kernel.org?part=4