Re: [PATCH] mm/nommu: Fix NULL mm dereference in __vmalloc_user_flags

Re: [PATCH] mm/nommu: Fix NULL mm dereference in __vmalloc_user_flags

[Matthew Wilcox]

On Tue, Jan 28, 2025 at 02:22:52AM -0500, sooraj wrote:
> Problem:
> The __vmalloc_user_flags() function attempts to acquire mmap_lock
> via current->mm without checking if the mm context exists. This causes
> a NULL pointer dereference when called from kernel threads (where
> current->mm == NULL), such as during filesystem operations.

I'm having a hard time thinking of a case where this could happen.
Could you provide a backtrace showing an example?

> Fix:
> Add a NULL check for current->mm before attempting mmap_lock operations.
> Kernel threads don't have user memory mappings, so VM_USERMAP flag
> setting can be safely skipped in this context.
> 
> Signed-off-by: sooraj <sooraj20636@gmail.com>

Do you sign cheques as 'sooraj'?  You need to use your real, legal name.

> +		struct mm_struct *mm = current->mm;
> +
> +		if(!mm){
> +			goto out;
> +		}

All kinds of whitespace problems here.  It should be:

		if (!mm)
			goto out;

>  		mmap_write_lock(current->mm);
>  		vma = find_vma(current->mm, (unsigned long)ret);

And since you've gone to the trouble of loading current->mm into
a local variable, you should use it throughout the function.

> @@ -160,6 +165,7 @@ static void *__vmalloc_user_flags(unsigned long size, gfp_t flags)
>  		mmap_write_unlock(current->mm);
>  	}
>  
> +	out:

and this should not be indented.

>  	return ret;

[PATCH] mm/nommu: Fix NULL mm dereference in __vmalloc_user_flags

[sooraj]

Problem:
The __vmalloc_user_flags() function attempts to acquire mmap_lock
via current->mm without checking if the mm context exists. This causes
a NULL pointer dereference when called from kernel threads (where
current->mm == NULL), such as during filesystem operations.

Fix:
Add a NULL check for current->mm before attempting mmap_lock operations.
Kernel threads don't have user memory mappings, so VM_USERMAP flag
setting can be safely skipped in this context.

Signed-off-by: sooraj <sooraj20636@gmail.com>
---
 mm/nommu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/mm/nommu.c b/mm/nommu.c
index baa79abdaf03..df13c1c78d5b 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -152,6 +152,11 @@ static void *__vmalloc_user_flags(unsigned long size, gfp_t flags)
 	ret = __vmalloc(size, flags);
 	if (ret) {
 		struct vm_area_struct *vma;
+		struct mm_struct *mm = current->mm;
+
+		if(!mm){
+			goto out;
+		}
 
 		mmap_write_lock(current->mm);
 		vma = find_vma(current->mm, (unsigned long)ret);
@@ -160,6 +165,7 @@ static void *__vmalloc_user_flags(unsigned long size, gfp_t flags)
 		mmap_write_unlock(current->mm);
 	}
 
+	out:
 	return ret;
 }
 
-- 
2.45.2