linux-kernel - Re: [PATCH] mm/mmap: remove the mm parameter 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]

Message-ID: <20240129150417.3m6jyj3ftdh6ka7x@revolver>
Date: Mon, 29 Jan 2024 10:04:17 -0500
From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
To: Yajun Deng <yajun.deng@...ux.dev>
Cc: akpm@...ux-foundation.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm/mmap: remove the mm parameter in vma_complete()

* Yajun Deng <yajun.deng@...ux.dev> [240129 02:53]:
> There are vma_merge() and do_brk_flags() pass mm to vma_complete(), others
> would pass the vma->vm_mm. The following explains that the mm is the
> vma->vm_mm in vma_merge() and do_brk_flags().
> 
> All vma will point to the same mm struct if the vma_merge() is successful.
> So the mm and the vma->mm are the same.

Absolutely, they must be the same.  I don't think vma_merge() checks
this, but it is true.

> 
> vm_brk_flags() and brk syscall will initialize vmi with current->mm,
> so the vma->vm_mm and the current->mm are the same if vma exists in
> do_brk_flags().
> 
> Remove the mm parameter in vma_complete() and get mm from the vma in vp.

You have added a dereference to the two paths that don't need it to
reduce the argument list from 3 to 2.  It's the same number of lines as
well.  vma_shrink() is only used on process creation, but brk is more
common.  Note that this function is marked as inline.

I'm not sure this change is worth making.

> 
> Signed-off-by: Yajun Deng <yajun.deng@...ux.dev>
> ---
>  mm/mmap.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index e97b9144c61a..9b968d1edf55 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -509,11 +509,11 @@ static inline void vma_prepare(struct vma_prepare *vp)
>   *
>   * @vp: The vma_prepare struct
>   * @vmi: The vma iterator
> - * @mm: The mm_struct
>   */
> -static inline void vma_complete(struct vma_prepare *vp,
> -				struct vma_iterator *vmi, struct mm_struct *mm)
> +static inline void vma_complete(struct vma_prepare *vp, struct vma_iterator *vmi)
>  {
> +	struct mm_struct *mm = vp->vma->vm_mm;
> +
>  	if (vp->file) {
>  		if (vp->adj_next)
>  			vma_interval_tree_insert(vp->adj_next,
> @@ -666,7 +666,7 @@ int vma_expand(struct vma_iterator *vmi, struct vm_area_struct *vma,
>  	vma_set_range(vma, start, end, pgoff);
>  	vma_iter_store(vmi, vma);
>  
> -	vma_complete(&vp, vmi, vma->vm_mm);
> +	vma_complete(&vp, vmi);
>  	return 0;
>  
>  nomem:
> @@ -707,7 +707,7 @@ int vma_shrink(struct vma_iterator *vmi, struct vm_area_struct *vma,
>  
>  	vma_iter_clear(vmi);
>  	vma_set_range(vma, start, end, pgoff);
> -	vma_complete(&vp, vmi, vma->vm_mm);
> +	vma_complete(&vp, vmi);
>  	return 0;
>  }
>  
> @@ -1030,7 +1030,7 @@ static struct vm_area_struct
>  		}
>  	}
>  
> -	vma_complete(&vp, vmi, mm);
> +	vma_complete(&vp, vmi);
>  	khugepaged_enter_vma(res, vm_flags);
>  	return res;
>  
> @@ -2377,7 +2377,7 @@ static int __split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
>  	}
>  
>  	/* vma_complete stores the new vma */
> -	vma_complete(&vp, vmi, vma->vm_mm);
> +	vma_complete(&vp, vmi);
>  
>  	/* Success. */
>  	if (new_below)
> @@ -3145,7 +3145,7 @@ static int do_brk_flags(struct vma_iterator *vmi, struct vm_area_struct *vma,
>  		vm_flags_set(vma, VM_SOFTDIRTY);
>  		vma_iter_store(vmi, vma);
>  
> -		vma_complete(&vp, vmi, mm);
> +		vma_complete(&vp, vmi);
>  		khugepaged_enter_vma(vma, flags);
>  		goto out;
>  	}
> -- 
> 2.25.1
> 
>