Many places in the kernel use find_vma() to get a vma and then check the start address of the vma to ensure the next vma was not returned. Other places use the find_vma_intersection() call with add, addr + 1 as the range; looking for just the vma at a specific address. The third use of find_vma() is by developers who do not know that the function starts searching at the provided address upwards for the next vma. This results in a bug that is often overlooked for a long time. Adding the new vma_lookup() function will allow for cleaner code by removing the find_vma() calls which check limits, making find_vma_intersection() calls of a single address to be shorter, and potentially reduce the incorrect uses of find_vma(). This patch set was originally part of the maple tree patch set. Changes since being broken out on its own: - Changed initial implementation of vma_lookup() to use find_vma() as requested by Michel Lespinasse. - Updated commit comments to be more descriptive These patches are based on next-20210506 Liam R. Howlett (22): mm: Add vma_lookup() drm/i915/selftests: Use vma_lookup() in __igt_mmap() arch/arc/kernel/troubleshoot: use vma_lookup() instead of find_vma() arch/arm64/kvm: Use vma_lookup() instead of find_vma_intersection() arch/powerpc/kvm/book3s_hv_uvmem: Use vma_lookup() instead of find_vma_intersection() arch/powerpc/kvm/book3s: Use vma_lookup() in kvmppc_hv_setup_htab_rma() arch/mips/kernel/traps: Use vma_lookup() instead of find_vma() arch/m68k/kernel/sys_m68k: Use vma_lookup() in sys_cacheflush() x86/sgx: Use vma_lookup() in sgx_encl_find() virt/kvm: Use vma_lookup() instead of find_vma_intersection() vfio: Use vma_lookup() instead of find_vma_intersection() net/ipv5/tcp: Use vma_lookup() in tcp_zerocopy_receive() drm/amdgpu: Use vma_lookup() in amdgpu_ttm_tt_get_user_pages() media: videobuf2: Use vma_lookup() in get_vaddr_frames() misc/sgi-gru/grufault: Use vma_lookup() in gru_find_vma() kernel/events/uprobes: Use vma_lookup() in find_active_uprobe() lib/test_hmm: Use vma_lookup() in dmirror_migrate() mm/ksm: Use vma_lookup() in find_mergeable_vma() mm/migrate: Use vma_lookup() in do_pages_stat_array() mm/mremap: Use vma_lookup() in vma_to_resize() mm/memory.c: Use vma_lookup() in __access_remote_vm() mm/mempolicy: Use vma_lookup() in __access_remote_vm() arch/arc/kernel/troubleshoot.c | 8 ++++---- arch/arm64/kvm/mmu.c | 2 +- arch/m68k/kernel/sys_m68k.c | 4 ++-- arch/mips/kernel/traps.c | 4 +--- arch/powerpc/kvm/book3s_hv.c | 4 ++-- arch/powerpc/kvm/book3s_hv_uvmem.c | 2 +- arch/x86/kernel/cpu/sgx/encl.h | 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++-- .../gpu/drm/i915/gem/selftests/i915_gem_mman.c | 2 +- drivers/media/common/videobuf2/frame_vector.c | 2 +- drivers/misc/sgi-gru/grufault.c | 4 ++-- drivers/vfio/vfio_iommu_type1.c | 2 +- include/linux/mm.h | 18 ++++++++++++++++++ kernel/events/uprobes.c | 4 ++-- lib/test_hmm.c | 5 ++--- mm/ksm.c | 6 ++---- mm/memory.c | 4 ++-- mm/mempolicy.c | 2 +- mm/migrate.c | 4 ++-- mm/mremap.c | 4 ++-- net/ipv4/tcp.c | 4 ++-- virt/kvm/kvm_main.c | 2 +- 22 files changed, 54 insertions(+), 41 deletions(-) -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- arch/arc/kernel/troubleshoot.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arc/kernel/troubleshoot.c b/arch/arc/kernel/troubleshoot.c index a331bb5d8319..7654c2e42dc0 100644 --- a/arch/arc/kernel/troubleshoot.c +++ b/arch/arc/kernel/troubleshoot.c @@ -83,12 +83,12 @@ static void show_faulting_vma(unsigned long address) * non-inclusive vma */ mmap_read_lock(active_mm); - vma = find_vma(active_mm, address); + vma = vma_lookup(active_mm, address); - /* check against the find_vma( ) behaviour which returns the next VMA - * if the container VMA is not found + /* Lookup the vma at the address and report if the container VMA is not + * found */ - if (vma && (vma->vm_start <= address)) { + if (vma) { char buf[ARC_PATH_MAX]; char *nm = "?"; -- 2.30.2
Many places in the kernel use find_vma() to get a vma and then check the start address of the vma to ensure the next vma was not returned. Other places use the find_vma_intersection() call with add, addr + 1 as the range; looking for just the vma at a specific address. The third use of find_vma() is by developers who do not know that the function starts searching at the provided address upwards for the next vma. This results in a bug that is often overlooked for a long time. Adding the new vma_lookup() function will allow for cleaner code by removing the find_vma() calls which check limits, making find_vma_intersection() calls of a single address to be shorter, and potentially reduce the incorrect uses of find_vma(). Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- include/linux/mm.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/include/linux/mm.h b/include/linux/mm.h index 25b9041f9925..5f2a15e702ff 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2689,6 +2689,24 @@ static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * m return vma; } +/** + * vma_lookup() - Find a VMA at a specific address + * @mm: The process address space. + * @addr: The user address. + * + * Return: The vm_area_struct at the given address, %NULL otherwise. + */ +static inline +struct vm_area_struct *vma_lookup(struct mm_struct *mm, unsigned long addr) +{ + struct vm_area_struct * vma = find_vma(mm, addr); + + if (vma && addr < vma->vm_start) + vma = NULL; + + return vma; +} + static inline unsigned long vm_start_gap(struct vm_area_struct *vma) { unsigned long vm_start = vma->vm_start; -- 2.30.2
vma_lookup() will look up the vma at a specific address. find_vma() will start the search for a specific address and continue upwards. This fixes an issue with the selftest as the returned vma may not be the newly created vma, but simply the vma at a higher address. Fixes: 6fedafacae1b (drm/i915/selftests: Wrap vm_mmap() around GEM objects Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c index 5cf6df49c333..35c15ef1327d 100644 --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c @@ -871,7 +871,7 @@ static int __igt_mmap(struct drm_i915_private *i915, pr_debug("igt_mmap(%s, %d) @ %lx\n", obj->mm.region->name, type, addr); - area = find_vma(current->mm, addr); + area = vma_lookup(current->mm, addr); if (!area) { pr_err("%s: Did not create a vm_area_struct for the mmap\n", obj->mm.region->name); -- 2.30.2
vma_lookup() finds the vma of a specific address with a cleaner interface and is more readable. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- arch/arm64/kvm/mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c index c5d1f3c87dbd..8b112b594e09 100644 --- a/arch/arm64/kvm/mmu.c +++ b/arch/arm64/kvm/mmu.c @@ -855,7 +855,7 @@ static int user_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa, /* Let's check if we will get back a huge page backed by hugetlbfs */ mmap_read_lock(current->mm); - vma = find_vma_intersection(current->mm, hva, hva + 1); + vma = vma_lookup(current->mm, hva); if (unlikely(!vma)) { kvm_err("Failed to find VMA for hva 0x%lx\n", hva); mmap_read_unlock(current->mm); -- 2.30.2
vma_lookup() finds the vma of a specific address with a cleaner interface and is more readable. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- arch/powerpc/kvm/book3s_hv_uvmem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c index 84e5a2dc8be5..34720b79588f 100644 --- a/arch/powerpc/kvm/book3s_hv_uvmem.c +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c @@ -614,7 +614,7 @@ void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *slot, /* Fetch the VMA if addr is not in the latest fetched one */ if (!vma || addr >= vma->vm_end) { - vma = find_vma_intersection(kvm->mm, addr, addr+1); + vma = vma_lookup(kvm->mm, addr); if (!vma) { pr_err("Can't find VMA for gfn:0x%lx\n", gfn); break; -- 2.30.2
Using vma_lookup() removes the requirement to check if the address is within the returned vma. The code is easier to understand and more compact. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- arch/powerpc/kvm/book3s_hv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index 28a80d240b76..a3a4b2179350 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -4759,8 +4759,8 @@ static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu) /* Look up the VMA for the start of this memory slot */ hva = memslot->userspace_addr; mmap_read_lock(kvm->mm); - vma = find_vma(kvm->mm, hva); - if (!vma || vma->vm_start > hva || (vma->vm_flags & VM_IO)) + vma = vma_lookup(kvm->mm, hva); + if (!vma || (vma->vm_flags & VM_IO)) goto up_out; psize = vma_kernel_pagesize(vma); -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- arch/mips/kernel/traps.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c index 0b4e06303c55..6f07362de5ce 100644 --- a/arch/mips/kernel/traps.c +++ b/arch/mips/kernel/traps.c @@ -784,7 +784,6 @@ void force_fcr31_sig(unsigned long fcr31, void __user *fault_addr, int process_fpemu_return(int sig, void __user *fault_addr, unsigned long fcr31) { int si_code; - struct vm_area_struct *vma; switch (sig) { case 0: @@ -800,8 +799,7 @@ int process_fpemu_return(int sig, void __user *fault_addr, unsigned long fcr31) case SIGSEGV: mmap_read_lock(current->mm); - vma = find_vma(current->mm, (unsigned long)fault_addr); - if (vma && (vma->vm_start <= (unsigned long)fault_addr)) + if (vma_lookup(current->mm, (unsigned long)fault_addr)) si_code = SEGV_ACCERR; else si_code = SEGV_MAPERR; -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- arch/x86/kernel/cpu/sgx/encl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h index 6e74f85b6264..fec43ca65065 100644 --- a/arch/x86/kernel/cpu/sgx/encl.h +++ b/arch/x86/kernel/cpu/sgx/encl.h @@ -91,8 +91,8 @@ static inline int sgx_encl_find(struct mm_struct *mm, unsigned long addr, { struct vm_area_struct *result; - result = find_vma(mm, addr); - if (!result || result->vm_ops != &sgx_vm_ops || addr < result->vm_start) + result = vma_lookup(mm, addr); + if (!result || result->vm_ops != &sgx_vm_ops) return -EINVAL; *vma = result; -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- arch/m68k/kernel/sys_m68k.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/m68k/kernel/sys_m68k.c b/arch/m68k/kernel/sys_m68k.c index f55bdcb8e4f1..bd0274c7592e 100644 --- a/arch/m68k/kernel/sys_m68k.c +++ b/arch/m68k/kernel/sys_m68k.c @@ -402,8 +402,8 @@ sys_cacheflush (unsigned long addr, int scope, int cache, unsigned long len) * to this process. */ mmap_read_lock(current->mm); - vma = find_vma(current->mm, addr); - if (!vma || addr < vma->vm_start || addr + len > vma->vm_end) + vma = vma_lookup(current->mm, addr); + if (!vma || addr + len > vma->vm_end) goto out_unlock; } -- 2.30.2
vma_lookup() finds the vma of a specific address with a cleaner interface and is more readable. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- virt/kvm/kvm_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 2799c6660cce..a7703b11407a 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2154,7 +2154,7 @@ static kvm_pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async, } retry: - vma = find_vma_intersection(current->mm, addr, addr + 1); + vma = vma_lookup(current->mm, addr); if (vma == NULL) pfn = KVM_PFN_ERR_FAULT; -- 2.30.2
vma_lookup() finds the vma of a specific address with a cleaner interface and is more readable. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- drivers/vfio/vfio_iommu_type1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c index a0747c35a778..fb695bf0b1c4 100644 --- a/drivers/vfio/vfio_iommu_type1.c +++ b/drivers/vfio/vfio_iommu_type1.c @@ -567,7 +567,7 @@ static int vaddr_get_pfns(struct mm_struct *mm, unsigned long vaddr, vaddr = untagged_addr(vaddr); retry: - vma = find_vma_intersection(mm, vaddr, vaddr + 1); + vma = vma_lookup(mm, vaddr); if (vma && vma->vm_flags & VM_PFNMAP) { ret = follow_fault_pfn(vma, mm, vaddr, pfn, prot & IOMMU_WRITE); -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- net/ipv4/tcp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index e14fd0c50c10..d4781a514012 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -2094,8 +2094,8 @@ static int tcp_zerocopy_receive(struct sock *sk, mmap_read_lock(current->mm); - vma = find_vma(current->mm, address); - if (!vma || vma->vm_start > address || vma->vm_ops != &tcp_vm_ops) { + vma = vma_lookup(current->mm, address); + if (!vma || vma->vm_ops != &tcp_vm_ops) { mmap_read_unlock(current->mm); return -EINVAL; } -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c index 3251f6b67e23..00b7fa8b953b 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c @@ -694,9 +694,9 @@ int amdgpu_ttm_tt_get_user_pages(struct amdgpu_bo *bo, struct page **pages) return -ESRCH; mmap_read_lock(mm); - vma = find_vma(mm, start); + vma = vma_lookup(mm, start); mmap_read_unlock(mm); - if (unlikely(!vma || start < vma->vm_start)) { + if (unlikely(!vma)) { r = -EFAULT; goto out_putmm; } -- 2.30.2
vma_lookup() finds the vma of a specific address with a cleaner interface and is more readable. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- drivers/media/common/videobuf2/frame_vector.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/common/videobuf2/frame_vector.c b/drivers/media/common/videobuf2/frame_vector.c index 91fea7199e85..b84b706073cb 100644 --- a/drivers/media/common/videobuf2/frame_vector.c +++ b/drivers/media/common/videobuf2/frame_vector.c @@ -64,7 +64,7 @@ int get_vaddr_frames(unsigned long start, unsigned int nr_frames, do { unsigned long *nums = frame_vector_pfns(vec); - vma = find_vma_intersection(mm, start, start + 1); + vma = vma_lookup(mm, start); if (!vma) break; -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- drivers/misc/sgi-gru/grufault.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/misc/sgi-gru/grufault.c b/drivers/misc/sgi-gru/grufault.c index 723825524ea0..d7ef61e602ed 100644 --- a/drivers/misc/sgi-gru/grufault.c +++ b/drivers/misc/sgi-gru/grufault.c @@ -49,8 +49,8 @@ struct vm_area_struct *gru_find_vma(unsigned long vaddr) { struct vm_area_struct *vma; - vma = find_vma(current->mm, vaddr); - if (vma && vma->vm_start <= vaddr && vma->vm_ops == &gru_vm_ops) + vma = vma_lookup(current->mm, vaddr); + if (vma && vma->vm_ops == &gru_vm_ops) return vma; return NULL; } -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- kernel/events/uprobes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index 6addc9780319..907d4ee00cb2 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -2046,8 +2046,8 @@ static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp) struct vm_area_struct *vma; mmap_read_lock(mm); - vma = find_vma(mm, bp_vaddr); - if (vma && vma->vm_start <= bp_vaddr) { + vma = vma_lookup(mm, bp_vaddr); + if (vma) { if (valid_vma(vma, false)) { struct inode *inode = file_inode(vma->vm_file); loff_t offset = vaddr_to_offset(vma, bp_vaddr); -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- mm/ksm.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/mm/ksm.c b/mm/ksm.c index 6bbe314c5260..ced6830d0ff4 100644 --- a/mm/ksm.c +++ b/mm/ksm.c @@ -521,10 +521,8 @@ static struct vm_area_struct *find_mergeable_vma(struct mm_struct *mm, struct vm_area_struct *vma; if (ksm_test_exit(mm)) return NULL; - vma = find_vma(mm, addr); - if (!vma || vma->vm_start > addr) - return NULL; - if (!(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma) + vma = vma_lookup(mm, addr); + if (!vma || !(vma->vm_flags & VM_MERGEABLE) || !vma->anon_vma) return NULL; return vma; } -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- lib/test_hmm.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/test_hmm.c b/lib/test_hmm.c index 80a78877bd93..15f2e2db77bc 100644 --- a/lib/test_hmm.c +++ b/lib/test_hmm.c @@ -686,9 +686,8 @@ static int dmirror_migrate(struct dmirror *dmirror, mmap_read_lock(mm); for (addr = start; addr < end; addr = next) { - vma = find_vma(mm, addr); - if (!vma || addr < vma->vm_start || - !(vma->vm_flags & VM_READ)) { + vma = vma_lookup(mm, addr); + if (!vma || !(vma->vm_flags & VM_READ)) { ret = -EINVAL; goto out; } -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- mm/mremap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/mremap.c b/mm/mremap.c index 47c255b60150..04143755cd1e 100644 --- a/mm/mremap.c +++ b/mm/mremap.c @@ -634,10 +634,10 @@ static struct vm_area_struct *vma_to_resize(unsigned long addr, unsigned long *p) { struct mm_struct *mm = current->mm; - struct vm_area_struct *vma = find_vma(mm, addr); + struct vm_area_struct *vma = vma_lookup(mm, addr); unsigned long pgoff; - if (!vma || vma->vm_start > addr) + if (!vma) return ERR_PTR(-EFAULT); /* -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- mm/migrate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/migrate.c b/mm/migrate.c index b234c3f3acb7..611781c0f9b5 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -1833,8 +1833,8 @@ static void do_pages_stat_array(struct mm_struct *mm, unsigned long nr_pages, struct page *page; int err = -EFAULT; - vma = find_vma(mm, addr); - if (!vma || addr < vma->vm_start) + vma = vma_lookup(mm, addr); + if (!vma) goto set_status; /* FOLL_DUMP to ignore special (like zero) pages */ -- 2.30.2
Use vma_lookup() to find the VMA at a specific address. As vma_lookup() will return NULL if the address is not within any VMA, the start address no longer needs to be validated. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- mm/memory.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index 86ba6c1f6821..e3b56903b111 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -4954,8 +4954,8 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf, * Check if this is a VM_IO | VM_PFNMAP VMA, which * we can access using slightly different code. */ - vma = find_vma(mm, addr); - if (!vma || vma->vm_start > addr) + vma = vma_lookup(mm, addr); + if (!vma) break; if (vma->vm_ops && vma->vm_ops->access) ret = vma->vm_ops->access(vma, addr, buf, -- 2.30.2
vma_lookup() finds the vma of a specific address with a cleaner interface and is more readable. Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> --- mm/mempolicy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mm/mempolicy.c b/mm/mempolicy.c index d79fa299b70c..325771bef5e2 100644 --- a/mm/mempolicy.c +++ b/mm/mempolicy.c @@ -975,7 +975,7 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask, * want to return MPOL_DEFAULT in this case. */ mmap_read_lock(mm); - vma = find_vma_intersection(mm, addr, addr+1); + vma = vma_lookup(mm, addr); if (!vma) { mmap_read_unlock(mm); return -EFAULT; -- 2.30.2
On Mon, May 10, 2021 at 7:04 PM Liam Howlett <liam.howlett@oracle.com> wrote:
> Use vma_lookup() to find the VMA at a specific address. As vma_lookup()
> will return NULL if the address is not within any VMA, the start address
> no longer needs to be validated.
>
> Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Mon, May 10, 2021 at 7:04 PM Liam Howlett <liam.howlett@oracle.com> wrote:
> Use vma_lookup() to find the VMA at a specific address. As vma_lookup()
> will return NULL if the address is not within any VMA, the start address
> no longer needs to be validated.
>
> Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 10.05.21 18:58, Liam Howlett wrote:
> he third use of find_vma() is by developers who do not know that the
> function starts searching at the provided address upwards for the next
> vma. This results in a bug that is often overlooked for a long time.
>
> Adding the new vma_lookup() function will allow for cleaner code by
Sounds helpful to me
Acked-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
Le 10/05/2021 à 18:58, Liam Howlett a écrit : > Many places in the kernel use find_vma() to get a vma and then check the > start address of the vma to ensure the next vma was not returned. > > Other places use the find_vma_intersection() call with add, addr + 1 as > the range; looking for just the vma at a specific address. > > The third use of find_vma() is by developers who do not know that the > function starts searching at the provided address upwards for the next > vma. This results in a bug that is often overlooked for a long time. > > Adding the new vma_lookup() function will allow for cleaner code by > removing the find_vma() calls which check limits, making > find_vma_intersection() calls of a single address to be shorter, and > potentially reduce the incorrect uses of find_vma(). > > This patch set was originally part of the maple tree patch set. FWIW, for the whole series: Reviewed-by: Laurent Dufour <ldufour@linux.ibm.com> > Changes since being broken out on its own: > - Changed initial implementation of vma_lookup() to use find_vma() as requested > by Michel Lespinasse. > - Updated commit comments to be more descriptive > > These patches are based on next-20210506 > > Liam R. Howlett (22): > mm: Add vma_lookup() > drm/i915/selftests: Use vma_lookup() in __igt_mmap() > arch/arc/kernel/troubleshoot: use vma_lookup() instead of find_vma() > arch/arm64/kvm: Use vma_lookup() instead of find_vma_intersection() > arch/powerpc/kvm/book3s_hv_uvmem: Use vma_lookup() instead of > find_vma_intersection() > arch/powerpc/kvm/book3s: Use vma_lookup() in > kvmppc_hv_setup_htab_rma() > arch/mips/kernel/traps: Use vma_lookup() instead of find_vma() > arch/m68k/kernel/sys_m68k: Use vma_lookup() in sys_cacheflush() > x86/sgx: Use vma_lookup() in sgx_encl_find() > virt/kvm: Use vma_lookup() instead of find_vma_intersection() > vfio: Use vma_lookup() instead of find_vma_intersection() > net/ipv5/tcp: Use vma_lookup() in tcp_zerocopy_receive() > drm/amdgpu: Use vma_lookup() in amdgpu_ttm_tt_get_user_pages() > media: videobuf2: Use vma_lookup() in get_vaddr_frames() > misc/sgi-gru/grufault: Use vma_lookup() in gru_find_vma() > kernel/events/uprobes: Use vma_lookup() in find_active_uprobe() > lib/test_hmm: Use vma_lookup() in dmirror_migrate() > mm/ksm: Use vma_lookup() in find_mergeable_vma() > mm/migrate: Use vma_lookup() in do_pages_stat_array() > mm/mremap: Use vma_lookup() in vma_to_resize() > mm/memory.c: Use vma_lookup() in __access_remote_vm() > mm/mempolicy: Use vma_lookup() in __access_remote_vm() > > arch/arc/kernel/troubleshoot.c | 8 ++++---- > arch/arm64/kvm/mmu.c | 2 +- > arch/m68k/kernel/sys_m68k.c | 4 ++-- > arch/mips/kernel/traps.c | 4 +--- > arch/powerpc/kvm/book3s_hv.c | 4 ++-- > arch/powerpc/kvm/book3s_hv_uvmem.c | 2 +- > arch/x86/kernel/cpu/sgx/encl.h | 4 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 4 ++-- > .../gpu/drm/i915/gem/selftests/i915_gem_mman.c | 2 +- > drivers/media/common/videobuf2/frame_vector.c | 2 +- > drivers/misc/sgi-gru/grufault.c | 4 ++-- > drivers/vfio/vfio_iommu_type1.c | 2 +- > include/linux/mm.h | 18 ++++++++++++++++++ > kernel/events/uprobes.c | 4 ++-- > lib/test_hmm.c | 5 ++--- > mm/ksm.c | 6 ++---- > mm/memory.c | 4 ++-- > mm/mempolicy.c | 2 +- > mm/migrate.c | 4 ++-- > mm/mremap.c | 4 ++-- > net/ipv4/tcp.c | 4 ++-- > virt/kvm/kvm_main.c | 2 +- > 22 files changed, 54 insertions(+), 41 deletions(-) >
On Mon, 10 May 2021, Liam Howlett wrote: >Many places in the kernel use find_vma() to get a vma and then check the >start address of the vma to ensure the next vma was not returned. > >Other places use the find_vma_intersection() call with add, addr + 1 as >the range; looking for just the vma at a specific address. > >The third use of find_vma() is by developers who do not know that the >function starts searching at the provided address upwards for the next >vma. This results in a bug that is often overlooked for a long time. > >Adding the new vma_lookup() function will allow for cleaner code by >removing the find_vma() calls which check limits, making >find_vma_intersection() calls of a single address to be shorter, and >potentially reduce the incorrect uses of find_vma(). > >Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> >--- > include/linux/mm.h | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > >diff --git a/include/linux/mm.h b/include/linux/mm.h >index 25b9041f9925..5f2a15e702ff 100644 >--- a/include/linux/mm.h >+++ b/include/linux/mm.h >@@ -2689,6 +2689,24 @@ static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * m > return vma; > } While at it can we clean up find_vma_intersection? I'm not particularly user/fan of checkpatch.pl, but this one is kind of ridiculous. Thanks, Davidlohr diff --git a/include/linux/mm.h b/include/linux/mm.h index c274f75efcf9..16eddedf783f 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2678,9 +2678,14 @@ extern struct vm_area_struct * find_vma(struct mm_struct * mm, unsigned long add extern struct vm_area_struct * find_vma_prev(struct mm_struct * mm, unsigned long addr, struct vm_area_struct **pprev); -/* Look up the first VMA which intersects the interval start_addr..end_addr-1, - NULL if none. Assume start_addr < end_addr. */ -static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * mm, unsigned long start_addr, unsigned long end_addr) +/* + * Look up the first VMA which intersects the interval start_addr..end_addr-1, + * NULL if none. Assume start_addr < end_addr. + */ +static inline +struct vm_area_struct *find_vma_intersection(struct mm_struct * mm, + unsigned long start_addr, + unsigned long end_addr) { struct vm_area_struct * vma = find_vma(mm,start_addr);
On Mon, 10 May 2021, Liam Howlett wrote: >Use vma_lookup() to find the VMA at a specific address. As vma_lookup() >will return NULL if the address is not within any VMA, the start address >no longer needs to be validated. > >Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> >--- > mm/mremap.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > >diff --git a/mm/mremap.c b/mm/mremap.c >index 47c255b60150..04143755cd1e 100644 >--- a/mm/mremap.c >+++ b/mm/mremap.c >@@ -634,10 +634,10 @@ static struct vm_area_struct *vma_to_resize(unsigned long addr, > unsigned long *p) > { > struct mm_struct *mm = current->mm; >- struct vm_area_struct *vma = find_vma(mm, addr); >+ struct vm_area_struct *vma = vma_lookup(mm, addr); > unsigned long pgoff; > Nit, but could the vma_lookup() call be separate from the declaration of vma? vma = find_vma(); >- if (!vma || vma->vm_start > addr) >+ if (!vma) > return ERR_PTR(-EFAULT); Thanks, Davidlohr
On Mon, 10 May 2021, Liam Howlett wrote:
>Adding the new vma_lookup() function will allow for cleaner code by
>removing the find_vma() calls which check limits, making
>find_vma_intersection() calls of a single address to be shorter, and
>potentially reduce the incorrect uses of find_vma().
I like this, specially implemented around find_vma(). For the series,
feel free to add:
Acked-by: Davidlohr Bueso <dbueso@suse.de>
* Davidlohr Bueso <dave@stgolabs.net> [210520 23:32]: > On Mon, 10 May 2021, Liam Howlett wrote: > > > Many places in the kernel use find_vma() to get a vma and then check the > > start address of the vma to ensure the next vma was not returned. > > > > Other places use the find_vma_intersection() call with add, addr + 1 as > > the range; looking for just the vma at a specific address. > > > > The third use of find_vma() is by developers who do not know that the > > function starts searching at the provided address upwards for the next > > vma. This results in a bug that is often overlooked for a long time. > > > > Adding the new vma_lookup() function will allow for cleaner code by > > removing the find_vma() calls which check limits, making > > find_vma_intersection() calls of a single address to be shorter, and > > potentially reduce the incorrect uses of find_vma(). > > > > Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> > > --- > > include/linux/mm.h | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index 25b9041f9925..5f2a15e702ff 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -2689,6 +2689,24 @@ static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * m > > return vma; > > } > > While at it can we clean up find_vma_intersection? I'm not particularly > user/fan of checkpatch.pl, but this one is kind of ridiculous. Agreed. This addition is worth re-spin. I will change the comment into a kernel documentation style comment at the same time. > > Thanks, > Davidlohr > > diff --git a/include/linux/mm.h b/include/linux/mm.h > index c274f75efcf9..16eddedf783f 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -2678,9 +2678,14 @@ extern struct vm_area_struct * find_vma(struct mm_struct * mm, unsigned long add > extern struct vm_area_struct * find_vma_prev(struct mm_struct * mm, unsigned long addr, > struct vm_area_struct **pprev); > > -/* Look up the first VMA which intersects the interval start_addr..end_addr-1, > - NULL if none. Assume start_addr < end_addr. */ > -static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * mm, unsigned long start_addr, unsigned long end_addr) > +/* > + * Look up the first VMA which intersects the interval start_addr..end_addr-1, > + * NULL if none. Assume start_addr < end_addr. > + */ > +static inline > +struct vm_area_struct *find_vma_intersection(struct mm_struct * mm, > + unsigned long start_addr, > + unsigned long end_addr) > { > struct vm_area_struct * vma = find_vma(mm,start_addr);
* Davidlohr Bueso <dave@stgolabs.net> [210520 23:47]: > On Mon, 10 May 2021, Liam Howlett wrote: > > > Use vma_lookup() to find the VMA at a specific address. As vma_lookup() > > will return NULL if the address is not within any VMA, the start address > > no longer needs to be validated. > > > > Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com> > > --- > > mm/mremap.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/mm/mremap.c b/mm/mremap.c > > index 47c255b60150..04143755cd1e 100644 > > --- a/mm/mremap.c > > +++ b/mm/mremap.c > > @@ -634,10 +634,10 @@ static struct vm_area_struct *vma_to_resize(unsigned long addr, > > unsigned long *p) > > { > > struct mm_struct *mm = current->mm; > > - struct vm_area_struct *vma = find_vma(mm, addr); > > + struct vm_area_struct *vma = vma_lookup(mm, addr); > > unsigned long pgoff; > > > > Nit, but could the vma_lookup() call be separate from the declaration > of vma? Yes, I will make this change. > > vma = find_vma(); > > - if (!vma || vma->vm_start > addr) > > + if (!vma) > > return ERR_PTR(-EFAULT); > > Thanks, > Davidlohr