| 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
| ||
|
Message-ID: <d8af66d6-4ba7-45f6-877c-e89627506551@csgroup.eu>
Date: Wed, 13 Mar 2024 07:22:38 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Rick Edgecombe <rick.p.edgecombe@...el.com>, "Liam.Howlett@...cle.com"
<Liam.Howlett@...cle.com>, "akpm@...ux-foundation.org"
<akpm@...ux-foundation.org>, "bp@...en8.de" <bp@...en8.de>,
"broonie@...nel.org" <broonie@...nel.org>, "dave.hansen@...ux.intel.com"
<dave.hansen@...ux.intel.com>, "debug@...osinc.com" <debug@...osinc.com>,
"hpa@...or.com" <hpa@...or.com>, "keescook@...omium.org"
<keescook@...omium.org>, "kirill.shutemov@...ux.intel.com"
<kirill.shutemov@...ux.intel.com>, "luto@...nel.org" <luto@...nel.org>,
"mingo@...hat.com" <mingo@...hat.com>, "peterz@...radead.org"
<peterz@...radead.org>, "tglx@...utronix.de" <tglx@...utronix.de>,
"x86@...nel.org" <x86@...nel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mm@...ck.org" <linux-mm@...ck.org>
Subject: Re: [PATCH v3 02/12] mm: Introduce arch_get_unmapped_area_vmflags()
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@...el.com>
> ---
> 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)
Powered by blists - more mailing lists