Re: [PATCH v3 09/23] mm: unexport vm_brk_flags() and eliminate vm_flags parameter

["Vlastimil Babka (SUSE)" <vbabka@kernel.org>]

On 3/18/26 16:50, Lorenzo Stoakes (Oracle) wrote:
> This function is only used by elf_load(), and that is a static function
> that doesn't need an exported symbol to invoke an internal function, so
> un-EXPORT_SYMBOLS() it.
> 
> Also, the vm_flags parameter is unnecessary, as we only ever set VM_EXEC,
> so simply make this parameter a boolean.
> 
> While we're here, clean up the mm.h definitions for the various vm_xxx()
> helpers so we actually specify parameter names and elide the redundant
> extern's.
> 
> Signed-off-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org>

Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  fs/binfmt_elf.c    |  3 +--
>  include/linux/mm.h | 12 ++++++------
>  mm/mmap.c          |  8 ++------
>  3 files changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index fb857faaf0d6..16a56b6b3f6c 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -453,14 +453,13 @@ static unsigned long elf_load(struct file *filep, unsigned long addr,
>  		zero_end = ELF_PAGEALIGN(zero_end);
>  
>  		error = vm_brk_flags(zero_start, zero_end - zero_start,
> -				     prot & PROT_EXEC ? VM_EXEC : 0);
> +				     prot & PROT_EXEC);
>  		if (error)
>  			map_addr = error;
>  	}
>  	return map_addr;
>  }
>  
> -
>  static unsigned long total_mapping_size(const struct elf_phdr *phdr, int nr)
>  {
>  	elf_addr_t min_addr = -1;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0c35423177bf..42d346684678 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4005,12 +4005,12 @@ static inline void mm_populate(unsigned long addr, unsigned long len) {}
>  #endif
>  
>  /* This takes the mm semaphore itself */
> -extern int __must_check vm_brk_flags(unsigned long, unsigned long, unsigned long);
> -extern int vm_munmap(unsigned long, size_t);
> -extern unsigned long __must_check vm_mmap(struct file *, unsigned long,
> -        unsigned long, unsigned long,
> -        unsigned long, unsigned long);
> -extern unsigned long __must_check vm_mmap_shadow_stack(unsigned long addr,
> +int __must_check vm_brk_flags(unsigned long addr, unsigned long request, bool is_exec);
> +int vm_munmap(unsigned long start, size_t len);
> +unsigned long __must_check vm_mmap(struct file *file, unsigned long addr,
> +		unsigned long len, unsigned long prot,
> +		unsigned long flag, unsigned long offset);
> +unsigned long __must_check vm_mmap_shadow_stack(unsigned long addr,
>  		unsigned long len, unsigned long flags);
>  
>  struct vm_unmapped_area_info {
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 79544d893411..2d2b814978bf 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1201,8 +1201,9 @@ SYSCALL_DEFINE5(remap_file_pages, unsigned long, start, unsigned long, size,
>  	return ret;
>  }
>  
> -int vm_brk_flags(unsigned long addr, unsigned long request, vm_flags_t vm_flags)
> +int vm_brk_flags(unsigned long addr, unsigned long request, bool is_exec)
>  {
> +	const vm_flags_t vm_flags = is_exec ? VM_EXEC : 0;
>  	struct mm_struct *mm = current->mm;
>  	struct vm_area_struct *vma = NULL;
>  	unsigned long len;
> @@ -1217,10 +1218,6 @@ int vm_brk_flags(unsigned long addr, unsigned long request, vm_flags_t vm_flags)
>  	if (!len)
>  		return 0;
>  
> -	/* Until we need other flags, refuse anything except VM_EXEC. */
> -	if ((vm_flags & (~VM_EXEC)) != 0)
> -		return -EINVAL;
> -
>  	if (mmap_write_lock_killable(mm))
>  		return -EINTR;
>  
> @@ -1246,7 +1243,6 @@ int vm_brk_flags(unsigned long addr, unsigned long request, vm_flags_t vm_flags)
>  	mmap_write_unlock(mm);
>  	return ret;
>  }
> -EXPORT_SYMBOL(vm_brk_flags);
>  
>  static
>  unsigned long tear_down_vmas(struct mm_struct *mm, struct vma_iterator *vmi,