Re: [PATCH RESEND v7 3/3] usb: typec: tipd: Add sn201202x support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] usb: typec: tipd: Use-after-free of update_work on probe error path
- [Medium] usb: typec: tipd: sn201202x misses no_mode_control flag
--

commit 40f5bd6453edf7ec57cd88e8cdccab03d3c071c0
Author: Sasha Finkelstein <k@chaosmail.tech>

usb: typec: tipd: Add sn201202x support

Add support for sn201202x (aka ACE3), a tipd variant that uses a very
similar register map, that is exposed over a "logical register"
interface on the SPMI bus.

> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -1962,11 +1962,32 @@ static const struct tipd_data tipd_tps25750_data = {
[ ... ]
> +static const struct tipd_data tipd_sn201202x_data = {
> +	.irq_handler = cd321x_interrupt,

[Severity: Medium]
Should .no_mode_control = true be set here?

The sn201202x Type-C controller autonomously manages alternate mode
negotiation in hardware. By omitting .no_mode_control = true, the Type-C
core assumes the OS can control alternate modes, incorrectly exposing the
active and priority sysfs attributes as writable.

If a userspace daemon attempts to activate a mode via sysfs, the kernel
state will update without any hardware action, leading to a disjoint state
between kernel and hardware.

> +	.irq_mask1 = APPLE_CD_REG_INT_POWER_STATUS_UPDATE |
> +		     APPLE_CD_REG_INT_DATA_STATUS_UPDATE |
> +		     APPLE_CD_REG_INT_PLUG_EVENT,
> +	.tps_struct_size = sizeof(struct sn201202x),
> +	.remove = cd321x_remove,
> +	.register_port = cd321x_register_port,
> +	.unregister_port = cd321x_unregister_port,

[Severity: High]
Can this cause a use-after-free on the probe error path?

If devm_request_threaded_irq() or any subsequent step fails in tipd_init()
after .connect has been called (e.g., when a partner is already plugged in
at boot), the error path jumps to err_unregister_port.

This path calls tps->data->unregister_port() (which maps to
cd321x_unregister_port here) but fails to invoke .remove or explicitly
cancel the pending cd321x->update_work delayed work.

The probe function returns an error, causing the devres framework to free
the struct sn201202x containing the work item. When the scheduled work
eventually executes on the system workqueue, it dereferences the freed
memory.

Is there a way to ensure cancel_delayed_work_sync(&cd321x->update_work) is
called during this error unwind?

> +	.trace_data_status = trace_cd321x_data_status,
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010-tipd-ace3-v7-0-317218a96c01@chaosmail.tech?part=3