lkml.org 
[lkml]   [2004]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subjectanon-vma mprotect merging-extend fix
yesterday I got a crash bugreport where this BUG_ON bombed with wine.

static inline void anon_vma_page_link(struct page * page, struct
vm_area_struct
* vma,
unsigned long address)
{
unsigned long index = ((address - vma->vm_start) >> PAGE_SHIFT)
+
vma->vm_pgoff;

BUG_ON(!vma->anon_vma);
if (page->mapcount == 1) {
page->index = index;
BUG_ON(page->mapping);
page->mapping = (struct address_space *) vma->anon_vma;
} else {
BUG_ON(vma->anon_vma != (anon_vma_t *) page->mapping ||
index != page->index);
^^^^^^^^^^^^^^^^^^^^^^^
}
}

The problem is that I was not propagating the anon_vma correctly in the
extend case of mprotect-merging.

This fixes the problem:

--- sles/include/linux/objrmap.h.~1~ 2004-06-17 19:25:24.000000000 +0200
+++ sles/include/linux/objrmap.h 2004-06-18 03:16:29.000000000 +0200
@@ -48,6 +48,8 @@ static inline void anon_vma_unlock(struc
extern int FASTCALL(anon_vma_prepare(struct vm_area_struct * vma));
extern void FASTCALL(anon_vma_merge(struct vm_area_struct * vma,
struct vm_area_struct * vma_dying));
+extern void FASTCALL(anon_vma_merge_extend(struct vm_area_struct * prev,
+ struct vm_area_struct * next));
extern void FASTCALL(anon_vma_unlink(struct vm_area_struct * vma));
extern void FASTCALL(anon_vma_link(struct vm_area_struct * vma));
extern void FASTCALL(__anon_vma_link(struct vm_area_struct * vma));
--- sles/mm/objrmap.c.~1~ 2004-06-17 19:25:24.000000000 +0200
+++ sles/mm/objrmap.c 2004-06-18 03:32:43.000000000 +0200
@@ -302,8 +316,10 @@ void fastcall page_add_rmap(struct page
* allowed to read PG_anon outside the page_map_lock.
*/
last_anon = PageAnon(page);
- if (anon && !last_anon)
+ if (anon && !last_anon) {
+ BUG_ON(page->mapping);
SetPageAnon(page);
+ }
BUG_ON(!anon && last_anon);

if (!page->mapcount++)
@@ -798,6 +814,51 @@ void fastcall anon_vma_merge(struct vm_a
}
}

+/*
+ * This is called when _prev_ is being extended. This is needed
+ * by mprotect for example. This means prev->vm_end == next->vm_start
+ * and they both get moved to an address > next->vm_start and
+ * < next->vm_end. NOTE: it's critical that "prev" is extended.
+ * This will kernel-crash if it's "next" being extended down, this
+ * only works for prev->vm_end and next->vm_start going _up_,
+ * down not.
+ */
+void fastcall anon_vma_merge_extend(struct vm_area_struct * prev,
+ struct vm_area_struct * next)
+{
+ anon_vma_t * anon_vma = next->anon_vma;
+
+ /*
+ * If next->anon_vma is null we've nothing to do since
+ * next is being shrunk.
+ */
+ if (!anon_vma)
+ return;
+
+ /*
+ * If both prev->anon_vma != 0 and next->anon_vma != 0
+ * it means they're equal and we've nothing to do again.
+ */
+ if (prev->anon_vma) {
+ BUG_ON(prev->anon_vma != anon_vma);
+ return;
+ }
+
+ /*
+ * If we get here it means we've to propagate the anon_vma
+ * from "next" to "prev" because "prev" will now have to
+ * include pages in the previous "next" range that can
+ * point to "anon_vma". The setting of prev->anon_vma
+ * is serialized by the mmap_sem rwsem taken in write mode.
+ */
+ prev->anon_vma = anon_vma;
+
+ /* Now link the "prev" vma into the "anon_vma_head" list. */
+ spin_lock(&anon_vma->anon_vma_lock);
+ list_add(&prev->anon_vma_node, &next->anon_vma_node);
+ spin_unlock(&anon_vma->anon_vma_lock);
+}
+
void fastcall __anon_vma_link(struct vm_area_struct * vma)
{
anon_vma_t * anon_vma = vma->anon_vma;
--- sles/mm/mprotect.c.~1~ 2004-06-17 19:25:25.000000000 +0200
+++ sles/mm/mprotect.c 2004-06-18 03:39:38.608807352 +0200
@@ -135,7 +135,7 @@ mprotect_attempt_merge(struct vm_area_st

prev_pgoff = vma->vm_pgoff - ((prev->vm_end - prev->vm_start) >> PAGE_SHIFT);
file = vma->vm_file;
- if (!is_mergeable_vma(prev, file, newflags, prev_pgoff, NULL))
+ if (!vma->vm_private_data && !is_mergeable_vma(prev, file, newflags, prev_pgoff, NULL))
return 0;
if (!is_mergeable_anon_vma(prev, vma))
return 0;
@@ -215,6 +215,14 @@ mprotect_attempt_merge(struct vm_area_st
anon_vma_unlock(vma);
if (file)
up(i_shared_sem);
+
+ /*
+ * No need of any lock for this (except the internal anon_vma_lock),
+ * the whole vma->anon_vma handling is serialized enterely by the
+ * mmap_sem rwsem taken in write mode.
+ */
+ anon_vma_merge_extend(prev, vma);
+
return 1;
}

@@ -241,7 +249,7 @@ mprotect_attempt_merge_final(struct vm_a

next_pgoff = prev->vm_pgoff + ((prev->vm_end - prev->vm_start) >> PAGE_SHIFT);
file = prev->vm_file;
- if (!is_mergeable_vma(next, file, newflags, next_pgoff, NULL))
+ if (!prev->vm_private_data && !is_mergeable_vma(next, file, newflags, next_pgoff, NULL))
return;
if (!is_mergeable_anon_vma(prev, next))
return;

This adds another robustness BUG_ON to be sure we switch the PG_anon
bitflag only in unmapped pages, and secondly it makes sure not to merge
anything if vm_private_data is set on the "other" vma (the one not
checked internally in is_mergeable_vma). I considered adding another
parameter to is_mergeable_vma, but I think the above is fine too, such
construct can be copied as easily without forgetting about the "other"
vma private_data.

This is against latest 2.6-aa or sles kernel (SL9.1 is unaffected
because we didn't enable the mprotect merging there, so in short nothing
is affected because this is not a released kernel yet and I fixed this
just in time ;).

I didn't have much time to look at the status of mainline in the last
week, I assume Hugh will take care of merging this fix in a way that
will apply cleanly to mainline.

I plan to finish the fix-update for 2.4 before the weekend and to
synchronize with 2.6 early next week (sorry for being a bit out of sync
and thanks for all the help).
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 14:03    [W:0.085 / U:0.476 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site