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

[John Hubbard <jhubbard@nvidia.com>]

On 8/7/24 11:12 PM, Alistair Popple wrote:
> 
> 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.
>

I would add that this is really a serious bug. Alistair and the team
here were just getting ready to post a similar fix, after quite some
effort to locate the root cause.


>> 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;
>>   
>>   	/*
> 

I'm also Cc'ing Jason Gunthorpe, as this will be of interest over in RDMA.

Acked-by: John Hubbard <jhubbard@nvidia.com>


thanks,
-- 
John Hubbard
NVIDIA