[PATCH mm-unstable 0/2] mm/mmap: fix crashes in dup_mmap() error path

[PATCH mm-unstable 0/2] mm/mmap: fix crashes in dup_mmap() error path

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

This series fixes two potential kernel panics in the dup_mmap() error
path triggered during fork failures:
Fix Use-After-Free: Moves vma_iter_free() to the end of the cleanup
block to ensure the iterator remains valid during rollback.
Fix NULL Dereference: Adds a check for vma_next() results to prevent
crashing when the maple tree is empty.

Hui Zhu (2):
  mm/mmap: fix Use-After-Free of vma_iterator in dup_mmap() error path
  mm/mmap: fix NULL pointer dereference in dup_mmap() error handling

 mm/mmap.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

-- 
2.43.0

[PATCH mm-unstable 1/2] mm/mmap: fix Use-After-Free of vma_iterator in dup_mmap() error path

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

When dup_mmap() fails during the process of duplicating VMAs,
it jumps to the 'loop_out' label to clean up resources.
The current implementation calls vma_iter_free(&vmi) at the
beginning of this cleanup path.

The error handling logic still needs to use the 'vmi' to traverse
and tear down the partially initialized maple tree for the new mm.
Since vma_iter_free() calls mas_destroy(), this results in a
Use-After-Free (UAF).

This patch fixes the UAF by moving the vma_iter_free() call to the
end of the cleanup block, ensuring the iterator remains valid
throughout the entire rollback process.

Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 mm/mmap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 843160946aa5..498c88a54a36 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1848,8 +1848,8 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
 	/* a new mm has just been created */
 	retval = arch_dup_mmap(oldmm, mm);
 loop_out:
-	vma_iter_free(&vmi);
 	if (!retval) {
+		vma_iter_free(&vmi);
 		mt_set_in_rcu(vmi.mas.tree);
 		ksm_fork(mm, oldmm);
 		khugepaged_fork(mm, oldmm);
@@ -1893,6 +1893,7 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
 			charge = tear_down_vmas(mm, &vmi, tmp, end);
 			vm_unacct_memory(charge);
 		}
+		vma_iter_free(&vmi);
 		__mt_destroy(&mm->mm_mt);
 		/*
 		 * The mm_struct is going to exit, but the locks will be dropped
-- 
2.43.0

Re: [PATCH mm-unstable 1/2] mm/mmap: fix Use-After-Free of vma_iterator in dup_mmap() error path

[Lorenzo Stoakes (Oracle)]

On Wed, Mar 04, 2026 at 03:00:56PM +0800, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@kylinos.cn>
>
> When dup_mmap() fails during the process of duplicating VMAs,
> it jumps to the 'loop_out' label to clean up resources.
> The current implementation calls vma_iter_free(&vmi) at the
> beginning of this cleanup path.
>
> The error handling logic still needs to use the 'vmi' to traverse
> and tear down the partially initialized maple tree for the new mm.
> Since vma_iter_free() calls mas_destroy(), this results in a
> Use-After-Free (UAF).

No it wouldn't be a UAF, vma_iter_free() calls mas_destroy() which simply
frees any allocations associated with the iterator, so it's fine.

The use of the VMA iterator in the else loop doesn't allocate anything as it's
not setting anything so it's fine.

>
> This patch fixes the UAF by moving the vma_iter_free() call to the
> end of the cleanup block, ensuring the iterator remains valid
> throughout the entire rollback process.
>
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>

So yeah this patch isn't necessary and suggests a UAF that doesn't exist.

Let's keep the code as it is.

Thanks, Lorenzo

> ---
>  mm/mmap.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 843160946aa5..498c88a54a36 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1848,8 +1848,8 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
>  	/* a new mm has just been created */
>  	retval = arch_dup_mmap(oldmm, mm);
>  loop_out:
> -	vma_iter_free(&vmi);
>  	if (!retval) {
> +		vma_iter_free(&vmi);
>  		mt_set_in_rcu(vmi.mas.tree);
>  		ksm_fork(mm, oldmm);
>  		khugepaged_fork(mm, oldmm);
> @@ -1893,6 +1893,7 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
>  			charge = tear_down_vmas(mm, &vmi, tmp, end);
>  			vm_unacct_memory(charge);
>  		}
> +		vma_iter_free(&vmi);
>  		__mt_destroy(&mm->mm_mt);
>  		/*
>  		 * The mm_struct is going to exit, but the locks will be dropped
> --
> 2.43.0
>

[PATCH mm-unstable 2/2] mm/mmap: fix NULL pointer dereference in dup_mmap() error handling

[Hui Zhu]

From: Hui Zhu <zhuhui@kylinos.cn>

If dup_mmap() fails very early in its execution, it's possible that
no VMAs have been inserted into the new mm's maple tree. When
vma_next() is called in the cleanup block to retrieve the first
VMA ('tmp'), it may return NULL.

The UNMAP_STATE macro and the subsequent call to tear_down_vmas()
do not perform a NULL check on 'tmp' and directly attempt to
access its fields (such as tmp->vm_end). This results in a
NULL pointer dereference and a kernel panic.

This patch adds an explicit NULL check for 'tmp' before proceeding
with the unmap and tear down logic in the failure path of dup_mmap().

Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>
---
 mm/mmap.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 498c88a54a36..ca5645a2e456 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1879,19 +1879,24 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
 		if (end) {
 			vma_iter_set(&vmi, 0);
 			tmp = vma_next(&vmi);
-			UNMAP_STATE(unmap, &vmi, /* first = */ tmp,
-				    /* vma_start = */ 0, /* vma_end = */ end,
-				    /* prev = */ NULL, /* next = */ NULL);
-
-			/*
-			 * Don't iterate over vmas beyond the failure point for
-			 * both unmap_vma() and free_pgtables().
-			 */
-			unmap.tree_end = end;
-			flush_cache_mm(mm);
-			unmap_region(&unmap);
-			charge = tear_down_vmas(mm, &vmi, tmp, end);
-			vm_unacct_memory(charge);
+			if (tmp) {
+				UNMAP_STATE(unmap, &vmi,
+					    /* first = */ tmp,
+					    /* vma_start = */ 0,
+					    /* vma_end = */ end,
+					    /* prev = */ NULL,
+					    /* next = */ NULL);
+
+				/*
+				 * Don't iterate over vmas beyond the failure point for
+				 * both unmap_vma() and free_pgtables().
+				 */
+				unmap.tree_end = end;
+				flush_cache_mm(mm);
+				unmap_region(&unmap);
+				charge = tear_down_vmas(mm, &vmi, tmp, end);
+				vm_unacct_memory(charge);
+			}
 		}
 		vma_iter_free(&vmi);
 		__mt_destroy(&mm->mm_mt);
-- 
2.43.0

Re: [PATCH mm-unstable 2/2] mm/mmap: fix NULL pointer dereference in dup_mmap() error handling

[Lorenzo Stoakes (Oracle)]

On Wed, Mar 04, 2026 at 03:00:57PM +0800, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@kylinos.cn>
>
> If dup_mmap() fails very early in its execution, it's possible that
> no VMAs have been inserted into the new mm's maple tree. When
> vma_next() is called in the cleanup block to retrieve the first
> VMA ('tmp'), it may return NULL.

Have you observed that or are you just guessing?

I mean given the below, you are just guessing :)

>
> The UNMAP_STATE macro and the subsequent call to tear_down_vmas()
> do not perform a NULL check on 'tmp' and directly attempt to
> access its fields (such as tmp->vm_end). This results in a
> NULL pointer dereference and a kernel panic.

Please don't suggest that something might happen if you can't demonstrate that
it might happen.

So there's this code just above that logic:

		/*
		 * The entire maple tree has already been duplicated, but
		 * replacing the vmas failed at mpnt (which could be NULL if
		 * all were allocated but the last vma was not fully set up).
		 * Use the start address of the failure point to clean up the
		 * partially initialized tree.
		 */
		if (!mm->map_count) {
			/* zero vmas were written to the new tree. */
			end = 0;
		} else if (mpnt) {
			/* partial tree failure */
			end = mpnt->vm_start;
		} else {
			/* All vmas were written to the new tree */
			end = ULONG_MAX;
		}

mm->map_count == number of VMAs, so we explicit check for this and the if (end)
{ ... } already covers this.

>
> This patch adds an explicit NULL check for 'tmp' before proceeding
> with the unmap and tear down logic in the failure path of dup_mmap().
>
> Signed-off-by: Hui Zhu <zhuhui@kylinos.cn>

As a result of the above, this patch is completely unnecessary, so let's not
please.

Do try to prove to yourself that a suggested thing-that-might-happen might
actually happen in practice :)

The kernel does sometimes skip NULL checks when we set the code up such that a
pointer cannot be NULL, and this is one of those cases.

So it's important not to assume that a pointer _in theory_ being NULL means that
we need a NULL check somewhere, first check to see if _in practice_ a pointer
could be NULL first.

Thanks, Lorenzo

> ---
>  mm/mmap.c | 31 ++++++++++++++++++-------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 498c88a54a36..ca5645a2e456 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1879,19 +1879,24 @@ __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)
>  		if (end) {
>  			vma_iter_set(&vmi, 0);
>  			tmp = vma_next(&vmi);
> -			UNMAP_STATE(unmap, &vmi, /* first = */ tmp,
> -				    /* vma_start = */ 0, /* vma_end = */ end,
> -				    /* prev = */ NULL, /* next = */ NULL);
> -
> -			/*
> -			 * Don't iterate over vmas beyond the failure point for
> -			 * both unmap_vma() and free_pgtables().
> -			 */
> -			unmap.tree_end = end;
> -			flush_cache_mm(mm);
> -			unmap_region(&unmap);
> -			charge = tear_down_vmas(mm, &vmi, tmp, end);
> -			vm_unacct_memory(charge);
> +			if (tmp) {

This kind of hugely indented code is horrible. I know this function isn't great
but let's not make it worse.

> +				UNMAP_STATE(unmap, &vmi,
> +					    /* first = */ tmp,
> +					    /* vma_start = */ 0,
> +					    /* vma_end = */ end,
> +					    /* prev = */ NULL,
> +					    /* next = */ NULL);
> +
> +				/*
> +				 * Don't iterate over vmas beyond the failure point for
> +				 * both unmap_vma() and free_pgtables().
> +				 */
> +				unmap.tree_end = end;
> +				flush_cache_mm(mm);
> +				unmap_region(&unmap);
> +				charge = tear_down_vmas(mm, &vmi, tmp, end);
> +				vm_unacct_memory(charge);
> +			}
>  		}
>  		vma_iter_free(&vmi);
>  		__mt_destroy(&mm->mm_mt);
> --
> 2.43.0
>