[PATCH] x86/PCI: Fix use after free in pci_acpi_root_prepare_resources()

[PATCH] x86/PCI: Fix use after free in pci_acpi_root_prepare_resources()

[Dan Carpenter]

The resource_list_destroy_entry() function frees "entry", so move the
dereferences before the free.

Fixes: 62fabd56faaf ("x86/PCI: Disable exclusion of E820 reserved addresses in some cases")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 arch/x86/pci/acpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index e4617df661a9..fa89ffba2e51 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -357,14 +357,14 @@ static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci)
 	status = acpi_pci_probe_root_resources(ci);
 	if (pci_use_crs) {
 		resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
-			if (resource_is_pcicfg_ioport(entry->res))
-				resource_list_destroy_entry(entry);
 			if (resource_is_efi_mmio_region(entry->res)) {
 				dev_info(&device->dev,
 					"host bridge window %pR is marked by EFI as MMIO\n",
 					entry->res);
 				pci_use_e820 = false;
 			}
+			if (resource_is_pcicfg_ioport(entry->res))
+				resource_list_destroy_entry(entry);
 		}
 		return status;
 	}
-- 
2.20.1

Re: [PATCH] x86/PCI: Fix use after free in pci_acpi_root_prepare_resources()

[Hans de Goede]

Hi,

On 3/1/22 09:10, Dan Carpenter wrote:
> The resource_list_destroy_entry() function frees "entry", so move the
> dereferences before the free.
> 
> Fixes: 62fabd56faaf ("x86/PCI: Disable exclusion of E820 reserved addresses in some cases")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  arch/x86/pci/acpi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index e4617df661a9..fa89ffba2e51 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -357,14 +357,14 @@ static int pci_acpi_root_prepare_resources(struct acpi_pci_root_info *ci)
>  	status = acpi_pci_probe_root_resources(ci);
>  	if (pci_use_crs) {
>  		resource_list_for_each_entry_safe(entry, tmp, &ci->resources) {
> -			if (resource_is_pcicfg_ioport(entry->res))
> -				resource_list_destroy_entry(entry);
>  			if (resource_is_efi_mmio_region(entry->res)) {
>  				dev_info(&device->dev,
>  					"host bridge window %pR is marked by EFI as MMIO\n",
>  					entry->res);
>  				pci_use_e820 = false;
>  			}
> +			if (resource_is_pcicfg_ioport(entry->res))
> +				resource_list_destroy_entry(entry);
>  		}
>  		return status;
>  	}


Thank you, I actually send a new version of that commit to replace the original
one also addressing some other issues:

https://lore.kernel.org/linux-acpi/20220228105259.230903-1-hdegoede@redhat.com/

I expect Rafael to pick up the new version, resolving this. But maybe
Rafael prefers a follow up commit instead ?

Regards,

Hans