On 04/08/25 02:27, Sasha Levin wrote:
From: Eliav Farber farbere@amazon.com
[ Upstream commit 6bca1e955830808dc90e0506b2951b4256b81bbb ]
The interrupt handler in pps_gpio_probe() is registered after calling pps_register_source() using devm_request_irq(). However, in the corresponding remove function, pps_unregister_source() is called before the IRQ is freed, since devm-managed resources are released after the remove function completes.
This creates a potential race condition where an interrupt may occur after the PPS source is unregistered but before the handler is removed, possibly leading to a kernel panic.
To prevent this, switch from devm-managed IRQ registration to manual management by using request_irq() and calling free_irq() explicitly in the remove path before unregistering the PPS source. This ensures the interrupt handler is safely removed before deactivating the PPS source.
Signed-off-by: Eliav Farber farbere@amazon.com Link: https://lore.kernel.org/r/20250527053355.37185-1-farbere@amazon.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Sasha Levin sashal@kernel.org
Signed-off-by: Rodolfo Giometti giometti@enneenne.com
LLM Generated explanations, may be completely bogus:
Based on my analysis of the commit and the surrounding context, here is my assessment:
**Backport Status: YES**
This commit should be backported to stable kernel trees for the following reasons:
**Fixes a Real Bug**: The commit addresses a genuine race condition where an interrupt can occur after the PPS source is unregistered but before the IRQ handler is freed. This can lead to a kernel panic - a serious stability issue that affects users.
**Clear Race Condition Pattern**: The code shows the problematic ordering:
- In probe: `pps_register_source()` → `devm_request_irq()`
- In remove: `pps_unregister_source()` → (implicit devm cleanup frees IRQ)
This creates a window where the hardware can generate an interrupt
after the PPS source is gone but before the handler is removed, causing the handler to access freed memory.
**Minimal and Contained Fix**: The change is small and focused:
- Changes `devm_request_irq()` to `request_irq()`
- Adds explicit `free_irq()` call before `pps_unregister_source()`
- Only 3 lines of functional code changes
- No new features or architectural changes
**Similar to Other Stable Fixes**: This follows a common pattern seen in other drivers where devm resource ordering causes issues in removal paths. The kernel has many similar fixes for CAN drivers, network drivers, and other subsystems that have been backported to stable.
**Low Risk of Regression**: The change is straightforward and doesn't modify the driver's functionality - it only fixes the resource cleanup ordering. The manual IRQ management is a well-established pattern.
**Affects User-Visible Stability**: A kernel panic during device removal/module unload is a serious issue that can affect system stability, especially in environments where PPS devices might be dynamically added/removed or during system shutdown.
The commit message clearly describes the problem, the root cause, and the solution. The fix is conservative and follows established kernel patterns for fixing devm ordering issues. This is exactly the type of bug fix that stable kernel rules recommend for backporting.
drivers/pps/clients/pps-gpio.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c index 374ceefd6f2a..2866636b0554 100644 --- a/drivers/pps/clients/pps-gpio.c +++ b/drivers/pps/clients/pps-gpio.c @@ -210,8 +210,8 @@ static int pps_gpio_probe(struct platform_device *pdev) } /* register IRQ interrupt handler */
- ret = devm_request_irq(dev, data->irq, pps_gpio_irq_handler,
get_irqf_trigger_flags(data), data->info.name, data);
- ret = request_irq(data->irq, pps_gpio_irq_handler,
get_irqf_trigger_flags(data), data->info.name, data);@@ -228,6 +228,7 @@ static void pps_gpio_remove(struct platform_device *pdev) { struct pps_gpio_device_data *data = platform_get_drvdata(pdev);
- free_irq(data->irq, data); pps_unregister_source(data->pps); timer_delete_sync(&data->echo_timer); /* reset echo pin in any case */