Le 12/03/2024 à 23:28, Rick Edgecombe a écrit :
> When memory is being placed, mmap() will take care to respect the guard
> gaps of certain types of memory (VM_SHADOWSTACK, VM_GROWSUP and
> VM_GROWSDOWN). In order to ensure guard gaps between mappings, mmap()
> needs to consider two things:
> 1. That the new mapping isn’t placed in an any existing mappings guard
> gaps.
> 2. That the new mapping isn’t placed such that any existing mappings
> are not in *its* guard gaps.
>
> The long standing behavior of mmap() is to ensure 1, but not take any care
> around 2. So for example, if there is a PAGE_SIZE free area, and a
> mmap() with a PAGE_SIZE size, and a type that has a guard gap is being
> placed, mmap() may place the shadow stack in the PAGE_SIZE free area. Then
> the mapping that is supposed to have a guard gap will not have a gap to
> the adjacent VMA.
>
> In order to take the start gap into account, the maple tree search needs
> to know the size of start gap the new mapping will need. The call chain
> from do_mmap() to the actual maple tree search looks like this:
>
> do_mmap(size, vm_flags, map_flags, ..)
> mm/mmap.c:get_unmapped_area(size, map_flags, ...)
> arch_get_unmapped_area(size, map_flags, ...)
> vm_unmapped_area(struct vm_unmapped_area_info)
>
> One option would be to add another MAP_ flag to mean a one page start gap
> (as is for shadow stack), but this consumes a flag unnecessarily. Another
> option could be to simply increase the size passed in do_mmap() by the
> start gap size, and adjust after the fact, but this will interfere with
> the alignment requirements passed in struct vm_unmapped_area_info, and
> unknown to mmap.c. Instead, introduce variants of
> arch_get_unmapped_area/_topdown() that take vm_flags. In future changes,
> these variants can be used in mmap.c:get_unmapped_area() to allow the
> vm_flags to be passed through to vm_unmapped_area(), while preserving the
> normal arch_get_unmapped_area/_topdown() for the existing callers.
>
> Signed-off-by: Rick Edgecombe <rick.p.edgecombe@xxxxxxxxx>
> ---
> include/linux/sched/mm.h | 17 +++++++++++++++++
> mm/mmap.c | 28 ++++++++++++++++++++++++++++
> 2 files changed, 45 insertions(+)
>
> diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
> index cde946e926d8..7b44441865c5 100644
> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -191,6 +191,23 @@ unsigned long mm_get_unmapped_area(struct mm_struct *mm, struct file *filp,
> unsigned long addr, unsigned long len,
> unsigned long pgoff, unsigned long flags);
>
> +extern unsigned long
> +arch_get_unmapped_area_vmflags(struct file *filp, unsigned long addr,
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t vm_flags);
> +extern unsigned long
> +arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr,
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t);
Please, no new "extern", that's pointless for function prototypes and
outdate coding style. Checkpatch --strict is likely protesting about it.
> +
> +unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm,
> + struct file *filp,
> + unsigned long addr,
> + unsigned long len,
> + unsigned long pgoff,
> + unsigned long flags,
> + vm_flags_t vm_flags);
> +
> unsigned long
> generic_get_unmapped_area(struct file *filp, unsigned long addr,
> unsigned long len, unsigned long pgoff,
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 39e9a3ae3ca5..e23ce8ca24c9 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1810,6 +1810,34 @@ arch_get_unmapped_area_topdown(struct file *filp, unsigned long addr,
> }
> #endif
>
> +#ifndef HAVE_ARCH_UNMAPPED_AREA_VMFLAGS
> +extern unsigned long
> +arch_get_unmapped_area_vmflags(struct file *filp, unsigned long addr, unsigned long len,
> + unsigned long pgoff, unsigned long flags, vm_flags_t vm_flags)
> +{
> + return arch_get_unmapped_area(filp, addr, len, pgoff, flags);
> +}
Same, what is the point with that "extern" keyword in function definition ?
> +
> +extern unsigned long
> +arch_get_unmapped_area_topdown_vmflags(struct file *filp, unsigned long addr,
> + unsigned long len, unsigned long pgoff,
> + unsigned long flags, vm_flags_t vm_flags)
> +{
> + return arch_get_unmapped_area_topdown(filp, addr, len, pgoff, flags);
> +}
> +#endif
> +
> +unsigned long mm_get_unmapped_area_vmflags(struct mm_struct *mm, struct file *filp,
> + unsigned long addr, unsigned long len,
> + unsigned long pgoff, unsigned long flags,
> + vm_flags_t vm_flags)
> +{
> + if (test_bit(MMF_TOPDOWN, &mm->flags))
> + return arch_get_unmapped_area_topdown_vmflags(filp, addr, len, pgoff,
> + flags, vm_flags);
> + return arch_get_unmapped_area_vmflags(filp, addr, len, pgoff, flags, vm_flags);
> +}
> +
> unsigned long
> get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
> unsigned long pgoff, unsigned long flags)
