Re: [PATCH v2 4/5] powerpc: drop redundant unmaps of failed vmap mappings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] powerpc/pci: leak of vm_struct in ioremap_phb() when ioremap_page_range() fails
--

commit 293efb97e53f2ab49d30288da6e26fcbf75e032f
Author: Hao Ge <hao.ge@linux.dev>

powerpc: drop redundant unmaps of failed vmap mappings

This commit removes vunmap_range() calls from the error paths of
remap_isa_base() and ioremap_phb(), since vmap_page_range() and
ioremap_page_range() already clean up their installed PTEs upon failure.

> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index e27342ef128b8..f74269636d5ec 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -139,10 +139,8 @@ void __iomem *ioremap_phb(phys_addr_t paddr, unsigned long size)
>  
>  	addr = (unsigned long)area->addr;
>  	if (ioremap_page_range(addr, addr + size, paddr,
> -			pgprot_noncached(PAGE_KERNEL))) {
> -		vunmap_range(addr, addr + size);
> +			pgprot_noncached(PAGE_KERNEL)))
>  		return NULL;
> -	}
>  
>  	return (void __iomem *)addr;
>  }

[Severity: Medium]
Does ioremap_phb() still leak area and its reserved virtual address range in
[PHB_IO_BASE, PHB_IO_END) when ioremap_page_range() fails?

Prior to calling ioremap_page_range(), ioremap_phb() allocates a virtual
memory area:

arch/powerpc/kernel/pci_64.c:ioremap_phb() {
    ...
	area = __get_vm_area_caller(size, VM_IOREMAP, PHB_IO_BASE, PHB_IO_END,
				    __builtin_return_address(0));
	if (!area)
		return NULL;
    ...
}

When ioremap_page_range() fails (for example, -ENOMEM when allocating page
tables in vmap_range_noflush()), vmap_page_range() rolls back the PTEs via
__vunmap_range_noflush(), but it does not free or unlink the struct vm_struct
and struct vmap_area allocated by __get_vm_area_caller().

Should this error path call free_vm_area(area) before returning NULL, matching
generic_ioremap_prot() in mm/ioremap.c?

mm/ioremap.c:generic_ioremap_prot() {
    ...
	if (ioremap_page_range(vaddr, vaddr + size, phys_addr, prot)) {
		free_vm_area(area);
		return NULL;
	}
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019054531.87652-1-hao.ge@linux.dev?part=4