linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).