mm: nommu: free unused resources when mremap shrinks the vma
Subsystems (2)
PATCH v2 1/1 mm: nommu: free unused resources when mremap shrinks the vma
When shrinking a VMA via mremap, the bounds are modified directly:
mm/nommu.c:do_mremap() {
...
vma->vm_end = vma->vm_start + new_len;
...
}This shrink the VMA without updating its bounds in the maple tree. If the maple tree (mm->mm_mt) still contains the old bounds, a user process could access the freed portion. The stale maple tree would incorrectly return the shrunk VMA for an address past its new vm_end.
This commit fixes this issue by calling vmi_shrink_vma() when shrink happens. Additionally, if a file-backed map is to be shrunk, it reports -EINVAL like do_munmap() does.
The issue is reported by Sashiko review, linked below.
--
v1 -> v2:
- handle error when vmi_shrink_vma() failed (reported by Sashiko)
- prevents mremap() with being shrunk for file-backed one like munmap()
v1: https://lore.kernel.org/linux-mm/[email protected]/
--- mm/nommu.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
mm/nommu.c +19 -1
@@ -1597,8 +1597,26 @@ static unsigned long do_mremap(unsigned long addr,if (new_len > vma->vm_region->vm_end - vma->vm_region->vm_start)return (unsigned long ) -ENOMEM ; +/* like do_munmap(), we're allowed to shrink an anonymous VMA but not a file-backed one */ +if (vma->vm_file) +return (unsigned long ) -EINVAL ; +/* all checks complete - do it */ - vma->vm_end = vma->vm_start + new_len; +if (new_len < old_len) { +/* shrink only happens addr + new_len and old_len are in different pages */ +VMA_ITERATOR (vmi, current->mm, addr); +int ret; + +/* vmi_shrink_vma() needs from/to pointers to be removed, +* (mainly used in munmap) so, specify them. +*/ + ret =vmi_shrink_vma (&vmi, vma, addr + new_len, addr + old_len); +if (ret <0 ) +return (unsigned long ) ret; + }else { +/* when there are no shrink, update vma. */ + vma->vm_end = vma->vm_start + new_len; + }return vma->vm_start; }
Signature · 1 line
2.43.0
Quoted 16 lines · Hajime Tazaki
When shrinking a VMA via mremap, the bounds are modified directly:
mm/nommu.c:do_mremap() { ... vma->vm_end = vma->vm_start + new_len; ... }This shrink the VMA without updating its bounds in the maple tree. If the maple tree (
mm->mm_mt) still contains the old bounds, a user process could access the freed portion. The stale maple tree would incorrectly return the shrunk VMA for an address past its newvm_end.This commit fixes this issue by calling
vmi_shrink_vma()when shrink happens. Additionally, if a file-backed map is to be shrunk, it reports-EINVALlikedo_munmap()does.
Quoted 5 lines
The issue is reported by Sashiko review, linked below.
I think the original issue reported by Sashiko is addressed, but found another existing issue, which might take a bit to look into detail (e.g., currently under nommu we cannot map (actually kernel_read()) /dev/zero, vma_set_anonymous() isn't called for /dev/zero on nommu, etc).
So I will address this in a different patch, not in this one.
-- Hajime
Lorenzo Stoakes
maintainerQuoted 25 lines · Hajime Tazaki
Quoted 16 lines · Hajime Tazaki
When shrinking a VMA via mremap, the bounds are modified directly:
mm/nommu.c:do_mremap() { ... vma->vm_end = vma->vm_start + new_len; ... }This shrink the VMA without updating its bounds in the maple tree. If the maple tree (
mm->mm_mt) still contains the old bounds, a user process could access the freed portion. The stale maple tree would incorrectly return the shrunk VMA for an address past its newvm_end.This commit fixes this issue by calling
vmi_shrink_vma()when shrink happens. Additionally, if a file-backed map is to be shrunk, it reports-EINVALlikedo_munmap()does.Quoted 5 lines
The issue is reported by Sashiko review, linked below.
Quoted 7 lines
I think the original issue reported by Sashiko is addressed, but found another existing issue, which might take a bit to look into detail (e.g., currently under nommu we cannot map (actually
kernel_read())/dev/zero,vma_set_anonymous()isn't called for/dev/zeroon nommu, etc).So I will address this in a different patch, not in this one.
OK thanks sorry for not getting to these quicker, between jobs atm and my inbox is an ongoing disaster :)
Quoted 2 lines
-- Hajime
Thanks, Lorenzo