Re: [PATCH 16/19] thunderbolt: Add Apple Silicon support
flat view
From: sashiko-bot@kernel.org
Date: 2026-08-30 20:39:58
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Use-After-Free of `acio` when the `apple-cio` driver is unbound via sysfs while a Type-C switch reference is still held. - [High] Missing `reset_control_assert` in `apple_cio_stop` and error paths, leading to unbalanced hardware reset state. -- commit 6a1dc8288abd453dc286fa2a4ef8d8b16dc3f168 Author: Sven Peter [off-list ref] thunderbolt: Add Apple Silicon support This commit adds a platform driver for the ACIO host router complex and Native Host Interface (NHI) found on Apple Silicon SoCs. It brings up the coprocessor in response to Type-C notifications and handles child devices.
quoted hunk ↗ jump to hunk
diff --git a/drivers/thunderbolt/apple.c b/drivers/thunderbolt/apple.c new file mode 100644 --- /dev/null +++ b/drivers/thunderbolt/apple.c
[ ... ]
quoted hunk ↗ jump to hunk
+static void apple_cio_stop(struct apple_cio *acio) +{ + int ret, i;
[ ... ]
quoted hunk ↗ jump to hunk
+ /* Finally, remove the links to the PD domains to power everything off */ + for (i = 0; i < acio->pd_list->num_pds; i++) { + if (acio->pd_list->pd_links[i]) + device_link_del(acio->pd_list->pd_links[i]); + acio->pd_list->pd_links[i] = NULL; + } + + acio->current_cable_info = 0; +}
[Severity: High] Does this sequence need to assert the reset control before cutting power? When a cable is disconnected, apple_cio_stop() cuts the ACIO power domains but does not call reset_control_assert() on acio->reset. Because the hardware reset remains in the deasserted state in the reset controller, could the next cable insertion fail to generate the required transition (assert -> deassert) when power is reapplied in apple_cio_start()? [ ... ]
+static int apple_cio_start(struct apple_cio *acio)
+{[ ... ]
quoted hunk ↗ jump to hunk
+ /* + * After the power domains are on we need to signal and wait for the ACIO block + * to actually start before we can bring up the co-processor. + */ + ret = reset_control_deassert(acio->reset); + if (ret) { + dev_err(acio->dev, "ACIO block failed to start: %d\n", ret); + goto remove_links; + }
[ ... ]
quoted hunk ↗ jump to hunk
+remove_links: + /* Cut power to reset the entire block */ + for (i = 0; i < acio->pd_list->num_pds; i++) { + if (acio->pd_list->pd_links[i]) + device_link_del(acio->pd_list->pd_links[i]); + acio->pd_list->pd_links[i] = NULL; + } + return ret; +}
[Severity: High] Similar to apple_cio_stop(), the remove_links error path drops the power domains, but it looks like the reset might remain deasserted if this path is reached after reset_control_deassert() succeeds. Would it be safer to ensure reset_control_assert() is called on the error paths before dropping power? [ ... ]
quoted hunk ↗ jump to hunk
+static int apple_cio_tbt_switch_set(struct typec_thunderbolt_switch_dev *sw, + const struct typec_thunderbolt_switch_data *data) +{ + struct apple_cio *acio = typec_thunderbolt_switch_get_drvdata(sw); + + guard(mutex)(&acio->lock);
[ ... ]
quoted hunk ↗ jump to hunk
+static void apple_cio_remove(struct platform_device *pdev) +{ + struct apple_cio *acio = platform_get_drvdata(pdev); + + typec_thunderbolt_switch_unregister(acio->tbt_switch); + + guard(mutex)(&acio->lock); + if (acio->current_cable_info) + apple_cio_stop(acio); +}
[Severity: High] Can this lead to a use-after-free of the acio structure if the driver is unbound while a Type-C switch reference is still held? If the thunderbolt-apple-acio driver is unbound via sysfs, apple_cio_remove() will unregister the switch, but devres will immediately free the acio structure. If the Type-C port driver (tipd) still retains a reference to the switch device, and a cable hotplug occurs, could the port driver invoke apple_cio_tbt_switch_set() and dereference the freed acio pointer to acquire its mutex? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260830-b4-apple-soc-tbt-v1-0-44bc9348683c@kernel.org?part=16