2022-08-24 04:50:02

by Yang Yingliang

Subject: [PATCH -next] mm/nommu: fix error handling in split_vma()

The memory allocated before calling mas_preallocate() is leaked if it fails.
'mas' won't be modify until calling mas_preallocate(), so move it up and
add error label for free the memory.

Fixes: 8aff7dbeaeb1 ("nommu: remove uses of VMA linked list")
Signed-off-by: Yang Yingliang <[email protected]>
---
mm/nommu.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/mm/nommu.c b/mm/nommu.c
index 6c611a689ec0..214c70e1d059 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1355,9 +1355,13 @@ int split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
return -ENOMEM;

new = vm_area_dup(vma);
- if (!new) {
- kmem_cache_free(vm_region_jar, region);
- return -ENOMEM;
+ if (!new)
+ goto err_vma_dup;
+
+ if (mas_preallocate(&mas, vma, GFP_KERNEL)) {
+ pr_warn("Allocation of vma tree for process %d failed\n",
+ current->pid);
+ goto err_mas_preallocate;
}

/* most fields are the same, copy all, and then fixup */
@@ -1388,11 +1392,6 @@ int split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
add_nommu_region(vma->vm_region);
add_nommu_region(new->vm_region);
up_write(&nommu_region_sem);
- if (mas_preallocate(&mas, vma, GFP_KERNEL)) {
- pr_warn("Allocation of vma tree for process %d failed\n",
- current->pid);
- return -ENOMEM;
- }

setup_vma_to_mm(vma, mm);
setup_vma_to_mm(new, mm);
@@ -1400,6 +1399,12 @@ int split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
mas_store(&mas, vma);
vma_mas_store(new, &mas);
return 0;
+
+err_mas_preallocate:
+ vm_area_free(new);
+err_vma_dup:
+ kmem_cache_free(vm_region_jar, region);
+ return -ENOMEM;
}

/*
--
2.25.1

2022-08-24 14:59:03

by Liam R. Howlett

Subject: Re: [PATCH -next] mm/nommu: fix error handling in split_vma()

Thanks. This is a valid issue and a good fix.

Reviewed-by: Liam R. Howlett <[email protected]>

* Yang Yingliang <[email protected]> [220824 00:16]:
> The memory allocated before calling mas_preallocate() is leaked if it fails.
> 'mas' won't be modify until calling mas_preallocate(), so move it up and
> add error label for free the memory.
>
> Fixes: 8aff7dbeaeb1 ("nommu: remove uses of VMA linked list")
> Signed-off-by: Yang Yingliang <[email protected]>
> ---
> mm/nommu.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 6c611a689ec0..214c70e1d059 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -1355,9 +1355,13 @@ int split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
> return -ENOMEM;
>
> new = vm_area_dup(vma);
> - if (!new) {
> - kmem_cache_free(vm_region_jar, region);
> - return -ENOMEM;
> + if (!new)
> + goto err_vma_dup;
> +
> + if (mas_preallocate(&mas, vma, GFP_KERNEL)) {
> + pr_warn("Allocation of vma tree for process %d failed\n",
> + current->pid);
> + goto err_mas_preallocate;
> }
>
> /* most fields are the same, copy all, and then fixup */
> @@ -1388,11 +1392,6 @@ int split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
> add_nommu_region(vma->vm_region);
> add_nommu_region(new->vm_region);
> up_write(&nommu_region_sem);
> - if (mas_preallocate(&mas, vma, GFP_KERNEL)) {
> - pr_warn("Allocation of vma tree for process %d failed\n",
> - current->pid);
> - return -ENOMEM;
> - }
>
> setup_vma_to_mm(vma, mm);
> setup_vma_to_mm(new, mm);
> @@ -1400,6 +1399,12 @@ int split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
> mas_store(&mas, vma);
> vma_mas_store(new, &mas);
> return 0;
> +
> +err_mas_preallocate:
> + vm_area_free(new);
> +err_vma_dup:
> + kmem_cache_free(vm_region_jar, region);
> + return -ENOMEM;
> }
>
> /*
> --
> 2.25.1
>