diff mbox series

[v25,29/30] mm: Update arch_validate_flags() to include vma anonymous

Message ID 20210415221419.31835-30-yu-cheng.yu@intel.com (mailing list archive)
State New
Headers show
Series Control-flow Enforcement: Shadow Stack | expand

Commit Message

Yu, Yu-cheng April 15, 2021, 10:14 p.m. UTC
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(-)

Comments

Kirill A. Shutemov April 26, 2021, 6:40 a.m. UTC | #1
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.
Catalin Marinas April 26, 2021, 11:11 a.m. UTC | #2
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.
Yu, Yu-cheng April 26, 2021, 5:56 p.m. UTC | #3
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 mbox series

Patch

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;
 		}