From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Jinhui Guo <guojinhui.liam@bytedance.com>,
Corey Minyard <corey@minyard.net>,
Sasha Levin <sashal@kernel.org>,
openipmi-developer@lists.sourceforge.net
Subject: [PATCH AUTOSEL 6.18-5.10] ipmi: Fix __scan_channels() failing to rescan channels
Date: Tue, 9 Dec 2025 22:48:52 -0500 [thread overview]
Message-ID: <20251210034915.2268617-11-sashal@kernel.org> (raw)
In-Reply-To: <20251210034915.2268617-1-sashal@kernel.org>
From: Jinhui Guo <guojinhui.liam@bytedance.com>
[ Upstream commit 6bd30d8fc523fb880b4be548e8501bc0fe8f42d4 ]
channel_handler() sets intf->channels_ready to true but never
clears it, so __scan_channels() skips any rescan. When the BMC
firmware changes a rescan is required. Allow it by clearing
the flag before starting a new scan.
Signed-off-by: Jinhui Guo <guojinhui.liam@bytedance.com>
Message-ID: <20250930074239.2353-3-guojinhui.liam@bytedance.com>
Signed-off-by: Corey Minyard <corey@minyard.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Analysis of IPMI Channel Rescan Fix
### 1. COMMIT MESSAGE ANALYSIS
**Subject:** "ipmi: Fix __scan_channels() failing to rescan channels" -
clear bug fix indication.
**Problem described:**
- `channel_handler()` sets `intf->channels_ready = true` but never
clears it
- This causes `__scan_channels()` to skip rescans
- When BMC firmware changes, a rescan is required but doesn't happen
**Tags check:**
- No "Cc: stable@vger.kernel.org" tag
- No "Fixes:" tag
- Signed off by IPMI maintainer Corey Minyard
### 2. CODE CHANGE ANALYSIS
The fix adds a `bool rescan` parameter to `__scan_channels()`:
```c
static int __scan_channels(struct ipmi_smi *intf,
struct ipmi_device_id *id,
bool rescan)
{
if (rescan) {
/* Clear channels_ready to force channels rescan. */
intf->channels_ready = false;
}
...
}
```
**Call site updates:**
- `ipmi_add_smi()`: `__scan_channels(intf, &id, false)` - initial scan
- `__bmc_get_device_id()` after BMC re-registration:
`__scan_channels(intf, &id, false)` - fresh state
- `__bmc_get_device_id()` when version info changes:
`__scan_channels(intf, &bmc->fetch_id, true)` - rescan needed
**Bug mechanism:** When BMC firmware changes and `__bmc_get_device_id()`
detects version info differences, it calls `__scan_channels()` to update
channel information. However, since `channels_ready` was already set
`true` from the initial scan, the rescan logic is skipped, leaving stale
channel information.
### 3. CLASSIFICATION
- **Type:** Bug fix (not a feature)
- **Category:** Functional bug in existing driver logic
- **Security:** No security implications
### 4. SCOPE AND RISK ASSESSMENT
**Scope:**
- 1 file changed: `drivers/char/ipmi/ipmi_msghandler.c`
- ~15 lines of actual changes (mostly parameter additions)
- Localized to the `__scan_channels()` function and its callers
**Risk:** LOW
- The logic is simple and obvious: clear a boolean flag before
rescanning
- No complex interactions or side effects
- The differentiation between initial scan (`false`) and rescan (`true`)
is well-reasoned
### 5. USER IMPACT
**Affected users:**
- Servers with IPMI/BMC interfaces (common in enterprise/datacenter
environments)
- Users who update BMC firmware while the system is running
**Impact without fix:**
- After BMC firmware updates, IPMI channel information becomes stale
- System management through IPMI may malfunction
- Users must reboot to get correct channel information
**Severity:** Medium - affects functionality, not crashes or data
corruption
### 6. STABILITY INDICATORS
- Signed off by IPMI subsystem maintainer
- The fix logic is straightforward and verifiable by inspection
- No complex algorithmic changes
### 7. DEPENDENCY CHECK
- Self-contained fix with no dependencies on other commits
- `ipmi_msghandler.c` exists in all stable trees (mature driver)
- The affected functions (`__scan_channels`, `channel_handler`) exist in
stable kernels
### Risk vs Benefit Assessment
**Benefits:**
- Fixes real-world bug: BMC firmware updates are common maintenance
operations
- Small, surgical fix with minimal code changes
- IPMI is critical for server management in enterprise environments
**Risks:**
- Very low - the change is obviously correct
- Adding a parameter and clearing a flag is straightforward
- No behavioral changes to working code paths (initial scan unchanged)
### Concerns
1. **No Fixes: tag** - Cannot determine when bug was introduced, making
it harder to know which stable versions need it
2. **No explicit Cc: stable** - Author didn't request backport
3. **Specific scenario** - Only affects systems where BMC firmware
changes at runtime
### Conclusion
This commit fixes a legitimate functional bug where IPMI channel
rescanning fails after BMC firmware changes. The fix is:
- **Obviously correct**: Clear a "ready" flag before rescanning
- **Small and contained**: ~15 lines in one file
- **Fixes a real bug**: Users who update BMC firmware encounter this
- **Low risk**: Simple boolean flag manipulation
While it lacks explicit stable tags, the fix is valuable for enterprise
users who rely on IPMI for server management. The change is minimal, the
logic is clear, and it restores expected functionality without
introducing new features.
**YES**
drivers/char/ipmi/ipmi_msghandler.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index d3f84deee4513..0a886399f9daf 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -599,7 +599,8 @@ static void __ipmi_bmc_unregister(struct ipmi_smi *intf);
static int __ipmi_bmc_register(struct ipmi_smi *intf,
struct ipmi_device_id *id,
bool guid_set, guid_t *guid, int intf_num);
-static int __scan_channels(struct ipmi_smi *intf, struct ipmi_device_id *id);
+static int __scan_channels(struct ipmi_smi *intf,
+ struct ipmi_device_id *id, bool rescan);
static void free_ipmi_user(struct kref *ref)
{
@@ -2668,7 +2669,7 @@ static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc,
if (__ipmi_bmc_register(intf, &id, guid_set, &guid, intf_num))
need_waiter(intf); /* Retry later on an error. */
else
- __scan_channels(intf, &id);
+ __scan_channels(intf, &id, false);
if (!intf_set) {
@@ -2688,7 +2689,7 @@ static int __bmc_get_device_id(struct ipmi_smi *intf, struct bmc_device *bmc,
goto out_noprocessing;
} else if (memcmp(&bmc->fetch_id, &bmc->id, sizeof(bmc->id)))
/* Version info changes, scan the channels again. */
- __scan_channels(intf, &bmc->fetch_id);
+ __scan_channels(intf, &bmc->fetch_id, true);
bmc->dyn_id_expiry = jiffies + IPMI_DYN_DEV_ID_EXPIRY;
@@ -3438,10 +3439,17 @@ channel_handler(struct ipmi_smi *intf, struct ipmi_recv_msg *msg)
/*
* Must be holding intf->bmc_reg_mutex to call this.
*/
-static int __scan_channels(struct ipmi_smi *intf, struct ipmi_device_id *id)
+static int __scan_channels(struct ipmi_smi *intf,
+ struct ipmi_device_id *id,
+ bool rescan)
{
int rv;
+ if (rescan) {
+ /* Clear channels_ready to force channels rescan. */
+ intf->channels_ready = false;
+ }
+
if (ipmi_version_major(id) > 1
|| (ipmi_version_major(id) == 1
&& ipmi_version_minor(id) >= 5)) {
@@ -3656,7 +3664,7 @@ int ipmi_add_smi(struct module *owner,
}
mutex_lock(&intf->bmc_reg_mutex);
- rv = __scan_channels(intf, &id);
+ rv = __scan_channels(intf, &id, false);
mutex_unlock(&intf->bmc_reg_mutex);
if (rv)
goto out_err_bmc_reg;
--
2.51.0
next prev parent reply other threads:[~2025-12-10 3:49 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-10 3:48 [PATCH AUTOSEL 6.18-6.17] functionfs: fix the open/removal races Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-5.10] scsi: qla2xxx: Use reinit_completion on mbx_intr_comp Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-5.10] via_wdt: fix critical boot hang due to unnamed resource allocation Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-5.10] ti-sysc: allow OMAP2 and OMAP4 timers to be reserved on AM33xx Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-5.15] scsi: qla2xxx: Fix lost interrupts with qlini_mode=disabled Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-6.12] exfat: zero out post-EOF page cache on file extension Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-6.12] scsi: smartpqi: Add support for Hurray Data new controller PCI device Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-6.12] scsi: ufs: host: mediatek: Fix shutdown/suspend race condition Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-5.10] powerpc/addnote: Fix overflow on 32-bit builds Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-6.6] fuse: Invalidate the page cache after FOPEN_DIRECT_IO write Sasha Levin
2025-12-10 3:48 ` Sasha Levin [this message]
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-5.10] scsi: qla2xxx: Fix initiator mode with qlini_mode=exclusive Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-6.17] um: init cpu_tasks[] earlier Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-5.10] reset: fix BIT macro reference Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-5.10] clk: mvebu: cp110 add CLK_IGNORE_UNUSED to pcie_x10, pcie_x11 & pcie_x4 Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-6.6] fuse: Always flush the page cache before FOPEN_DIRECT_IO write Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-5.10] firmware: imx: scu-irq: Init workqueue before request mbox channel Sasha Levin
2025-12-10 3:48 ` [PATCH AUTOSEL 6.18-6.17] scsi: lpfc: Fix reusing an ndlp that is marked NLP_DROPPED during FLOGI Sasha Levin
2025-12-10 3:49 ` [PATCH AUTOSEL 6.18-5.10] exfat: fix remount failure in different process environments Sasha Levin
2025-12-10 3:49 ` [PATCH AUTOSEL 6.18-5.10] ipmi: Fix the race between __scan_channels() and deliver_response() 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=20251210034915.2268617-11-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=corey@minyard.net \
--cc=guojinhui.liam@bytedance.com \
--cc=openipmi-developer@lists.sourceforge.net \
--cc=patches@lists.linux.dev \
--cc=stable@vger.kernel.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