* [PATCH AUTOSEL 6.15 06/23] Revert "PCI/ACPI: Fix allocated memory release on error in pci_acpi_scan_root()"
[not found] <20250630204429.1357695-1-sashal@kernel.org>
@ 2025-06-30 20:44 ` Sasha Levin
0 siblings, 0 replies; only message in thread
From: Sasha Levin @ 2025-06-30 20:44 UTC (permalink / raw)
To: patches, stable
Cc: Zhe Qiao, Dan Carpenter, Rafael J . Wysocki, Sasha Levin, rafael,
bhelgaas, linux-acpi, linux-pci
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
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2025-06-30 20:58 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250630204429.1357695-1-sashal@kernel.org>
2025-06-30 20:44 ` [PATCH AUTOSEL 6.15 06/23] Revert "PCI/ACPI: Fix allocated memory release on error in pci_acpi_scan_root()" Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).