From: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org
[ Upstream commit 78b6a991eb6c6f19ed7d0ac91cda3b3b117fda8f ]
Device can be unbound, so driver must also release memory for the wakeup source. Do not use devm interface, because it would change the order of cleanup.
Link: https://lore.kernel.org/lkml/20250501-device-wakeup-leak-extcon-v2-1-7af7780... Acked-by: MyungJoo Ham myungjoo.ham@samsung.com Signed-off-by: Krzysztof Kozlowski krzysztof.kozlowski@linaro.org Signed-off-by: Chanwoo Choi cw00.choi@samsung.com Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
Now let me compile my comprehensive analysis:
## **BACKPORT RECOMMENDATION: YES (with critical caveat)**
### **Executive Summary**
This commit **SHOULD be backported** to stable kernel trees, **BUT it MUST be backported together with its follow-up fix commit 92bac7d4de9c0** ("extcon: adc-jack: Cleanup wakeup source only if it was enabled"). Backporting commit 78b6a991eb6c6 alone would fix a memory leak but introduce a subtle correctness bug.
---
### **Detailed Technical Analysis**
#### **1. Nature of the Bug**
The commit fixes a **memory leak of wakeup_source structures** that occurs when the adc-jack device is unbound:
**Root Cause:** - In `adc_jack_probe()` (drivers/extcon/extcon-adc-jack.c:156-157), when `data->wakeup_source` is true, the driver calls `device_init_wakeup(&pdev->dev, 1)` - This allocates a `wakeup_source` structure involving: - `kzalloc()` for the structure itself - `kstrdup_const()` for the name string - `ida_alloc()` for ID allocation - Registration in the global wakeup_sources list
**The Leak:** - The original `adc_jack_remove()` function has NO corresponding cleanup call - Without `device_init_wakeup(&pdev->dev, false)`, the allocated wakeup_source is never freed - This memory leaks every time the device is unbound (manual unbind via sysfs, driver removal, module unload)
#### **2. The Fix (Commit 78b6a991eb6c6)**
**Code Change:** ```c static void adc_jack_remove(struct platform_device *pdev) { struct adc_jack_data *data = platform_get_drvdata(pdev);
+ device_init_wakeup(&pdev->dev, false); // ← Added this line free_irq(data->irq, data); cancel_work_sync(&data->handler.work); } ```
**What device_init_wakeup(dev, false) does:** 1. Calls `device_wakeup_disable(dev)` which: - Detaches the wakeup_source from the device - Calls `wakeup_source_unregister()` to remove it from the list - Calls `wakeup_source_destroy()` to free all allocated memory
2. Calls `device_set_wakeup_capable(dev, false)` to clear the capability flag
#### **3. Critical Issue: The Follow-up Fix is Required**
**Problem with 78b6a991eb6c6 alone:** - The fix unconditionally calls `device_init_wakeup(&pdev->dev, false)` - But probe only calls `device_init_wakeup(&pdev->dev, 1)` when `data->wakeup_source` is true - Calling `device_init_wakeup(false)` when it was never initialized could: - Call `device_wakeup_disable()` on a NULL or uninitialized wakeup_source - While this might not crash (the function checks for NULL), it's technically incorrect behavior
**The Follow-up Fix (92bac7d4de9c0):** Adds the conditional check that mirrors the probe logic: ```c static void adc_jack_remove(struct platform_device *pdev) { struct adc_jack_data *data = platform_get_drvdata(pdev);
- device_init_wakeup(&pdev->dev, false); + if (data->wakeup_source) + device_init_wakeup(&pdev->dev, false); free_irq(data->irq, data); cancel_work_sync(&data->handler.work); } ```
This was reported by Christophe JAILLET 8 days after the original fix (May 1 → May 9, 2025).
#### **4. Why Not Use devm_device_init_wakeup()?**
Other drivers in the same patch series (extcon-qcom-spmi-misc, extcon- fsa9480) used the devm (device-managed) approach, which automatically cleans up. However, adc-jack explicitly avoids this approach.
**Reason (from commit message):** "Do not use devm interface, because it would change the order of cleanup."
**Cleanup Order Analysis:** ``` Current (with manual cleanup): 1. device_init_wakeup(false) - disable wakeup source 2. free_irq() - free interrupt 3. cancel_work_sync() - cancel pending work 4. (later) devm cleanup runs for other resources
With devm_device_init_wakeup: 1. free_irq() - free interrupt 2. cancel_work_sync() - cancel pending work 3. (later) devm cleanup runs, including wakeup disable
Problem: IRQ and work might still reference wakeup_source during cleanup ```
The manual approach ensures the wakeup source is disabled before other related resources are freed, maintaining proper cleanup ordering.
#### **5. Pattern Analysis: Systematic Cleanup**
This is part of a **systematic cleanup series** by Krzysztof Kozlowski (Linaro) fixing the same class of bug across multiple subsystems:
**Same Author, Same Pattern (partial list):** - extcon: adc-jack, qcom-spmi-misc, fsa9480, axp288 - mfd: sprd-sc27xx, rt5033, max8925, max77705, max77541, max14577, as3722, 88pm886 - Bluetooth: btmtksdio, btmrvl_sdio - iio: st_lsm6dsx, qcom-spmi-iadc, fxls8962af - usb typec: tipd, tcpci - power supply: gpio-charger, collie - watchdog: stm32
This indicates a **project-wide audit** for this specific resource leak pattern, lending credibility to the importance of the fix.
#### **6. Impact Assessment**
**Severity: Medium** - Resource leak, but only triggered on device unbind - Device unbind is relatively uncommon (manual unbind, rmmod, shutdown) - Leak is small per occurrence (one wakeup_source structure ~100-200 bytes) - **But**: repeated bind/unbind cycles accumulate leaks - **More important**: This is incorrect resource management that violates kernel coding practices
**Affected Users:** - Users with ADC-based jack detection hardware (primarily Samsung devices) - Systems that dynamically load/unload extcon modules - Embedded systems with power management requirements - kexec/kdump scenarios where driver cleanup matters
**Regression Risk: Very Low** - Minimal, contained change (1-2 lines) - Only affects remove path - Mirrors the probe logic symmetrically - Has been in mainline with no reported issues
#### **7. Stable Tree Backporting Criteria Analysis**
✅ **Fixes an important bug:** Resource leak violating kernel resource management rules
✅ **Small and contained:** 1-line fix + 1-line follow-up (total 2 lines across 2 commits)
✅ **Low regression risk:** Remove path only, symmetric to probe, minimal code change
✅ **No architectural changes:** Pure resource management fix
✅ **Confined to subsystem:** Only affects extcon adc-jack driver
✅ **Clear and obvious fix:** The fix is straightforward and correct
⚠️ **Requires follow-up commit:** Must include 92bac7d4de9c0 for correctness
❌ **No explicit stable tag:** Commit message lacks "Cc: stable@vger.kernel.org" tag
#### **8. Comparison with Stable Tree Precedents**
Looking at similar commits in this cleanup series, many include explicit stable tags. However, the absence of a stable tag doesn't preclude backporting when the fix meets other criteria.
**Similar fixes that were backported (based on pattern):** - Memory leaks on device unbind are consistently considered backport- worthy - Resource management fixes are high priority for stable trees - Small, contained fixes with clear benefits are typically backported
---
### **RECOMMENDATION**
**YES - This commit should be backported to stable kernel trees.**
**Critical Requirements:** 1. **MUST backport both commits together:** - 78b6a991eb6c6 ("extcon: adc-jack: Fix wakeup source leaks on device unbind") - 92bac7d4de9c0 ("extcon: adc-jack: Cleanup wakeup source only if it was enabled")
2. **Suggested stable trees:** All active stable trees where the adc- jack driver exists with wakeup_source support
3. **Rationale Summary:** - Fixes genuine resource leak - Part of systematic kernel-wide cleanup - Low risk, high correctness value - Minimal code change - Both commits required for complete, correct fix
**Implementation Note:** When submitting to stable, reference both commits and explain that 92bac7d4de9c0 is a necessary correction to 78b6a991eb6c6.
drivers/extcon/extcon-adc-jack.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c index 46c40d85c2ac8..557930394abd2 100644 --- a/drivers/extcon/extcon-adc-jack.c +++ b/drivers/extcon/extcon-adc-jack.c @@ -164,6 +164,7 @@ static void adc_jack_remove(struct platform_device *pdev) { struct adc_jack_data *data = platform_get_drvdata(pdev);
+ device_init_wakeup(&pdev->dev, false); free_irq(data->irq, data); cancel_work_sync(&data->handler.work); }