Re: [PATCH hotfix 6.12 v2 4/8] mm: resolve faulty mmap_region() error path behaviour
From: Catalin Marinas <catalin.marinas@arm.com>
Date: 2024-10-29 15:04:45
Also in:
linux-mm, lkml
Hi Lorenzo, Thanks for trying to fix this. On Mon, Oct 28, 2024 at 10:14:50PM +0000, Lorenzo Stoakes wrote:
From 247003cd2a4b5f4fc2dac97f5ef7e473a47f4324 Mon Sep 17 00:00:00 2001 From: Lorenzo Stoakes <redacted> Date: Mon, 28 Oct 2024 22:05:44 +0000 Subject: [PATCH] mm: perform MTE check within arm64 hook entirely It doesn't make sense to have shmem explicitly check this one arch-specific case, it is arch-specific, so the arch should handle it. We know shmem is a case in which we want to permit MTE, so simply check for this directly. This also fixes the issue with checking arch_validate_flags() early, which would otherwise break mmap_region(). In order to implement this we must pass a file pointer, and additionally update the sparc code to accept this parameter too. 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.
MAP_HUGETLB support for MTE is only in -next currently, so there wouldn't be any ABI change if we also allowed MAP_SHARED | MAP_HUGETLB. In 6.12, MAP_HUGETLB is not allowed to have PROT_MTE.
So continue to check VM_MTE_ALLOWED which arch_calc_vm_flag_bits() sets if MAP_ANON.
[...]
quoted hunk ↗ jump to hunk
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);
This breaks arm64 KVM if the VMM uses shared mappings for the memory slots (which is possible). We have kvm_vma_mte_allowed() that checks for the VM_MTE_ALLOWED flag as the VMM may not use PROT_MTE/VM_MTE directly. I need to read this thread properly but why not pass the file argument to arch_calc_vm_flag_bits() and set VM_MTE_ALLOWED in there? -- Catalin