fix for munmap/truncate races

Background: truncate() ends up going through the shared mappings
of file being truncated (under ->i_mmap_mutex, to protect them from getting removed while we do that) and calling unmap_vmas() on them, with range passed to unmap_vmas() sitting entirely within the vma being passed to it. The trouble is, unmap_vmas() expects a chain of vmas. It will look into the next vma, see that it's beyond the range we'd been given and do nothing to it. Fine, except that there's nothing to protect that next vma from being removed just as we do that - we do *not* hold ->i_mmap and ->i_mmap_mutex held on our file won't do anything to mappings that have nothing to do with the file in question.

There's an obvious way to deal with that - introducing a variant
of unmap_vmas() that would handle a single vma and switch these callers of unmap_vmas() to using it. It requires some preparations; below is the combined diff, for those who prefer to review the splitup, it is in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #vm

 include/linux/mm.h |    4 +-
 mm/memory.c        |  133 +++++++++++++++++++++++++++++++---------------------
 mm/mmap.c          |    5 +-
 3 files changed, 84 insertions(+), 58 deletions(-)

index 17b27cd..b5bb54d 100644
@@ -893,9 +893,9 @@ struct page *vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
 
 int zap_vma_ptes(struct vm_area_struct *vma, unsigned long address,
 		unsigned long size);
-unsigned long zap_page_range(struct vm_area_struct *vma, unsigned long address,
+void zap_page_range(struct vm_area_struct *vma, unsigned long address,
 		unsigned long size, struct zap_details *);
-unsigned long unmap_vmas(struct mmu_gather *tlb,
+void unmap_vmas(struct mmu_gather *tlb,
 		struct vm_area_struct *start_vma, unsigned long start_addr,
 		unsigned long end_addr, unsigned long *nr_accounted,
 		struct zap_details *);

index da15a79..9365a8f 100644
@@ -2224,7 +2224,6 @@ void exit_mmap(struct mm_struct *mm)
 	struct mmu_gather tlb;
 	struct vm_area_struct *vma;
 	unsigned long nr_accounted = 0;
-	unsigned long end;
 
 	/* mm's last user has gone, and its about to be pulled down */
 	mmu_notifier_release(mm);

@@ -2249,11 +2248,11 @@ void exit_mmap(struct mm_struct *mm)
 	tlb_gather_mmu(&tlb, mm, 1);
 	/* update_hiwater_rss(mm) here? but nobody should be looking */
 	/* Use -1 here to ensure all VMAs in the mm are unmapped */
-	end = unmap_vmas(&tlb, vma, 0, -1, &nr_accounted, NULL);
+	unmap_vmas(&tlb, vma, 0, -1, &nr_accounted, NULL);
 	vm_unacct_memory(nr_accounted);
 
 	free_pgtables(&tlb, vma, FIRST_USER_ADDRESS, 0);
-	tlb_finish_mmu(&tlb, 0, end);
+	tlb_finish_mmu(&tlb, 0, -1);
 
 	/*
 	 * Walk the list again, actually closing and freeing it,

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hmm. The fact that you drop the end pointer means that some architectures that optimize the TLB flushing for ranges now effectively can't do it any more.

Now, I think it's only ia64 that really is affected, but it *might* matter.

In particular, ia64 has some logic for "if you only flush one single region, you can optimize it", and the region sizes are in the terabytes. And I'm pretty sure you broke that - I'm just not entirely sure how much we care.

Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Nope - ia64 check explicitly for precisely that case:
static inline void
ia64_tlb_flush_mmu (struct mmu_gather *tlb, unsigned long start, unsigned long end)

{
        unsigned int nr;

        if (!tlb->need_flush)
                return;
        tlb->need_flush = 0;

        if (tlb->fullmm) {
                /*
                 * Tearing down the entire address space.  This happens both as a result
                 * of exit() and execve().  The latter case necessitates the call to
                 * flush_tlb_mm() here. 
                 */
                flush_tlb_mm(tlb->mm);

....
and if that condition is true, we don't even look at start or end.

[snip]
... and everything else doesn't look at start or end at all.

Ok, then I don't really care, and it certainly simplifies the calling conventions.

Linus

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

BTW, the missing part of pull request:

Shortlog:
Al Viro (6):
VM: unmap_page_range() can return void
VM: can't go through the inner loop in unmap_vmas() more than once...
VM: make zap_page_range() return void
VM: don't bother with feeding upper limit to tlb_finish_mmu() in exit_mmap()
VM: make unmap_vmas() return void
VM: make zap_page_range() callers that act on a single VMA use separate helper

Diffstat:

 include/linux/mm.h |    4 +-
 mm/memory.c        |  133 +++++++++++++++++++++++++++++++---------------------
 mm/mmap.c          |    5 +-
 3 files changed, 84 insertions(+), 58 deletions(-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>