Skip to content

Commit 612b8a3

Browse files
mjkravetzakpm00
authored andcommitted
hugetlb: fix memory leak associated with vma_lock structure
The hugetlb vma_lock structure hangs off the vm_private_data pointer of sharable hugetlb vmas. The structure is vma specific and can not be shared between vmas. At fork and various other times, vmas are duplicated via vm_area_dup(). When this happens, the pointer in the newly created vma must be cleared and the structure reallocated. Two hugetlb specific routines deal with this hugetlb_dup_vma_private and hugetlb_vm_op_open. Both routines are called for newly created vmas. hugetlb_dup_vma_private would always clear the pointer and hugetlb_vm_op_open would allocate the new vms_lock structure. This did not work in the case of this calling sequence pointed out in [1]. move_vma copy_vma new_vma = vm_area_dup(vma); new_vma->vm_ops->open(new_vma); --> new_vma has its own vma lock. is_vm_hugetlb_page(vma) clear_vma_resv_huge_pages hugetlb_dup_vma_private --> vma->vm_private_data is set to NULL When clearing hugetlb_dup_vma_private we actually leak the associated vma_lock structure. The vma_lock structure contains a pointer to the associated vma. This information can be used in hugetlb_dup_vma_private and hugetlb_vm_op_open to ensure we only clear the vm_private_data of newly created (copied) vmas. In such cases, the vma->vma_lock->vma field will not point to the vma. Update hugetlb_dup_vma_private and hugetlb_vm_op_open to not clear vm_private_data if vma->vma_lock->vma == vma. Also, log a warning if hugetlb_vm_op_open ever encounters the case where vma_lock has already been correctly allocated for the vma. [1] https://lore.kernel.org/linux-mm/[email protected]/ Link: https://lkml.kernel.org/r/[email protected] Fixes: 131a79b ("hugetlb: fix vma lock handling during split vma and range unmapping") Signed-off-by: Mike Kravetz <[email protected]> Reviewed-by: Miaohe Lin <[email protected]> Cc: Andrea Arcangeli <[email protected]> Cc: "Aneesh Kumar K.V" <[email protected]> Cc: Axel Rasmussen <[email protected]> Cc: David Hildenbrand <[email protected]> Cc: Davidlohr Bueso <[email protected]> Cc: James Houghton <[email protected]> Cc: "Kirill A. Shutemov" <[email protected]> Cc: Michal Hocko <[email protected]> Cc: Mina Almasry <[email protected]> Cc: Muchun Song <[email protected]> Cc: Naoya Horiguchi <[email protected]> Cc: Pasha Tatashin <[email protected]> Cc: Peter Xu <[email protected]> Cc: Prakash Sangappa <[email protected]> Cc: Sven Schnelle <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
1 parent df48a5f commit 612b8a3

File tree

1 file changed

+27
-8
lines changed

1 file changed

+27
-8
lines changed

mm/hugetlb.c

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,15 +1014,23 @@ void hugetlb_dup_vma_private(struct vm_area_struct *vma)
10141014
VM_BUG_ON_VMA(!is_vm_hugetlb_page(vma), vma);
10151015
/*
10161016
* Clear vm_private_data
1017+
* - For shared mappings this is a per-vma semaphore that may be
1018+
* allocated in a subsequent call to hugetlb_vm_op_open.
1019+
* Before clearing, make sure pointer is not associated with vma
1020+
* as this will leak the structure. This is the case when called
1021+
* via clear_vma_resv_huge_pages() and hugetlb_vm_op_open has already
1022+
* been called to allocate a new structure.
10171023
* - For MAP_PRIVATE mappings, this is the reserve map which does
10181024
* not apply to children. Faults generated by the children are
10191025
* not guaranteed to succeed, even if read-only.
1020-
* - For shared mappings this is a per-vma semaphore that may be
1021-
* allocated in a subsequent call to hugetlb_vm_op_open.
10221026
*/
1023-
vma->vm_private_data = (void *)0;
1024-
if (!(vma->vm_flags & VM_MAYSHARE))
1025-
return;
1027+
if (vma->vm_flags & VM_MAYSHARE) {
1028+
struct hugetlb_vma_lock *vma_lock = vma->vm_private_data;
1029+
1030+
if (vma_lock && vma_lock->vma != vma)
1031+
vma->vm_private_data = NULL;
1032+
} else
1033+
vma->vm_private_data = NULL;
10261034
}
10271035

10281036
/*
@@ -4601,6 +4609,7 @@ static void hugetlb_vm_op_open(struct vm_area_struct *vma)
46014609
struct resv_map *resv = vma_resv_map(vma);
46024610

46034611
/*
4612+
* HPAGE_RESV_OWNER indicates a private mapping.
46044613
* This new VMA should share its siblings reservation map if present.
46054614
* The VMA will only ever have a valid reservation map pointer where
46064615
* it is being copied for another still existing VMA. As that VMA
@@ -4615,11 +4624,21 @@ static void hugetlb_vm_op_open(struct vm_area_struct *vma)
46154624

46164625
/*
46174626
* vma_lock structure for sharable mappings is vma specific.
4618-
* Clear old pointer (if copied via vm_area_dup) and create new.
4627+
* Clear old pointer (if copied via vm_area_dup) and allocate
4628+
* new structure. Before clearing, make sure vma_lock is not
4629+
* for this vma.
46194630
*/
46204631
if (vma->vm_flags & VM_MAYSHARE) {
4621-
vma->vm_private_data = NULL;
4622-
hugetlb_vma_lock_alloc(vma);
4632+
struct hugetlb_vma_lock *vma_lock = vma->vm_private_data;
4633+
4634+
if (vma_lock) {
4635+
if (vma_lock->vma != vma) {
4636+
vma->vm_private_data = NULL;
4637+
hugetlb_vma_lock_alloc(vma);
4638+
} else
4639+
pr_warn("HugeTLB: vma_lock already exists in %s.\n", __func__);
4640+
} else
4641+
hugetlb_vma_lock_alloc(vma);
46234642
}
46244643
}
46254644

0 commit comments

Comments
 (0)