linux-kernel - Re: [PATCH -next] mm/nommu: fix error handling in split

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20220824144703.gwr45mmdk6xl73my@revolver>
Date:   Wed, 24 Aug 2022 14:47:11 +0000
From:   Liam Howlett <liam.howlett@...cle.com>
To:     Yang Yingliang <yangyingliang@...wei.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "willy@...radead.org" <willy@...radead.org>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "vbabka@...e.cz" <vbabka@...e.cz>
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 <Liam.Howlett@...cle.com>

* Yang Yingliang <yangyingliang@...wei.com> [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 <yangyingliang@...wei.com>
> ---
>  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
>