Re: [PATCH 1/1] x86/ioremap: Use is_vmalloc_addr in iounmap

Alistair Popple
2024-08-08T06:28:19Z

Max Ramanouski <max8rr8@gmail.com> writes:

> On systems that use HMM (most notably amdgpu driver)
> high_memory can jump over VMALLOC_START. That causes
> some iounmap to exit early. This in addition to leaking,
> causes problems with rebinding devices to vfio_pci from
> other drivers with error of conflicting memtypes,
> as they aren't freed in iounmap.

This can also manifest in more subtle ways because the conflict does not
always result in an error. Sometimes for example it can cause ioremap_uc()
to actually return WC mappings if for example the early iounmap exit
leaves a WC mapping behind. This is because is_new_memtype_allowed()
returns true in most cases.

> Replace comparison against high_memory with is_vmalloc_addr to
> fix the issue and make x86 iounmap implementation more similar
> to generic one, it also uses is_vmalloc_addr to validate pointer.

This seems correct to me, but I'm not an expert in the x86 memory
map. However it does fix the issue we observed that lead us to finding
this fix, and it doesn't seem to introduce any other obvious problems so
please add:

Tested-by: Alistair Popple <apopple@nvidia.com>

> Signed-off-by: Max Ramanouski <max8rr8@gmail.com>
> ---
> arch/x86/mm/ioremap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
> index aa7d27932..0b596a1d2 100644
> --- a/arch/x86/mm/ioremap.c
> +++ b/arch/x86/mm/ioremap.c
> @@ -457,7 +457,7 @@ void iounmap(volatile void __iomem *addr)
> {
> struct vm_struct *p, *o;
>
> - if ((void __force *)addr <= high_memory)
> + if (!is_vmalloc_addr((void __force *)addr))
> return;
>
> /*