Currently MTE is permitted in two circumstances (desiring to use MTE having been specified by the VM_MTE flag) - where MAP_ANONYMOUS is specified, as checked by arch_calc_vm_flag_bits() and actualised by setting the VM_MTE_ALLOWED flag, or if the file backing the mapping is shmem, in which case we set VM_MTE_ALLOWED in shmem_mmap() when the mmap hook is activated in mmap_region(). The function that checks that, if VM_MTE is set, VM_MTE_ALLOWED is also set is the arm64 implementation of arch_validate_flags(). Unfortunately, we intend to refactor mmap_region() to perform this check earlier, meaning that in the case of a shmem backing we will not have invoked shmem_mmap() yet, causing the mapping to fail spuriously. It is inappropriate to set this architecture-specific flag in general mm code anyway, so a sensible resolution of this issue is to instead move the check to arch_validate_flags() itself. This requires a modification to the arch_validate_flags() signature to pass in a pointer to the struct file associated with the mapping, however this is not too egregious as this is only used by two architectures anyway - arm64 and sparc. So this patch performs this adjustment and moves the check to arch_validate_flags() which resolves the issue. We'd ideally like to have eliminated the arch_calc_vm_flag_bits() case, but we risk inadvertently changing behaviour as we do not have mmap() flags available at the point of the arch_validate_flags() check and a MAP_ANON | MAP_HUGETLB case would be accepted for MTE currently, but a MAP_SHARED | MAP_HUGETLB would not. This is likely an oversight but we want to try to keep behaviour identical to before in this patch. So continue to check VM_MTE_ALLOWED which arch_calc_vm_flag_bits() sets if MAP_ANON. Reported-by: Jann Horn <jannh@xxxxxxxxxx> Fixes: deb0f6562884 ("mm/mmap: undo ->mmap() when arch_validate_flags() fails") Cc: stable <stable@xxxxxxxxxx> Acked-by: Vlastimil Babka <vbabka@xxxxxxx> Tested-by: Mark Brown <broonie@xxxxxxxxxx> Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> --- arch/arm64/include/asm/mman.h | 29 +++++++++++++++++++++++++---- arch/sparc/include/asm/mman.h | 5 +++-- include/linux/mman.h | 2 +- mm/mmap.c | 2 +- mm/mprotect.c | 2 +- mm/shmem.c | 3 --- 6 files changed, 31 insertions(+), 12 deletions(-) diff --git a/arch/arm64/include/asm/mman.h b/arch/arm64/include/asm/mman.h index 9e39217b4afb..94925a0482e3 100644 --- a/arch/arm64/include/asm/mman.h +++ b/arch/arm64/include/asm/mman.h @@ -6,7 +6,9 @@ #ifndef BUILD_VDSO #include <linux/compiler.h> +#include <linux/fs.h> #include <linux/types.h> +#include <linux/shmem_fs.h> static inline unsigned long arch_calc_vm_prot_bits(unsigned long prot, unsigned long pkey) @@ -60,15 +62,34 @@ 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(struct file *file, unsigned long vm_flags) { if (!system_supports_mte()) return true; - /* only allow VM_MTE if VM_MTE_ALLOWED has been set previously */ - return !(vm_flags & VM_MTE) || (vm_flags & VM_MTE_ALLOWED); + if (!(vm_flags & VM_MTE)) + return true; + + /* + * We explicitly permit MAP_ANONYMOUS and shmem backed mappings to use + * MTE. We check for shmem explicitly here. + * + * The MAP_ANONYMOUS case is handled by arch_calc_vm_flag_bits() which + * explicitly sets VM_MTE_ALLOWED which we check for here also. + * + * Ideally we'd perform both checks here but existing semantics support + * hugetlb in MAP_ANONYMOUS mode but not a MAP_SHARED mapping, which is + * likely unintended but we maintain identical behaviour for + * consistency. + */ + if (vm_flags & VM_MTE_ALLOWED) + return true; + if (shmem_file(file)) + return true; + + return false; } -#define arch_validate_flags(vm_flags) arch_validate_flags(vm_flags) +#define arch_validate_flags(file, vm_flags) arch_validate_flags(file, vm_flags) #endif /* !BUILD_VDSO */ diff --git a/arch/sparc/include/asm/mman.h b/arch/sparc/include/asm/mman.h index af9c10c83dc5..d426e1f7c2c1 100644 --- a/arch/sparc/include/asm/mman.h +++ b/arch/sparc/include/asm/mman.h @@ -10,6 +10,7 @@ int sparc_mmap_check(unsigned long addr, unsigned long len); #ifdef CONFIG_SPARC64 #include <asm/adi_64.h> +#include <linux/fs.h> static inline void ipi_set_tstate_mcde(void *arg) { @@ -54,11 +55,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(file, vm_flags) arch_validate_flags(file, vm_flags) /* 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(struct file *file, unsigned long vm_flags) { /* 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 8ddca62d6460..82e6488026b7 100644 --- a/include/linux/mman.h +++ b/include/linux/mman.h @@ -117,7 +117,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(struct file *file, unsigned long flags) { return true; } diff --git a/mm/mmap.c b/mm/mmap.c index ab71d4c3464c..40b5858ae875 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1511,7 +1511,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(file, vma->vm_flags)) { error = -EINVAL; goto close_and_free_vma; } diff --git a/mm/mprotect.c b/mm/mprotect.c index 6f450af3252e..c6db98b893fc 100644 --- a/mm/mprotect.c +++ b/mm/mprotect.c @@ -816,7 +816,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(vma->vm_file, newflags)) { error = -EINVAL; break; } diff --git a/mm/shmem.c b/mm/shmem.c index 4ba1d00fabda..e87f5d6799a7 100644 --- a/mm/shmem.c +++ b/mm/shmem.c @@ -2733,9 +2733,6 @@ static int shmem_mmap(struct file *file, struct vm_area_struct *vma) if (ret) return ret; - /* arm64 - allow memory tagging on RAM-based files */ - vm_flags_set(vma, VM_MTE_ALLOWED); - file_accessed(file); /* This is anonymous shared memory if it is unlinked at the time of mmap */ if (inode->i_nlink) -- 2.47.0