tjh.dev / kernel
Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

mm: unconditionally close VMAs on error

Incorrect invocation of VMA callbacks when the VMA is no longer in a
consistent state is bug prone and risky to perform.

With regards to the important vm_ops->close() callback We have gone to
great lengths to try to track whether or not we ought to close VMAs.

Rather than doing so and risking making a mistake somewhere, instead
unconditionally close and reset vma->vm_ops to an empty dummy operations
set with a NULL .close operator.

We introduce a new function to do so - vma_close() - and simplify existing
vms logic which tracked whether we needed to close or not.

This simplifies the logic, avoids incorrect double-calling of the .close()
callback and allows us to update error paths to simply call vma_close()
unconditionally - making VMA closure idempotent.

Link: https://lkml.kernel.org/r/28e89dda96f68c505cb6f8e9fc9b57c3e9f74b42.1730224667.git.lorenzo.stoakes@oracle.com
Fixes: deb0f6562884 ("mm/mmap: undo ->mmap() when arch_validate_flags() fails")
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Reported-by: Jann Horn <jannh@google.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Reviewed-by: Liam R. Howlett <Liam.Howlett@oracle.com>
Reviewed-by: Jann Horn <jannh@google.com>
Cc: Andreas Larsson <andreas@gaisler.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Helge Deller <deller@gmx.de>
Cc: James E.J. Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Will Deacon <will@kernel.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

authored by

Lorenzo Stoakes and committed by
Andrew Morton 4080ef15 3dd6ed34

+27 -17
+18
mm/internal.h
··· 135 135 return err; 136 136 } 137 137 138 + /* 139 + * If the VMA has a close hook then close it, and since closing it might leave 140 + * it in an inconsistent state which makes the use of any hooks suspect, clear 141 + * them down by installing dummy empty hooks. 142 + */ 143 + static inline void vma_close(struct vm_area_struct *vma) 144 + { 145 + if (vma->vm_ops && vma->vm_ops->close) { 146 + vma->vm_ops->close(vma); 147 + 148 + /* 149 + * The mapping is in an inconsistent state, and no further hooks 150 + * may be invoked upon it. 151 + */ 152 + vma->vm_ops = &vma_dummy_vm_ops; 153 + } 154 + } 155 + 138 156 #ifdef CONFIG_MMU 139 157 140 158 /* Flags for folio_pte_batch(). */
+2 -3
mm/mmap.c
··· 1573 1573 return addr; 1574 1574 1575 1575 close_and_free_vma: 1576 - if (file && !vms.closed_vm_ops && vma->vm_ops && vma->vm_ops->close) 1577 - vma->vm_ops->close(vma); 1576 + vma_close(vma); 1578 1577 1579 1578 if (file || vma->vm_file) { 1580 1579 unmap_and_free_vma: ··· 1933 1934 do { 1934 1935 if (vma->vm_flags & VM_ACCOUNT) 1935 1936 nr_accounted += vma_pages(vma); 1936 - remove_vma(vma, /* unreachable = */ true, /* closed = */ false); 1937 + remove_vma(vma, /* unreachable = */ true); 1937 1938 count++; 1938 1939 cond_resched(); 1939 1940 vma = vma_next(&vmi);
+1 -2
mm/nommu.c
··· 589 589 */ 590 590 static void delete_vma(struct mm_struct *mm, struct vm_area_struct *vma) 591 591 { 592 - if (vma->vm_ops && vma->vm_ops->close) 593 - vma->vm_ops->close(vma); 592 + vma_close(vma); 594 593 if (vma->vm_file) 595 594 fput(vma->vm_file); 596 595 put_nommu_region(vma->vm_region);
+5 -9
mm/vma.c
··· 323 323 /* 324 324 * Close a vm structure and free it. 325 325 */ 326 - void remove_vma(struct vm_area_struct *vma, bool unreachable, bool closed) 326 + void remove_vma(struct vm_area_struct *vma, bool unreachable) 327 327 { 328 328 might_sleep(); 329 - if (!closed && vma->vm_ops && vma->vm_ops->close) 330 - vma->vm_ops->close(vma); 329 + vma_close(vma); 331 330 if (vma->vm_file) 332 331 fput(vma->vm_file); 333 332 mpol_put(vma_policy(vma)); ··· 1114 1115 vms_clear_ptes(vms, mas_detach, true); 1115 1116 mas_set(mas_detach, 0); 1116 1117 mas_for_each(mas_detach, vma, ULONG_MAX) 1117 - if (vma->vm_ops && vma->vm_ops->close) 1118 - vma->vm_ops->close(vma); 1119 - vms->closed_vm_ops = true; 1118 + vma_close(vma); 1120 1119 } 1121 1120 1122 1121 /* ··· 1157 1160 /* Remove and clean up vmas */ 1158 1161 mas_set(mas_detach, 0); 1159 1162 mas_for_each(mas_detach, vma, ULONG_MAX) 1160 - remove_vma(vma, /* = */ false, vms->closed_vm_ops); 1163 + remove_vma(vma, /* unreachable = */ false); 1161 1164 1162 1165 vm_unacct_memory(vms->nr_accounted); 1163 1166 validate_mm(mm); ··· 1681 1684 return new_vma; 1682 1685 1683 1686 out_vma_link: 1684 - if (new_vma->vm_ops && new_vma->vm_ops->close) 1685 - new_vma->vm_ops->close(new_vma); 1687 + vma_close(new_vma); 1686 1688 1687 1689 if (new_vma->vm_file) 1688 1690 fput(new_vma->vm_file);
+1 -3
mm/vma.h
··· 42 42 int vma_count; /* Number of vmas that will be removed */ 43 43 bool unlock; /* Unlock after the munmap */ 44 44 bool clear_ptes; /* If there are outstanding PTE to be cleared */ 45 - bool closed_vm_ops; /* call_mmap() was encountered, so vmas may be closed */ 46 45 /* 1 byte hole */ 47 46 unsigned long nr_pages; /* Number of pages being removed */ 48 47 unsigned long locked_vm; /* Number of locked pages */ ··· 197 198 vms->unmap_start = FIRST_USER_ADDRESS; 198 199 vms->unmap_end = USER_PGTABLES_CEILING; 199 200 vms->clear_ptes = false; 200 - vms->closed_vm_ops = false; 201 201 } 202 202 #endif 203 203 ··· 267 269 unsigned long start, size_t len, struct list_head *uf, 268 270 bool unlock); 269 271 270 - void remove_vma(struct vm_area_struct *vma, bool unreachable, bool closed); 272 + void remove_vma(struct vm_area_struct *vma, bool unreachable); 271 273 272 274 void unmap_region(struct ma_state *mas, struct vm_area_struct *vma, 273 275 struct vm_area_struct *prev, struct vm_area_struct *next);