linux-kernel - Re: [PATCH 1/2] mm/mmap: remove unnecessary vp->vma check in vma

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 1 Mar 2023 09:10:03 -0500
From:   "Liam R. Howlett" <Liam.Howlett@...cle.com>
To:     Suren Baghdasaryan <surenb@...gle.com>
Cc:     akpm@...ux-foundation.org, sfr@...b.auug.org.au, error27@...il.com,
        willy@...radead.org, david@...hat.com, jgg@...pe.ca,
        yuzhao@...gle.com, dhowells@...hat.com, hughd@...gle.com,
        mathieu.desnoyers@...icios.com, pasha.tatashin@...een.com,
        laurent.dufour@...ibm.com, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, kernel test robot <lkp@...el.com>
Subject: Re: [PATCH 1/2] mm/mmap: remove unnecessary vp->vma check in
 vma_prepare

* Suren Baghdasaryan <surenb@...gle.com> [230228 21:27]:
> vp->vma in vma_prepare() is always non-NULL, therefore checking it is
> not necessary. Remove the extra check.
> 
> Fixes: e8f071350ea5 ("mm/mmap: write-lock VMAs in vma_prepare before modifying them")
> Reported-by: kernel test robot <lkp@...el.com>
> Reported-by: Dan Carpenter <error27@...il.com>
> Link: https://lore.kernel.org/r/202302281802.J93Nma7q-lkp@intel.com/
> Signed-off-by: Suren Baghdasaryan <surenb@...gle.com>
> ---
> Fix cleanly apply over mm-unstable, SHA in "Fixes" is from that tree.
> 
>  mm/mmap.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 0cd3714c2182..0759d53b470c 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -505,8 +505,7 @@ static inline void init_vma_prep(struct vma_prepare *vp,
>   */
>  static inline void vma_prepare(struct vma_prepare *vp)
>  {
> -	if (vp->vma)
> -		vma_start_write(vp->vma);
> +	vma_start_write(vp->vma);

Would a WARN_ON_ONCE() be worth it?  Maybe not since it will be detected
rather quickly once we dereference it below, but it might make it more
clear as to what happened?

I'm happy either way.

Reviewed-by: Liam R. Howlett <Liam.Howlett@...cle.com>

>  	if (vp->adj_next)
>  		vma_start_write(vp->adj_next);
>  	/* vp->insert is always a newly created VMA, no need for locking */
> -- 
> 2.39.2.722.g9855ee24e9-goog
>