From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Zhe Qiao <qiaozhe@iscas.ac.cn>,
Dan Carpenter <dan.carpenter@linaro.org>,
"Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
Sasha Levin <sashal@kernel.org>,
rafael@kernel.org, bhelgaas@google.com,
linux-acpi@vger.kernel.org, linux-pci@vger.kernel.org
Subject: [PATCH AUTOSEL 6.15 06/23] Revert "PCI/ACPI: Fix allocated memory release on error in pci_acpi_scan_root()"
Date: Mon, 30 Jun 2025 16:44:11 -0400 [thread overview]
Message-ID: <20250630204429.1357695-6-sashal@kernel.org> (raw)
In-Reply-To: <20250630204429.1357695-1-sashal@kernel.org>
From: Zhe Qiao <qiaozhe@iscas.ac.cn>
[ Upstream commit 2b8be57fa0c88ac824a906f29c04d728f9f6047a ]
This reverts commit 631b2af2f357 ("PCI/ACPI: Fix allocated memory release
on error in pci_acpi_scan_root()").
The reverted patch causes the 'ri->cfg' and 'root_ops' resources to be
released multiple times.
When acpi_pci_root_create() fails, these resources have already been
released internally by the __acpi_pci_root_release_info() function.
Releasing them again in pci_acpi_scan_root() leads to incorrect behavior
and potential memory issues.
We plan to resolve the issue using a more appropriate fix.
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/all/aEmdnuw715btq7Q5@stanley.mountain/
Signed-off-by: Zhe Qiao <qiaozhe@iscas.ac.cn>
Acked-by: Dan Carpenter <dan.carpenter@linaro.org>
Link: https://patch.msgid.link/20250619072608.2075475-1-qiaozhe@iscas.ac.cn
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
Based on my analysis of the commit and examination of the kernel code, I
need to determine if this revert should be backported.
**YES**, this commit should be backported to stable kernel trees.
## Extensive Explanation:
### 1. **Critical Double-Free Bug Fix**
This revert fixes a serious double-free bug introduced by commit
631b2af2f357. The code analysis reveals that when
`acpi_pci_root_create()` fails, it internally calls
`__acpi_pci_root_release_info()`, which then invokes the `release_info`
callback (`pci_acpi_generic_release_info()`). This callback already
frees:
- `ri->cfg` via `pci_ecam_free(ri->cfg)`
- `ci->ops` (which is `root_ops`) via `kfree(ci->ops)`
- `ri` via `kfree(ri)`
The explicit cleanup added by the reverted commit causes these resources
to be freed a second time, leading to memory corruption.
### 2. **Error Path Issue - High Impact**
The double-free occurs in the error handling path when
`acpi_pci_root_create()` fails. While this is not the common case,
PCI/ACPI initialization failures can occur due to:
- Hardware issues
- Firmware bugs
- Resource allocation failures
- Configuration problems
When such failures occur, the double-free can cause kernel crashes, data
corruption, or security vulnerabilities.
### 3. **Simple and Safe Fix**
The revert is straightforward - it removes the problematic error
handling code that was incorrectly added. The code changes show:
```diff
- if (!root_ops)
- goto free_ri;
+ if (!root_ops) {
+ kfree(ri);
+ return NULL;
+ }
```
This returns to the original, simpler error handling that only frees
what hasn't been passed to `acpi_pci_root_create()` yet.
### 4. **Matches Stable Backport Criteria**
This fix meets the stable kernel criteria:
- **Fixes a real bug**: Double-free causing potential crashes/corruption
- **Small and contained**: Only modifies error paths in one function
- **Already in Linus' tree**: This is a merged commit
- **No new features**: Pure bug fix
- **Low risk**: Reverts to previously known-working code
### 5. **Similar to Successful Backports**
Looking at the reference commits, this is similar to:
- "ARM64: PCI: Fix struct acpi_pci_root_ops allocation failure path"
(Backported: YES) - Also fixed memory management in error paths
- "Revert 'PCI: acpiphp: Reassign resources on bridge if necessary'"
(Backported: YES) - Also a revert fixing problematic behavior
The commit message explicitly states "We plan to resolve the issue using
a more appropriate fix," indicating the maintainers recognize there may
be an underlying issue to address, but the immediate concern is fixing
the double-free bug, making this suitable for stable backporting.
drivers/pci/pci-acpi.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index b78e0e4173244..af370628e5839 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1676,19 +1676,24 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
return NULL;
root_ops = kzalloc(sizeof(*root_ops), GFP_KERNEL);
- if (!root_ops)
- goto free_ri;
+ if (!root_ops) {
+ kfree(ri);
+ return NULL;
+ }
ri->cfg = pci_acpi_setup_ecam_mapping(root);
- if (!ri->cfg)
- goto free_root_ops;
+ if (!ri->cfg) {
+ kfree(ri);
+ kfree(root_ops);
+ return NULL;
+ }
root_ops->release_info = pci_acpi_generic_release_info;
root_ops->prepare_resources = pci_acpi_root_prepare_resources;
root_ops->pci_ops = (struct pci_ops *)&ri->cfg->ops->pci_ops;
bus = acpi_pci_root_create(root, root_ops, &ri->common, ri->cfg);
if (!bus)
- goto free_cfg;
+ return NULL;
/* If we must preserve the resource configuration, claim now */
host = pci_find_host_bridge(bus);
@@ -1705,14 +1710,6 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root)
pcie_bus_configure_settings(child);
return bus;
-
-free_cfg:
- pci_ecam_free(ri->cfg);
-free_root_ops:
- kfree(root_ops);
-free_ri:
- kfree(ri);
- return NULL;
}
void pcibios_add_bus(struct pci_bus *bus)
--
2.39.5
parent reply other threads:[~2025-06-30 20:58 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <20250630204429.1357695-1-sashal@kernel.org>]
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=20250630204429.1357695-6-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=bhelgaas@google.com \
--cc=dan.carpenter@linaro.org \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=patches@lists.linux.dev \
--cc=qiaozhe@iscas.ac.cn \
--cc=rafael.j.wysocki@intel.com \
--cc=rafael@kernel.org \
--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