mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>,
	Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>,
	Chanwoo Choi <cw00.choi@samsung.com>,
	Sasha Levin <sashal@kernel.org>,
	myungjoo.ham@samsung.com, wens@csie.org, hansg@kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.17] extcon: axp288: Fix wakeup source leaks on device unbind
Date: Sat, 25 Oct 2025 11:54:32 -0400	[thread overview]
Message-ID: <20251025160905.3857885-41-sashal@kernel.org> (raw)
In-Reply-To: <20251025160905.3857885-1-sashal@kernel.org>

From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

[ Upstream commit 93ccf3f2f22ceaa975b462156f98527febee4fe5 ]

Device can be unbound, so driver must also release memory for the wakeup
source.

Link: https://lore.kernel.org/lkml/20250501-device-wakeup-leak-extcon-v2-2-7af77802cbea@linaro.org/
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.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:

## BACKPORT RECOMMENDATION: YES

### EXECUTIVE SUMMARY

This commit should **YES** be backported to stable kernel trees. It
fixes a memory leak that has existed since March 2020 (commit
9c94553099ef) when wakeup support was added to the AXP288 extcon driver.
The leak occurs whenever the device is unbound, and the fix is minimal,
safe, and appropriate for stable trees.

---

### DETAILED TECHNICAL ANALYSIS

#### 1. **THE BUG BEING FIXED**

**Nature of the leak:**
The commit fixes a resource leak in the wakeup source subsystem. When
`device_init_wakeup(dev, true)` is called (line 473 in the original
code, which was line 446 before the fix), it:

1. Calls `device_wakeup_enable()` (drivers/base/power/wakeup.c:328)
2. Which allocates a `struct wakeup_source` via
   `wakeup_source_register()` (line 339)
3. This allocation includes:
   - The wakeup_source structure itself
     (include/linux/pm_wakeup.h:43-64) containing spinlocks, timers,
     statistics counters
   - A dynamically allocated name string
   - A sysfs entry via `wakeup_source_sysfs_add()`
   - Addition to a global wakeup sources list

**When the leak occurs:**
- When the device is unbound via sysfs
  (`/sys/bus/platform/drivers/axp288_extcon/unbind`)
- When the module is unloaded (the driver is tristate, can be built as a
  module)
- During driver probe failure after wakeup initialization

Without proper cleanup, all these resources remain allocated and are
never freed, causing a memory leak.

#### 2. **THE FIX**

**Code change (drivers/extcon/extcon-axp288.c:473):**
```c
- device_init_wakeup(dev, true);
+       devm_device_init_wakeup(dev);
```

**How the fix works:**
The `devm_device_init_wakeup()` helper (added in commit b317268368546,
Dec 18, 2024) provides automatic resource management:

```c
static inline int devm_device_init_wakeup(struct device *dev)
{
        device_init_wakeup(dev, true);
        return devm_add_action_or_reset(dev, device_disable_wakeup,
dev);
}
```

This uses the devres framework to automatically call
`device_disable_wakeup()` when the device is unbound, ensuring proper
cleanup.

#### 3. **HISTORICAL CONTEXT**

**Timeline:**
- **March 23, 2020**: Wakeup support added via commit 9c94553099ef by
  Hans de Goede
  - This commit had **`Cc: stable@vger.kernel.org`** - indicating the
    feature was important enough for stable backporting
  - Introduced the `device_init_wakeup(dev, true)` call without cleanup
  - Has existed in the codebase for **~5 years**

- **December 18, 2024**: `devm_device_init_wakeup()` helper introduced
  (commit b317268368546)
  - Created specifically to address wakeup source leaks across the
    kernel
  - Commit message explicitly states: "Some drivers that enable device
    wakeup fail to properly disable it during their cleanup, which
    results in a memory leak"

- **May 1, 2025**: This fix applied (commit 93ccf3f2f22ce)
  - Part of a systematic cleanup across multiple subsystems
  - 4 extcon drivers fixed: adc-jack, axp288, fsa9480, qcom-spmi-misc
  - Similar fixes applied to 13+ drivers across iio, usb, power supply,
    gpio, rtc, mfd subsystems

#### 4. **AFFECTED HARDWARE & USERS**

**Device scope:**
- AXP288 PMIC used on **Intel Cherry Trail** (Atom Airmont) devices
- These are tablets and 2-in-1 convertible devices from 2015-2017 era
- Still in active use today
- Examples: ASUS T100HA, Acer Aspire Switch series, HP Stream tablets

**Driver characteristics:**
- Platform driver (drivers/extcon/extcon-axp288.c)
- **Tristate** configuration (can be module or built-in)
- Actively maintained (8 commits since leak introduction, 9 commits
  since 2020)
- Handles USB charger detection and USB role switching
- Critical for proper charging and USB functionality

#### 5. **RISK ASSESSMENT**

**Regression risk: MINIMAL**

**Why this fix is safe:**
1. **One-line change**: Single function call replacement
2. **Functionally equivalent**: `devm_device_init_wakeup(dev)` calls
   `device_init_wakeup(dev, true)` internally
3. **Only adds cleanup**: The devres action is added with
   `devm_add_action_or_reset()`, which handles errors
4. **No behavioral change**: Wakeup functionality remains identical
   during normal operation
5. **Unconditional usage**: Unlike the adc-jack driver (which required a
   followup fix), axp288 **always** enables wakeup, so no conditional
   cleanup needed
6. **Tested pattern**: Same approach used in 13+ drivers across the
   kernel

**What could go wrong:**
- Theoretically, if `devm_add_action_or_reset()` fails to add the
  cleanup action, it will call `device_init_wakeup(dev, false)`
  immediately via the _or_reset behavior
- This has no practical negative impact - the driver would simply not
  have wakeup enabled, which is safe

#### 6. **IMPACT & SEVERITY**

**User-visible impact:**
- Memory leak accumulates with each device unbind/rebind cycle
- Particularly relevant for:
  - Development and debugging scenarios (common to unbind/rebind
    drivers)
  - Systems with dynamic device management
  - Long-running systems where modules are loaded/unloaded
  - Testing environments

**Severity: MODERATE**
- Not a critical security issue
- Not a system crash or data corruption bug
- But: genuine resource leak that grows over time
- Affects real hardware in active use

#### 7. **STABLE TREE CRITERIA COMPLIANCE**

Checking against stable kernel rules:

✅ **It must be obviously correct and tested** - One line change,
functionally identical, widely tested pattern

✅ **It must fix a real bug that bothers people** - Real memory leak
affecting real hardware

✅ **It must fix a problem that causes a build error, oops, hang, data
corruption, a real security issue, or some "oh, that's not good" issue**
- Memory leak qualifies as "not good"

✅ **Serious issues as reported by a user of a distribution kernel may
also be considered if they fix a notable performance or interactivity
issue** - Resource leaks affect system health

✅ **It must not contain any "trivial" fixes** - This is a genuine bug
fix

✅ **It must follow the Documentation/process/submitting-patches.rst
rules** - Follows kernel coding standards

✅ **It or an equivalent fix must already exist in Linus' tree** - Commit
93ccf3f2f22ce is in mainline

❌ **No "theoretical race condition" fixes** - N/A

❌ **No "janitor" style fixes** - This is a real bug fix, not just
cleanup

✅ **It cannot contain any "trivial" spelling fixes** - N/A

✅ **It must be relatively small and self-contained** - Single line
change

✅ **It cannot be larger than 100 lines** - 1 line changed

#### 8. **RELATED COMMITS & DEPENDENCIES**

**Dependency:** Requires commit b317268368546 "PM: wakeup: implement
devm_device_init_wakeup() helper" (merged Dec 18, 2024 in v6.10)

**Note:** The dependency commit is already in stable trees since v6.10,
so this fix can be backported to kernels >= 6.10.

**Related fixes in the series:**
- 78b6a991eb6c: extcon: adc-jack: Fix wakeup source leaks
- 6f982d55f8c5d: extcon: fsa9480: Fix wakeup source leaks
- 369259d5104d6: extcon: qcom-spmi-misc: Fix wakeup source leaks

All four extcon fixes should be considered together for backporting.

#### 9. **WHY NO STABLE TAG?**

The original commit **does not have** a "Fixes:" tag or "Cc:
stable@vger.kernel.org". This is likely because:
1. The developer may have relied on the autosel process to pick it up
2. It's part of a systematic cleanup that might have been considered low
   priority
3. The leak only manifests during unbind, which is less common than
   other bugs

However, **this does not mean it shouldn't be backported**. The autosel
process exists precisely to catch fixes like this that should go to
stable but weren't explicitly tagged.

#### 10. **CODE VERIFICATION**

I verified the following in the codebase:
- The driver unconditionally calls wakeup initialization (line 473)
- No .remove() function exists, but leak still occurs on unbind
- suspend/resume functions correctly use `device_may_wakeup()` checks
  (lines 483, 498)
- No conditional logic around wakeup initialization (unlike adc-jack
  which needed a followup fix)

---

### CONCLUSION

**BACKPORT RECOMMENDATION: YES**

This is a clean, minimal, safe fix for a real memory leak that has
existed since 2020. The fix:
- Resolves a genuine resource leak affecting real hardware
- Is minimal (one line) with negligible regression risk
- Uses a well-tested pattern applied across many drivers
- Affects actively-used Intel Cherry Trail devices
- Meets all stable kernel criteria

The commit should be backported to all stable kernels that have:
1. The wakeup support (commit 9c94553099ef from v5.7+)
2. The devm helper (commit b317268368546 from v6.10+)

**Recommended stable backport targets: v6.10+ stable trees**

 drivers/extcon/extcon-axp288.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index d3bcbe839c095..19856dddade62 100644
--- a/drivers/extcon/extcon-axp288.c
+++ b/drivers/extcon/extcon-axp288.c
@@ -470,7 +470,7 @@ static int axp288_extcon_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	device_init_wakeup(dev, true);
+	devm_device_init_wakeup(dev);
 	platform_set_drvdata(pdev, info);
 
 	return 0;
-- 
2.51.0


  parent reply	other threads:[~2025-10-25 16:11 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20251025160905.3857885-1-sashal@kernel.org>
2025-10-25 15:53 ` [PATCH AUTOSEL 6.17] wifi: mt76: improve phy reset on hw restart Sasha Levin
2025-10-25 15:53 ` [PATCH AUTOSEL 6.17-6.12] Bluetooth: btusb: Add new VID/PID 13d3/3633 for MT7922 Sasha Levin
2025-10-25 15:54 ` Sasha Levin [this message]
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17] wifi: mt76: mt7996: Set def_wcid pointer in mt7996_mac_sta_init_link() Sasha Levin
2025-10-25 15:54 ` [PATCH AUTOSEL 6.17-6.12] ASoC: mediatek: Use SND_JACK_AVOUT for HDMI/DP jacks Sasha Levin
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17-6.6] wifi: mt76: mt7996: Temporarily disable EPCS Sasha Levin
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17] scsi: ufs: core: Change MCQ interrupt enable flow Sasha Levin
2025-10-25 15:55 ` [PATCH AUTOSEL 6.17-6.1] scsi: ufs: host: mediatek: Fix invalid access in vccqx handling Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-6.1] scsi: ufs: host: mediatek: Change reset sequence for improved stability Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-6.6] scsi: ufs: host: mediatek: Disable auto-hibern8 during power mode changes Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-6.6] char: Use list_del_init() in misc_deregister() to reinitialize list pointer Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-6.12] wifi: mt76: mt76_eeprom_override to int Sasha Levin
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17] wifi: mt76: mt7996: disable promiscuous mode by default Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17-6.12] scsi: ufs: host: mediatek: Fix PWM mode switch issue Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17-6.6] scsi: ufs: host: mediatek: Enhance recovery on hibernation exit failure Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17-6.12] wifi: mt76: mt7996: fix memory leak on mt7996_mcu_sta_key_tlv error Sasha Levin
2025-10-25 15:57 ` [PATCH AUTOSEL 6.17] extcon: fsa9480: Fix wakeup source leaks on device unbind Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17] wifi: mt76: mt7925: add pci restore for hibernate Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17] scsi: ufs: host: mediatek: Fix adapt issue after PA_Init Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17-6.1] wifi: mt76: mt7921: Add 160MHz beamformee capability for mt7922 device Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17-6.6] scsi: ufs: core: Disable timestamp functionality if not supported Sasha Levin
2025-10-25 15:58 ` [PATCH AUTOSEL 6.17-6.1] scsi: ufs: host: mediatek: Assign power mode userdata before FASTAUTO mode change Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17] Bluetooth: btusb: Add new VID/PID 13d3/3627 for MT7925 Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17-5.4] extcon: adc-jack: Fix wakeup source leaks on device unbind Sasha Levin
2025-10-25 15:59 ` [PATCH AUTOSEL 6.17] wifi: mt76: mt7996: support writing MAC TXD for AddBA Request Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17] mei: make a local copy of client uuid in connect Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17-6.1] ftrace: Fix softlockup in ftrace_module_enable Sasha Levin
2025-10-25 19:25   ` Steven Rostedt
2025-10-28 17:48     ` Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17-6.6] scsi: ufs: host: mediatek: Fix auto-hibern8 timer configuration Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17-6.12] scsi: ufs: host: mediatek: Fix unbalanced IRQ enable issue Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17-6.1] scsi: ufs: host: mediatek: Enhance recovery on resume failure Sasha Levin
2025-10-25 16:00 ` [PATCH AUTOSEL 6.17] wifi: mt76: use altx queue for offchannel tx on connac+ Sasha Levin
2025-10-25 16:01 ` [PATCH AUTOSEL 6.17] wifi: mt76: mt7996: Fix mt7996_reverse_frag0_hdr_trans for MLO Sasha Levin
2025-10-25 16:01 ` [PATCH AUTOSEL 6.17-6.12] scsi: ufs: host: mediatek: Correct system PM flow Sasha Levin
2025-10-25 16:01 ` [PATCH AUTOSEL 6.17-5.15] page_pool: always add GFP_NOWARN for ATOMIC allocations Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251025160905.3857885-41-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=cw00.choi@samsung.com \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=hansg@kernel.org \
    --cc=krzysztof.kozlowski@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=myungjoo.ham@samsung.com \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=wens@csie.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®