diff mbox series

[08/10] mm: move debug checks from __vunmap to remove_vm_area

Message ID 20230121071051.1143058-9-hch@lst.de (mailing list archive)
State New
Headers show
Series [01/10] mm: reject vmap with VM_FLUSH_RESET_PERMS | expand

Commit Message

Christoph Hellwig Jan. 21, 2023, 7:10 a.m. UTC
All these checks apply to the free_vm_area interface as well, so move
them to the common routine.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 mm/vmalloc.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

David Hildenbrand Jan. 23, 2023, 10:43 a.m. UTC | #1
On 21.01.23 08:10, Christoph Hellwig wrote:
> All these checks apply to the free_vm_area interface as well, so move
> them to the common routine.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>   mm/vmalloc.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 97156eab6fe581..5b432508319a4f 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -2588,11 +2588,20 @@ struct vm_struct *remove_vm_area(const void *addr)
>   
>   	might_sleep();
>   
> +	if (WARN(!PAGE_ALIGNED(addr), "Trying to vfree() bad address (%p)\n",
> +			addr))
> +		return NULL;

While at it, might want to use WARN_ONCE() instead.

Reviewed-by: David Hildenbrand <david@redhat.com>
Christoph Hellwig Jan. 23, 2023, 2:50 p.m. UTC | #2
On Mon, Jan 23, 2023 at 11:43:31AM +0100, David Hildenbrand wrote:
>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>> index 97156eab6fe581..5b432508319a4f 100644
>> --- a/mm/vmalloc.c
>> +++ b/mm/vmalloc.c
>> @@ -2588,11 +2588,20 @@ struct vm_struct *remove_vm_area(const void *addr)
>>     	might_sleep();
>>   +	if (WARN(!PAGE_ALIGNED(addr), "Trying to vfree() bad address (%p)\n",
>> +			addr))
>> +		return NULL;
>
> While at it, might want to use WARN_ONCE() instead.

One thing at a time.  But yes, this makes sense and could be an
incremental patch.
David Hildenbrand Jan. 23, 2023, 2:52 p.m. UTC | #3
On 23.01.23 15:50, Christoph Hellwig wrote:
> On Mon, Jan 23, 2023 at 11:43:31AM +0100, David Hildenbrand wrote:
>>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
>>> index 97156eab6fe581..5b432508319a4f 100644
>>> --- a/mm/vmalloc.c
>>> +++ b/mm/vmalloc.c
>>> @@ -2588,11 +2588,20 @@ struct vm_struct *remove_vm_area(const void *addr)
>>>      	might_sleep();
>>>    +	if (WARN(!PAGE_ALIGNED(addr), "Trying to vfree() bad address (%p)\n",
>>> +			addr))
>>> +		return NULL;
>>
>> While at it, might want to use WARN_ONCE() instead.
> 
> One thing at a time.  But yes, this makes sense and could be an
> incremental patch.

Sure, there are some more !ONCE WARN calls hiding in that file.
diff mbox series

Patch

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 97156eab6fe581..5b432508319a4f 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -2588,11 +2588,20 @@  struct vm_struct *remove_vm_area(const void *addr)
 
 	might_sleep();
 
+	if (WARN(!PAGE_ALIGNED(addr), "Trying to vfree() bad address (%p)\n",
+			addr))
+		return NULL;
+
 	va = find_unlink_vmap_area((unsigned long)addr);
 	if (!va || !va->vm)
 		return NULL;
 	vm = va->vm;
+
+	debug_check_no_locks_freed(vm->addr, get_vm_area_size(vm));
+	debug_check_no_obj_freed(vm->addr, get_vm_area_size(vm));
 	kasan_free_module_shadow(vm);
+	kasan_poison_vmalloc(vm->addr, get_vm_area_size(vm));
+
 	free_unmap_vmap_area(va);
 	return vm;
 }
@@ -2664,10 +2673,6 @@  static void __vunmap(const void *addr, int deallocate_pages)
 	if (!addr)
 		return;
 
-	if (WARN(!PAGE_ALIGNED(addr), "Trying to vfree() bad address (%p)\n",
-			addr))
-		return;
-
 	area = remove_vm_area(addr);
 	if (unlikely(!area)) {
 		WARN(1, KERN_ERR "Trying to vfree() nonexistent vm area (%p)\n",
@@ -2675,11 +2680,6 @@  static void __vunmap(const void *addr, int deallocate_pages)
 		return;
 	}
 
-	debug_check_no_locks_freed(area->addr, get_vm_area_size(area));
-	debug_check_no_obj_freed(area->addr, get_vm_area_size(area));
-
-	kasan_poison_vmalloc(area->addr, get_vm_area_size(area));
-
 	vm_remove_mappings(area, deallocate_pages);
 
 	if (deallocate_pages) {