| Message ID | 20210415221419.31835-30-yu-cheng.yu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Control-flow Enforcement: Shadow Stack | expand |
On Thu, Apr 15, 2021 at 03:14:18PM -0700, Yu-cheng Yu wrote: > When newer VM flags are being created, such as VM_MTE, it becomes necessary > for mmap/mprotect to verify if certain flags are being applied to an > anonymous VMA. > > To solve this, one approach is adding a VM flag to track that MAP_ANONYMOUS > is specified [1], and then using the flag in arch_validate_flags(). > > Another approach is passing vma_is_anonymous() to arch_validate_flags(). > To prepare the introduction of PROT_SHSTK, which creates a shadow stack > mapping and can only be applied to an anonymous VMA, update arch_validate_ > flags() to include anonymous VMA information. I would rather pass down whole vma. Who knows what else arch_validate_flags() would need to know about the VMA tomorrow: arch_validate_flags(vma, newflags); should do the trick.
On Mon, Apr 26, 2021 at 09:40:56AM +0300, Kirill A. Shutemov wrote: > On Thu, Apr 15, 2021 at 03:14:18PM -0700, Yu-cheng Yu wrote: > > When newer VM flags are being created, such as VM_MTE, it becomes necessary > > for mmap/mprotect to verify if certain flags are being applied to an > > anonymous VMA. > > > > To solve this, one approach is adding a VM flag to track that MAP_ANONYMOUS > > is specified [1], and then using the flag in arch_validate_flags(). > > > > Another approach is passing vma_is_anonymous() to arch_validate_flags(). > > To prepare the introduction of PROT_SHSTK, which creates a shadow stack > > mapping and can only be applied to an anonymous VMA, update arch_validate_ > > flags() to include anonymous VMA information. > > I would rather pass down whole vma. Who knows what else > arch_validate_flags() would need to know about the VMA tomorrow: > > arch_validate_flags(vma, newflags); > > should do the trick. A reason why we added a separate VM_MTE_ALLOWED flag was that we wanted MTE on other RAM-based based mappings, not just anonymous pages. See 51b0bff2f703 ("mm: Allow arm64 mmap(PROT_MTE) on RAM-based files"). Anyway, the above change doesn't get in the way.
On 4/26/2021 4:11 AM, Catalin Marinas wrote: > On Mon, Apr 26, 2021 at 09:40:56AM +0300, Kirill A. Shutemov wrote: >> On Thu, Apr 15, 2021 at 03:14:18PM -0700, Yu-cheng Yu wrote: >>> When newer VM flags are being created, such as VM_MTE, it becomes necessary >>> for mmap/mprotect to verify if certain flags are being applied to an >>> anonymous VMA. >>> >>> To solve this, one approach is adding a VM flag to track that MAP_ANONYMOUS >>> is specified [1], and then using the flag in arch_validate_flags(). >>> >>> Another approach is passing vma_is_anonymous() to arch_validate_flags(). >>> To prepare the introduction of PROT_SHSTK, which creates a shadow stack >>> mapping and can only be applied to an anonymous VMA, update arch_validate_ >>> flags() to include anonymous VMA information. >> >> I would rather pass down whole vma. Who knows what else >> arch_validate_flags() would need to know about the VMA tomorrow: >> >> arch_validate_flags(vma, newflags); >> >> should do the trick. > > A reason why we added a separate VM_MTE_ALLOWED flag was that we wanted > MTE on other RAM-based based mappings, not just anonymous pages. See > 51b0bff2f703 ("mm: Allow arm64 mmap(PROT_MTE) on RAM-based files"). > > Anyway, the above change doesn't get in the way. > Thanks a lot for the clarification! Yu-cheng
diff --git a/arch/arm64/include/asm/mman.h b/arch/arm64/include/asm/mman.h index e3e28f7daf62..44add1a09041 100644 --- a/arch/arm64/include/asm/mman.h +++ b/arch/arm64/include/asm/mman.h @@ -74,7 +74,7 @@ static inline bool arch_validate_prot(unsigned long prot, } #define arch_validate_prot(prot, addr) arch_validate_prot(prot, addr) -static inline bool arch_validate_flags(unsigned long vm_flags) +static inline bool arch_validate_flags(unsigned long vm_flags, bool is_anon) { if (!system_supports_mte()) return true; @@ -82,6 +82,6 @@ static inline bool arch_validate_flags(unsigned long vm_flags) /* only allow VM_MTE if VM_MTE_ALLOWED has been set previously */ return !(vm_flags & VM_MTE) || (vm_flags & VM_MTE_ALLOWED); } -#define arch_validate_flags(vm_flags) arch_validate_flags(vm_flags) +#define arch_validate_flags(vm_flags, is_anon) arch_validate_flags(vm_flags, is_anon) #endif /* ! __ASM_MMAN_H__ */ diff --git a/arch/sparc/include/asm/mman.h b/arch/sparc/include/asm/mman.h index 274217e7ed70..4a897c8a3f1a 100644 --- a/arch/sparc/include/asm/mman.h +++ b/arch/sparc/include/asm/mman.h @@ -60,11 +60,11 @@ static inline int sparc_validate_prot(unsigned long prot, unsigned long addr) return 1; } -#define arch_validate_flags(vm_flags) arch_validate_flags(vm_flags) +#define arch_validate_flags(vm_flags, is_anon) arch_validate_flags(vm_flags, is_anon) /* arch_validate_flags() - Ensure combination of flags is valid for a * VMA. */ -static inline bool arch_validate_flags(unsigned long vm_flags) +static inline bool arch_validate_flags(unsigned long vm_flags, bool is_anon) { /* If ADI is being enabled on this VMA, check for ADI * capability on the platform and ensure VMA is suitable diff --git a/include/linux/mman.h b/include/linux/mman.h index 629cefc4ecba..a22ed4495d13 100644 --- a/include/linux/mman.h +++ b/include/linux/mman.h @@ -114,7 +114,7 @@ static inline bool arch_validate_prot(unsigned long prot, unsigned long addr) * * Returns true if the VM_* flags are valid. */ -static inline bool arch_validate_flags(unsigned long flags) +static inline bool arch_validate_flags(unsigned long flags, bool is_anonymous) { return true; } diff --git a/mm/mmap.c b/mm/mmap.c index 7b2992ef8ee0..db849e3ed9d3 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1850,7 +1850,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr, } /* Allow architectures to sanity-check the vm_flags */ - if (!arch_validate_flags(vma->vm_flags)) { + if (!arch_validate_flags(vma->vm_flags, vma_is_anonymous(vma))) { error = -EINVAL; if (file) goto unmap_and_free_vma; diff --git a/mm/mprotect.c b/mm/mprotect.c index 3b2f0d75519f..64378b963548 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -611,7 +611,7 @@ static int do_mprotect_pkey(unsigned long start, size_t len, } /* Allow architectures to sanity-check the new flags */ - if (!arch_validate_flags(newflags)) { + if (!arch_validate_flags(newflags, vma_is_anonymous(vma))) { error = -EINVAL; goto out; }
When newer VM flags are being created, such as VM_MTE, it becomes necessary for mmap/mprotect to verify if certain flags are being applied to an anonymous VMA. To solve this, one approach is adding a VM flag to track that MAP_ANONYMOUS is specified [1], and then using the flag in arch_validate_flags(). Another approach is passing vma_is_anonymous() to arch_validate_flags(). To prepare the introduction of PROT_SHSTK, which creates a shadow stack mapping and can only be applied to an anonymous VMA, update arch_validate_ flags() to include anonymous VMA information. [1] commit 9f3419315f3c ("arm64: mte: Add PROT_MTE support to mmap() and mprotect()"), Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Kees Cook <keescook@chromium.org> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> Cc: Will Deacon <will@kernel.org> --- arch/arm64/include/asm/mman.h | 4 ++-- arch/sparc/include/asm/mman.h | 4 ++-- include/linux/mman.h | 2 +- mm/mmap.c | 2 +- mm/mprotect.c | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-)