Re: [PATCH v2 1/3] Input: applespi - force PIO mode on MacBook8,1
From: sashiko-bot@kernel.org
Date: 2026-07-11 07:04:04
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Incomplete restoration of `spi->controller->can_dma` leads to an execute-after-free on module unload if the original callback was NULL. - [High] Architectural violation and data race due to mutating shared SPI host controller, which is applied too late to prevent DMA timeouts. --
--- Patch [1]: [PATCH v2 1/3] Input: applespi - force PIO mode on MacBook8,1 ---commit c3d66aabf4cd47b8a9f7af8039fad4fc085aef5f Author: Shih-Yuan Lee [off-list ref] Input: applespi - force PIO mode on MacBook8,1 This patch introduces a DMI quirk to force PIO mode on MacBook8,1 laptops. It overrides the shared SPI controller's can_dma callback to avoid timeout errors during DMA handshakes, and attempts to restore the original callback when the driver is removed. [ ... ]
quoted hunk ↗ jump to hunk
--- a/drivers/input/keyboard/applespi.c +++ b/drivers/input/keyboard/applespi.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1788,6 +1800,19 @@ static int applespi_probe(struct spi_device *spi) debugfs_create_file("tp_dim", 0400, applespi->debugfs_root, applespi, &applespi_tp_dim_fops); + /* + * MacBook8,1's SPI host controller DMA is broken (timeout errors). + * Force PIO mode by overriding the controller's can_dma callback. + * + * Since we modify the shared controller's callback, we save the + * original pointer and restore it in applespi_remove(). + */ + applespi->original_can_dma = spi->controller->can_dma; + if (dmi_match(DMI_PRODUCT_NAME, "MacBook8,1")) { + dev_info(&spi->dev, "Disabling DMA for MacBook8,1 SPI to force PIO mode\n"); + spi->controller->can_dma = applespi_can_not_dma; + } + return 0; }
[Severity: High] Is this too late to prevent the DMA timeouts? The override occurs at the very end of applespi_probe(). However, early SPI transfers triggered earlier in the function via applespi_setup_spi() and applespi_init(), as well as any reads triggered by ACPI GPE interrupts that were already enabled, will still attempt to use DMA and fail. Furthermore, does mutating the shared SPI host controller's method table from a peripheral client driver cause data races with concurrent SPI core operations? Typically, controller-level hardware quirks need to be implemented in the SPI host controller driver itself rather than the client driver to maintain encapsulation and prevent races. [ ... ]
quoted hunk ↗ jump to hunk
@@ -1822,6 +1847,9 @@ static void applespi_remove(struct spi_device *spi) applespi_drain_reads(applespi); + if (applespi->original_can_dma) + spi->controller->can_dma = applespi->original_can_dma; + debugfs_remove_recursive(applespi->debugfs_root); }
[Severity: High] Will this leave a dangling pointer in the host controller if the original callback was NULL? If the host controller does not natively provide a can_dma callback, applespi->original_can_dma will be NULL. In that scenario, this conditional check evaluates to false and skips the restoration entirely. Once the applespi module is unloaded and unmapped from memory, the shared host controller will be left pointing to the freed applespi_can_not_dma function. Any subsequent SPI transfers on the host controller would then dereference this freed pointer, resulting in an execute-after-free and kernel panic. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260711065415.7396-1-fourdollars@debian.org?part=1