Re: BUG: Bad page map in process udevd (anon_vma: (null)) in 2.6.38-rc4

[Linus Torvalds]

On Thu, Feb 17, 2011 at 9:07 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> [ Btw, that also shows another problem: "list_move()" doesn't trigger
> the debugging checks when it does the __list_del(). So
> CONFIG_DEBUG_LIST would never have caught the fact that the
> "list_move()" was done on a list-entry that didn't have valid list
> pointers any more. ]

Ok, so does this patch change things? IOW, if you enable
CONFIG_DEBUG_LIST, this patch should hopefully make the error case of
using "list_move()" on a stale and re-used entry trigger an error
printout.

NOTE! Even if the list is some stale entry on the stack, if nothing
has overwritten that stack entry, no amount of list debugging will
notice this. So you still need to hit the problem. But now the kernel
should print stuff out even if the page got re-allocated to something
else than a page table, so _if_ the problem is a list_move() or
similar, we don't need to hit quite the same very special case. If it
corrupts user space pages or some other random memory, it will still
complain (instead of just resulting in a SIGSEGV or whatever)

Of course, there is absolutely no guarantee that it's actually
"list_move()" at all.

And as usual, the patch is TOTALLY UNTESTED.

                                 Linus
 include/linux/list.h |   12 +++++++++---
 lib/list_debug.c     |   39 ++++++++++++++++++++++++++-------------
 2 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/include/linux/list.h b/include/linux/list.h
index 9a5f8a7..3a54266 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -96,6 +96,11 @@ static inline void __list_del(struct list_head * prev, struct list_head * next)
  * in an undefined state.
  */
 #ifndef CONFIG_DEBUG_LIST
+static inline void __list_del_entry(struct list_head *entry)
+{
+	__list_del(entry->prev, entry->next);
+}
+
 static inline void list_del(struct list_head *entry)
 {
 	__list_del(entry->prev, entry->next);
@@ -103,6 +108,7 @@ static inline void list_del(struct list_head *entry)
 	entry->prev = LIST_POISON2;
 }
 #else
+extern void __list_del_entry(struct list_head *entry);
 extern void list_del(struct list_head *entry);
 #endif
 
@@ -135,7 +141,7 @@ static inline void list_replace_init(struct list_head *old,
  */
 static inline void list_del_init(struct list_head *entry)
 {
-	__list_del(entry->prev, entry->next);
+	__list_del_entry(entry);
 	INIT_LIST_HEAD(entry);
 }
 
@@ -146,7 +152,7 @@ static inline void list_del_init(struct list_head *entry)
  */
 static inline void list_move(struct list_head *list, struct list_head *head)
 {
-	__list_del(list->prev, list->next);
+	__list_del_entry(list);
 	list_add(list, head);
 }
 
@@ -158,7 +164,7 @@ static inline void list_move(struct list_head *list, struct list_head *head)
 static inline void list_move_tail(struct list_head *list,
 				  struct list_head *head)
 {
-	__list_del(list->prev, list->next);
+	__list_del_entry(list);
 	list_add_tail(list, head);
 }
 
diff --git a/lib/list_debug.c b/lib/list_debug.c
index 344c710..b8029a5 100644
--- a/lib/list_debug.c
+++ b/lib/list_debug.c
@@ -35,6 +35,31 @@ void __list_add(struct list_head *new,
 }
 EXPORT_SYMBOL(__list_add);
 
+void __list_del_entry(struct list_head *entry)
+{
+	struct list_head *prev, *next;
+
+	prev = entry->prev;
+	next = entry->next;
+
+	if (WARN(next == LIST_POISON1,
+		"list_del corruption, %p->next is LIST_POISON1 (%p)\n",
+		entry, LIST_POISON1) ||
+	    WARN(prev == LIST_POISON2,
+		"list_del corruption, %p->prev is LIST_POISON2 (%p)\n",
+		entry, LIST_POISON2) ||
+	    WARN(prev->next != entry,
+		"list_del corruption. prev->next should be %p, "
+		"but was %p\n", entry, prev->next) ||
+	    WARN(next->prev != entry,
+		"list_del corruption. next->prev should be %p, "
+		"but was %p\n", entry, next->prev))
+		return;
+
+	__list_del(prev, next);
+}
+EXPORT_SYMBOL(__list_del_entry);
+
 /**
  * list_del - deletes entry from list.
  * @entry: the element to delete from the list.
@@ -43,19 +68,7 @@ EXPORT_SYMBOL(__list_add);
  */
 void list_del(struct list_head *entry)
 {
-	WARN(entry->next == LIST_POISON1,
-		"list_del corruption, next is LIST_POISON1 (%p)\n",
-		LIST_POISON1);
-	WARN(entry->next != LIST_POISON1 && entry->prev == LIST_POISON2,
-		"list_del corruption, prev is LIST_POISON2 (%p)\n",
-		LIST_POISON2);
-	WARN(entry->prev->next != entry,
-		"list_del corruption. prev->next should be %p, "
-		"but was %p\n", entry, entry->prev->next);
-	WARN(entry->next->prev != entry,
-		"list_del corruption. next->prev should be %p, "
-		"but was %p\n", entry, entry->next->prev);
-	__list_del(entry->prev, entry->next);
+	__list_del_entry(entry);
 	entry->next = LIST_POISON1;
 	entry->prev = LIST_POISON2;
 }