public inbox for linux-watchdog@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.18-5.10] via_wdt: fix critical boot hang due to unnamed resource allocation
       [not found] <20251210034915.2268617-1-sashal@kernel.org>
@ 2025-12-10  3:48 ` Sasha Levin
  0 siblings, 0 replies; only message in thread
From: Sasha Levin @ 2025-12-10  3:48 UTC (permalink / raw)
  To: patches, stable
  Cc: Li Qiang, Guenter Roeck, Wim Van Sebroeck, Sasha Levin,
	linux-watchdog

From: Li Qiang <liqiang01@kylinos.cn>

[ Upstream commit 7aa31ee9ec92915926e74731378c009c9cc04928 ]

The VIA watchdog driver uses allocate_resource() to reserve a MMIO
region for the watchdog control register. However, the allocated
resource was not given a name, which causes the kernel resource tree
to contain an entry marked as "<BAD>" under /proc/iomem on x86
platforms.

During boot, this unnamed resource can lead to a critical hang because
subsequent resource lookups and conflict checks fail to handle the
invalid entry properly.

Signed-off-by: Li Qiang <liqiang01@kylinos.cn>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Wim Van Sebroeck <wim@linux-watchdog.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis Summary

### 1. COMMIT MESSAGE ANALYSIS

**Subject:** "via_wdt: fix critical boot hang due to unnamed resource
allocation"
- Uses "fix" language indicating a bug fix
- Claims "critical boot hang" - a severe user-impacting issue
- Affects VIA chipset watchdog driver

**Tags analysis:**
- **Missing:** No "Cc: stable@vger.kernel.org" tag
- **Missing:** No "Fixes:" tag
- **Present:** Reviewed-by: Guenter Roeck (well-respected watchdog/hwmon
  maintainer)
- **Present:** Signed-off by both watchdog maintainers (Guenter Roeck
  and Wim Van Sebroeck)

### 2. CODE CHANGE ANALYSIS

The fix is a **single line addition**:
```c
wdt_res.name = "via_wdt";
```

**The bug mechanism:**
1. `wdt_res` is declared as a static `struct resource` without
   initialization at line 67
2. `allocate_resource()` is called without the resource having a name
   set
3. This results in a NULL `name` field, shown as `<BAD>` in
   `/proc/iomem`
4. The kernel's resource code in `kernel/resource.c:141` shows: `r->name
   ? r->name : "<BAD>"`

**Why this is needed:**
- Other watchdog drivers (e.g., `f71808e_wdt.c`) properly set
  `wdt_res.name = "superio port"`
- `struct resource` has a `name` field that should always be populated

### 3. CLASSIFICATION

- **Type:** Bug fix (not new feature)
- **Severity:** The commit claims "critical boot hang" - though the
  exact mechanism isn't fully clear from code inspection, unnamed
  resources can cause problems in resource lookup/conflict resolution
  paths
- **Scope:** Single driver, single line

### 4. SCOPE AND RISK ASSESSMENT

| Factor | Assessment |
|--------|------------|
| Lines changed | 1 |
| Files touched | 1 |
| Complexity | Extremely low |
| Risk of regression | **Zero** - adding a name to a resource cannot
cause problems |
| Self-contained | Yes, no dependencies |

### 5. USER IMPACT

- **Affected users:** VIA chipset hardware with watchdog (relatively
  rare, older hardware)
- **Severity if bug hits:** Boot hang (critical)
- **Bug age:** Present since driver was introduced in 2011
  (`dc3c56b703dad`)

### 6. STABILITY INDICATORS

- **Reviewed-by:** Guenter Roeck - respected maintainer
- **Correctness:** Obviously correct - other drivers do the same thing
- **Testing:** Implied through maintainer review

### 7. DEPENDENCY CHECK

- **No dependencies** - completely self-contained
- **Driver exists in all stable trees** - since 2011

## Risk vs Benefit Analysis

**Benefits:**
- Fixes potential boot hang for affected users
- Fixes incorrect `/proc/iomem` display (`<BAD>` entries)
- Brings via_wdt in line with other watchdog drivers

**Risks:**
- **None** - setting a resource name is a standard, safe operation

## Concerns

1. **No explicit stable request:** The maintainers didn't add `Cc:
   stable@vger.kernel.org`
2. **Boot hang claim verification:** The exact mechanism for the boot
   hang isn't easily traced in code, though I trust the
   reporter/maintainers' assessment
3. **Limited scope:** Only affects VIA chipset users (small population)

## Conclusion

This commit **should be backported** to stable kernels. Despite the
missing stable tag, it meets all stable criteria:

1. ✅ **Obviously correct** - trivial one-line fix matching other
   drivers' patterns
2. ✅ **Fixes a real bug** - unnamed resources are incorrect and can
   cause issues
3. ✅ **Small and contained** - one line in one file
4. ✅ **No new features** - just fixing existing code
5. ✅ **Zero regression risk** - adding a name cannot break anything
6. ✅ **Maintainer reviewed** - Guenter Roeck reviewed

The fix is so trivially safe that even if the boot hang is rare, there
is no downside to including it in stable.

**YES**

 drivers/watchdog/via_wdt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/watchdog/via_wdt.c b/drivers/watchdog/via_wdt.c
index d647923d68fed..f555763926518 100644
--- a/drivers/watchdog/via_wdt.c
+++ b/drivers/watchdog/via_wdt.c
@@ -165,6 +165,7 @@ static int wdt_probe(struct pci_dev *pdev,
 		dev_err(&pdev->dev, "cannot enable PCI device\n");
 		return -ENODEV;
 	}
+	wdt_res.name = "via_wdt";
 
 	/*
 	 * Allocate a MMIO region which contains watchdog control register
-- 
2.51.0


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2025-12-10  3:49 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20251210034915.2268617-1-sashal@kernel.org>
2025-12-10  3:48 ` [PATCH AUTOSEL 6.18-5.10] via_wdt: fix critical boot hang due to unnamed resource allocation Sasha Levin

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