public inbox for linux-hwmon@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.18-6.17] hwmon: (emc2305) fix double put in emc2305_probe_childs_from_dt
       [not found] <20251212020903.4153935-1-sashal@kernel.org>
@ 2025-12-12  2:08 ` Sasha Levin
  2025-12-12  2:08 ` [PATCH AUTOSEL 6.18-6.17] hwmon: (emc2305) fix device node refcount leak in error path Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2025-12-12  2:08 UTC (permalink / raw)
  To: patches, stable; +Cc: Pei Xiao, Guenter Roeck, Sasha Levin, linux-hwmon

From: Pei Xiao <xiaopei01@kylinos.cn>

[ Upstream commit 541dfb49dcb80c2509e030842de77adfb77820f5 ]

./drivers/hwmon/emc2305.c:597:4-15: ERROR: probable double put

Device node iterators put the previous value of the index variable, so an
explicit put causes a double put.

Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
Link: https://lore.kernel.org/r/tencent_CD373F952BE48697C949E39CB5EB77841D06@qq.com
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis Summary

### What the Bug Is
This commit fixes a **double reference count decrement (double put)** on
a device_node structure in the `emc2305_probe_childs_from_dt()`
function.

**Technical explanation:**
1. The `for_each_child_of_node()` macro uses `of_get_next_child()`
   internally for iteration
2. As confirmed in the kernel documentation, `of_get_next_child()`
   "**Decrements the refcount of prev**" automatically when advancing to
   the next child
3. The buggy code explicitly calls `of_node_put(child)` before
   `continue`
4. When the loop continues, `of_get_next_child()` decrements the
   refcount again → **double put**

**Impact:** Double put causes reference count underflow which can lead
to:
- Use-after-free vulnerabilities
- Memory corruption
- Kernel crashes/instability

### Stable Tree Criteria Evaluation

| Criteria | Met? | Details |
|----------|------|---------|
| Obviously correct | ✅ | Standard DT iterator pattern fix |
| Fixes real bug | ✅ | Reference counting bug confirmed |
| Important issue | ✅ | Potential UAF/memory corruption |
| Small and contained | ✅ | Removes 4 lines in one function |
| No new features | ✅ | Pure bug fix |
| Tested | ✅ | Signed-off by hwmon maintainer |

### Risk Assessment
- **Risk**: Very low - the fix simply removes incorrect `of_node_put()`
  calls
- **Scope**: Single function, single driver (emc2305 hwmon)
- **Backport complexity**: None - straightforward removal of lines

### Version Analysis
The buggy code was introduced in commit `2ed4db7a1d07b` which first
appeared in **v6.17-rc1**. This means:
- Only kernels 6.17+ have this bug
- Older stable trees (6.12.y, 6.6.y, 6.1.y, etc.) do **NOT** have this
  code

### Concerns
- No explicit "Cc: stable@vger.kernel.org" tag
- No "Fixes:" tag pointing to the introducing commit
- However, the bug and fix are clearly documented and understood

### Verdict
This is a legitimate bug fix that corrects an obvious reference counting
error. The fix is:
- Trivially correct (well-known DT iterator pattern)
- Very low risk
- Fixes a real bug that can cause memory corruption

While the affected code only exists in 6.17+, this is still a valid
stable backport candidate for the 6.17.y stable branch and should be
backported to ensure stable users don't hit this reference counting bug.

**YES**

 drivers/hwmon/emc2305.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
index 84cb9b72cb6c2..ceae96c07ac45 100644
--- a/drivers/hwmon/emc2305.c
+++ b/drivers/hwmon/emc2305.c
@@ -593,10 +593,8 @@ static int emc2305_probe_childs_from_dt(struct device *dev)
 	for_each_child_of_node(dev->of_node, child) {
 		if (of_property_present(child, "reg")) {
 			ret = emc2305_of_parse_pwm_child(dev, child, data);
-			if (ret) {
-				of_node_put(child);
+			if (ret)
 				continue;
-			}
 			count++;
 		}
 	}
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH AUTOSEL 6.18-6.17] hwmon: (emc2305) fix device node refcount leak in error path
       [not found] <20251212020903.4153935-1-sashal@kernel.org>
  2025-12-12  2:08 ` [PATCH AUTOSEL 6.18-6.17] hwmon: (emc2305) fix double put in emc2305_probe_childs_from_dt Sasha Levin
@ 2025-12-12  2:08 ` Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2025-12-12  2:08 UTC (permalink / raw)
  To: patches, stable; +Cc: Pei Xiao, Guenter Roeck, Sasha Levin, linux-hwmon

From: Pei Xiao <xiaopei01@kylinos.cn>

[ Upstream commit 4910da6b36b122db50a27fabf6ab7f8611b60bf8 ]

The for_each_child_of_node() macro automatically manages device node
reference counts during normal iteration. However, when breaking out
of the loop early with return, the current iteration's node is not
automatically released, leading to a reference count leak.

Fix this by adding of_node_put(child) before returning from the loop
when emc2305_set_single_tz() fails.

This issue could lead to memory leaks over multiple probe cycles.

Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
Link: https://lore.kernel.org/r/tencent_5CDC08544C901D5ECA270573D5AEE3117108@qq.com
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of Commit: hwmon: (emc2305) fix device node refcount leak in
error path

### 1. COMMIT MESSAGE ANALYSIS

**Subject:** Clear "fix" language indicates a bug fix targeting a
"refcount leak" in an "error path"

**Body explanation:**
- The `for_each_child_of_node()` macro manages device node reference
  counts internally
- When breaking out of the loop early with `return`, the current node's
  reference is NOT automatically released
- This causes a reference count leak leading to memory leaks over
  multiple probe cycles

**Tags present:**
- `Signed-off-by` from author and maintainer (Guenter Roeck)
- `Link:` to mailing list discussion

**Tags missing:**
- No `Cc: stable@vger.kernel.org`
- No `Fixes:` tag

### 2. CODE CHANGE ANALYSIS

The fix is extremely small and surgical:

```c
for_each_child_of_node(dev->of_node, child) {
    ret = emc2305_set_single_tz(dev, child, i);
- if (ret != 0)
+   if (ret != 0) {
+       of_node_put(child);
        return ret;
+   }
    i++;
}
```

**Technical mechanism:**
- `for_each_child_of_node()` calls `of_node_get()` on each child
  internally
- On normal loop completion, the macro decrements the refcount
- On early exit (return/break), the caller must manually call
  `of_node_put()` to release the reference
- Without this, each failed probe leaves an unreleased reference →
  memory leak

**Root cause:** Missing required cleanup call when breaking out of
device tree iterator macro

**Why fix is correct:** This is the standard, well-documented pattern in
the Linux kernel for handling early exits from
`for_each_child_of_node()`. Adding `of_node_put(child)` before return is
textbook correct.

### 3. CLASSIFICATION

- **Bug type:** Resource leak (reference count / memory leak)
- **Category:** Standard bug fix
- **Security:** Not a security issue
- **New features:** None

### 4. SCOPE AND RISK ASSESSMENT

| Metric | Value |
|--------|-------|
| Lines changed | 3 (effectively +1 functional line) |
| Files touched | 1 |
| Complexity | Very low |
| Risk | Near zero |

**Risk analysis:**
- The fix only adds a cleanup call in an error path that already returns
  immediately
- Cannot possibly affect normal operation
- The `of_node_put()` function is well-tested core kernel infrastructure
- Adding required cleanup where it was missing cannot cause regression

### 5. USER IMPACT

**Affected users:** Those with EMC2305 fan controller hardware using
device tree

**Trigger conditions:**
1. Device must have child nodes in device tree
2. `emc2305_set_single_tz()` must fail
3. Must happen repeatedly over time

**Severity:** Low to medium - memory leak that accumulates over multiple
failed probe cycles. Not a crash or security issue, but can eventually
exhaust memory on long-running systems.

### 6. STABILITY INDICATORS

- Accepted by hwmon subsystem maintainer (Guenter Roeck)
- Simple, well-understood fix pattern
- No complex interactions possible

### 7. DEPENDENCY CHECK

- No dependencies on other commits
- `for_each_child_of_node()` and `of_node_put()` are long-standing
  kernel APIs
- The emc2305 driver must exist in the target stable tree

---

## Summary

**What the commit fixes:** A device node reference count leak in the
emc2305 hwmon driver that occurs when `emc2305_set_single_tz()` fails
during probe. This can lead to memory leaks over multiple probe cycles.

**Stable kernel rules assessment:**
1. ✅ **Obviously correct:** Standard kernel pattern, textbook fix
2. ✅ **Fixes real bug:** Yes, reference count leak causing memory leak
3. ⚠️ **Important issue:** Moderate severity - memory leak in error path
4. ✅ **Small and contained:** 3 lines changed in 1 file
5. ✅ **No new features:** Pure bug fix
6. ✅ **Clean application:** Should apply cleanly

**Risk vs Benefit:**
- **Risk:** Essentially zero - adds required cleanup in error path
- **Benefit:** Prevents memory leak on systems using this hardware

**Concerns:**
- No explicit `Cc: stable` tag from maintainer
- Bug requires specific error condition to trigger
- Affects only specific hardware

**Verdict:** Despite the lack of explicit stable tagging, this fix is a
textbook example of a safe backport candidate. The fix is trivially
correct, follows a well-established kernel pattern, has zero risk of
regression, and fixes a real (if low-severity) resource leak. Similar
`for_each_child_of_node()` refcount leak fixes are regularly backported
to stable trees.

**YES**

 drivers/hwmon/emc2305.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/emc2305.c b/drivers/hwmon/emc2305.c
index 60809289f8169..84cb9b72cb6c2 100644
--- a/drivers/hwmon/emc2305.c
+++ b/drivers/hwmon/emc2305.c
@@ -685,8 +685,10 @@ static int emc2305_probe(struct i2c_client *client)
 			i = 0;
 			for_each_child_of_node(dev->of_node, child) {
 				ret = emc2305_set_single_tz(dev, child, i);
-				if (ret != 0)
+				if (ret != 0) {
+					of_node_put(child);
 					return ret;
+				}
 				i++;
 			}
 		} else {
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-12-12  2:09 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20251212020903.4153935-1-sashal@kernel.org>
2025-12-12  2:08 ` [PATCH AUTOSEL 6.18-6.17] hwmon: (emc2305) fix double put in emc2305_probe_childs_from_dt Sasha Levin
2025-12-12  2:08 ` [PATCH AUTOSEL 6.18-6.17] hwmon: (emc2305) fix device node refcount leak in error path Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox