mm: kernel BUG at mm/mempolicy.c:1203!

Hi all,

While fuzzing with trinity inside a KVM tools guest running latest -next kernel, I've stumbled on the following spew.

This seems to be due to commit 0bf598d863e "mbind: add BUG_ON(!vma) in new_vma_page()" which added that BUG_ON.

[  538.836802] kernel BUG at mm/mempolicy.c:1203!
[  538.838245] invalid opcode: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
[  538.838507] Dumping ftrace buffer:
[  538.840150]    (ftrace buffer empty)
[  538.840150] Modules linked in:
[  538.840256] CPU: 15 PID: 21136 Comm: trinity-child96 Tainted: G        W    3.13.0-rc
3-next-20131213-sasha-00010-g6749b49-dirty #4104
[  538.840256] task: ffff880e73288000 ti: ffff880e73290000 task.ti: ffff880e73290000
[  538.840256] RIP: 0010:[<ffffffff812a4092>]  [<ffffffff812a4092>] new_vma_page+0x52/0x
b0
[  538.840256] RSP: 0000:ffff880e73291d38  EFLAGS: 00010246
[  538.840256] RAX: fffffffffffffff2 RBX: 0000000000000000 RCX: 0000000000000000
[  538.840256] RDX: 0000000008040075 RSI: ffff880dc59ebe00 RDI: ffffea003e017c00
[  538.840256] RBP: ffff880e73291d58 R08: 0000000000000002 R09: 0000000000000001
[  538.840256] R10: 0000000000000001 R11: 0000000000000000 R12: fffffffffffffff2
[  538.840256] R13: ffffea003e017c00 R14: 0000000000000002 R15: 00000000fffffff4
[  538.840256] FS:  00007f43832a3700(0000) GS:ffff880fe7400000(0000) knlGS:0000000000000
000
[  538.840256] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  538.840256] CR2: 0000000002083be8 CR3: 0000000ec894e000 CR4: 00000000000006e0
[  538.840256] Stack:
[  538.840256]  0000000000000000 ffffea003e017c00 0000000000000000 ffff880e73291e58
[  538.840256]  ffff880e73291da8 ffffffff812b5664 000000000000044e 0000000000000000
[  538.840256]  ffff880e73291da8 ffffea003e017c00 ffffea003e017c40 ffff880e73291e58
[  538.840256] Call Trace:
[  538.840256]  [<ffffffff812b5664>] unmap_and_move+0x44/0x180
[  538.840256]  [<ffffffff812b588b>] migrate_pages+0xeb/0x2f0
[  538.840256]  [<ffffffff812a4040>] ? alloc_pages_vma+0x220/0x220
[  538.840256]  [<ffffffff812a4ba3>] do_mbind+0x283/0x340
[  538.840256]  [<ffffffff812794af>] ? might_fault+0x9f/0xb0
[  538.840256]  [<ffffffff81279466>] ? might_fault+0x56/0xb0
[  538.840256]  [<ffffffff81249a98>] ? context_tracking_user_exit+0xb8/0x1d0
[  538.840256]  [<ffffffff812a4ce9>] SYSC_mbind+0x89/0xb0

[  538.840256]  [<ffffffff81249b75>] ? context_tracking_user_exit+0x195/0x1d0
[  538.840256]  [<ffffffff812a4d1e>] SyS_mbind+0xe/0x10
[  538.840256]  [<ffffffff843bc489>] ia32_do_call+0x13/0x13
[  538.840256] Code: 95 58 fe ff 49 89 c4 48 83 f8 f2 75 14 48 8b 5b 10 48 85 db 75 e3 0f 1f 00 eb 
10 66 0f 1f 44 00 00 48 85 db 0f 1f 44 00 00 75 0e <0f> 0b 0f 1f 40 00 eb fe 66 0f 1f 44 00 00 4c 89 
ef e8 68 6a ff
[  538.840256] RIP  [<ffffffff812a4092>] new_vma_page+0x52/0xb0
[  538.840256]  RSP <ffff880e73291d38>

Hi all,

While fuzzing with trinity inside a KVM tools guest running latest -next kernel, I've
stumbled on the following spew.

This seems to be due to commit 0bf598d863e "mbind: add BUG_ON(!vma) in new_vma_page()"
which added that BUG_ON.

Quoted 9 lines · Sasha Levin

Hi all,

While fuzzing with trinity inside a KVM tools guest running latest -next kernel, I've
stumbled on the following spew.

This seems to be due to commit 0bf598d863e "mbind: add BUG_ON(!vma) in new_vma_page()"
which added that BUG_ON.

Could you take a try with this patch from Wanpeng Li?

Thanks,
-Bob

Subject: [PATCH] mm/mempolicy: fix !vma in new_vma_page()
....

Signed-off-by Wanpeng Li <[email protected]>

index eca4a31..73b5a35 100644

mm/mempolicy.c +8 -6

@@ -1197,14 +1197,16 @@ static struct page *new_vma_page(struct page
 *page, unsigned long private, int *
  			break;
  		vma = vma->vm_next;
  	}
+
+	if (PageHuge(page)) {
+		if (vma)
+			return alloc_huge_page_noerr(vma, address, 1);
+		else
+			return NULL;
+	}
  	/*
-	 * queue_pages_range() confirms that @page belongs to some vma,
-	 * so vma shouldn't be NULL.
+	 * if !vma, alloc_page_vma() will use task or system default policy
  	 */
-	BUG_ON(!vma);
-
-	if (PageHuge(page))
-		return alloc_huge_page_noerr(vma, address, 1);
  	return alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, address);
  }

  #else

Quoted 44 lines · Bob Liu

Quoted 9 lines · Sasha Levin

Hi all,

While fuzzing with trinity inside a KVM tools guest running latest -next kernel, I've
stumbled on the following spew.

This seems to be due to commit 0bf598d863e "mbind: add BUG_ON(!vma) in new_vma_page()"
which added that BUG_ON.

Could you take a try with this patch from Wanpeng Li?

Thanks,
-Bob

Subject: [PATCH] mm/mempolicy: fix !vma in new_vma_page()
....

Signed-off-by Wanpeng Li <[email protected]>

index eca4a31..73b5a35 100644

mm/mempolicy.c +8 -6

@@ -1197,14 +1197,16 @@ static struct page *new_vma_page(struct page
 *page, unsigned long private, int *
              break;
          vma = vma->vm_next;
      }
+
+    if (PageHuge(page)) {
+        if (vma)
+            return alloc_huge_page_noerr(vma, address, 1);
+        else
+            return NULL;
+    }
      /*
-     * queue_pages_range() confirms that @page belongs to some vma,
-     * so vma shouldn't be NULL.
+     * if !vma, alloc_page_vma() will use task or system default policy
       */
-    BUG_ON(!vma);
-
-    if (PageHuge(page))
-        return alloc_huge_page_noerr(vma, address, 1);

Quoted 4 lines

mm/mempolicy.c +0 -0

      return alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, address);
  }

  #else

Hmm... So in essence it's mostly a revert of Naoya's patch, who seemed pretty certain that this
situation shouldn't happen at all. What's the reasoning behind just

Quoted 45 lines · Sasha Levin

Quoted 44 lines · Bob Liu

Quoted 9 lines · Sasha Levin

Hi all,

While fuzzing with trinity inside a KVM tools guest running latest -next kernel, I've
stumbled on the following spew.

This seems to be due to commit 0bf598d863e "mbind: add BUG_ON(!vma) in new_vma_page()"
which added that BUG_ON.

Could you take a try with this patch from Wanpeng Li?

Thanks,
-Bob

Subject: [PATCH] mm/mempolicy: fix !vma in new_vma_page()
....

Signed-off-by Wanpeng Li <[email protected]>

index eca4a31..73b5a35 100644

mm/mempolicy.c +8 -6

@@ -1197,14 +1197,16 @@ static struct page *new_vma_page(struct page
 *page, unsigned long private, int *
              break;
          vma = vma->vm_next;
      }
+
+    if (PageHuge(page)) {
+        if (vma)
+            return alloc_huge_page_noerr(vma, address, 1);
+        else
+            return NULL;
+    }
      /*
-     * queue_pages_range() confirms that @page belongs to some vma,
-     * so vma shouldn't be NULL.
+     * if !vma, alloc_page_vma() will use task or system default policy
       */
-    BUG_ON(!vma);
-
-    if (PageHuge(page))
-        return alloc_huge_page_noerr(vma, address, 1);

Quoted 8 lines

Quoted 4 lines

mm/mempolicy.c +0 -0

      return alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, address);
  }

  #else

Hmm... So in essence it's mostly a revert of Naoya's patch, who seemed pretty certain that this
situation shouldn't happen at all. What's the reasoning behind just

I think this assumption may not correct.
Even if
address = __vma_address(page, vma);
and
vma->start < address < vma->end;
page_address_in_vma() may still return -EFAULT because of many other conditions in it.
As a result the while loop in new_vma_page() may end with vma=NULL.

Naoya, any idea?

Hello Bob,

Quoted 64 lines · Bob Liu

Quoted 45 lines · Sasha Levin

Quoted 44 lines · Bob Liu

Quoted 9 lines · Sasha Levin

Hi all,

While fuzzing with trinity inside a KVM tools guest running latest -next kernel, I've
stumbled on the following spew.

This seems to be due to commit 0bf598d863e "mbind: add BUG_ON(!vma) in new_vma_page()"
which added that BUG_ON.

Could you take a try with this patch from Wanpeng Li?

Thanks,
-Bob

Subject: [PATCH] mm/mempolicy: fix !vma in new_vma_page()
....

Signed-off-by Wanpeng Li <[email protected]>

index eca4a31..73b5a35 100644

mm/mempolicy.c +8 -6

@@ -1197,14 +1197,16 @@ static struct page *new_vma_page(struct page
 *page, unsigned long private, int *
              break;
          vma = vma->vm_next;
      }
+
+    if (PageHuge(page)) {
+        if (vma)
+            return alloc_huge_page_noerr(vma, address, 1);
+        else
+            return NULL;
+    }
      /*
-     * queue_pages_range() confirms that @page belongs to some vma,
-     * so vma shouldn't be NULL.
+     * if !vma, alloc_page_vma() will use task or system default policy
       */
-    BUG_ON(!vma);
-
-    if (PageHuge(page))
-        return alloc_huge_page_noerr(vma, address, 1);

Quoted 8 lines

Quoted 4 lines

mm/mempolicy.c +0 -0

      return alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, address);
  }

  #else

Hmm... So in essence it's mostly a revert of Naoya's patch, who seemed pretty certain that this
situation shouldn't happen at all. What's the reasoning behind just

I think this assumption may not correct.
Even if
address = __vma_address(page, vma);
and
vma->start < address < vma->end;
page_address_in_vma() may still return -EFAULT because of many other conditions in it.
As a result the while loop in new_vma_page() may end with vma=NULL.

Quoted 1 line

Naoya, any idea?

Yes, you totally make sense. So please apply Wanpeng's patch.

Thanks,
Naoya Horiguchi

--
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/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hello Bob,

Quoted 64 lines · Bob Liu

Quoted 45 lines · Sasha Levin

Quoted 44 lines · Bob Liu

Quoted 9 lines · Sasha Levin

Hi all,

While fuzzing with trinity inside a KVM tools guest running latest -next kernel, I've
stumbled on the following spew.

This seems to be due to commit 0bf598d863e "mbind: add BUG_ON(!vma) in new_vma_page()"
which added that BUG_ON.

Could you take a try with this patch from Wanpeng Li?

Thanks,
-Bob

Subject: [PATCH] mm/mempolicy: fix !vma in new_vma_page()
....

Signed-off-by Wanpeng Li <[email protected]>

index eca4a31..73b5a35 100644

mm/mempolicy.c +8 -6

@@ -1197,14 +1197,16 @@ static struct page *new_vma_page(struct page
 *page, unsigned long private, int *
               break;
           vma = vma->vm_next;
       }
+
+    if (PageHuge(page)) {
+        if (vma)
+            return alloc_huge_page_noerr(vma, address, 1);
+        else
+            return NULL;
+    }
       /*
-     * queue_pages_range() confirms that @page belongs to some vma,
-     * so vma shouldn't be NULL.
+     * if !vma, alloc_page_vma() will use task or system default policy
        */
-    BUG_ON(!vma);
-
-    if (PageHuge(page))
-        return alloc_huge_page_noerr(vma, address, 1);

Quoted 8 lines

Quoted 4 lines

mm/mempolicy.c +0 -0

       return alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, address);
   }

   #else

Hmm... So in essence it's mostly a revert of Naoya's patch, who seemed pretty certain that this
situation shouldn't happen at all. What's the reasoning behind just

I think this assumption may not correct.
Even if
address = __vma_address(page, vma);
and
vma->start < address < vma->end;
page_address_in_vma() may still return -EFAULT because of many other conditions in it.
As a result the while loop in new_vma_page() may end with vma=NULL.

Quoted 1 line

Naoya, any idea?

Yes, you totally make sense. So please apply Wanpeng's patch.

Quoted 67 lines · Naoya Horiguchi

Hello Bob,

Quoted 64 lines · Bob Liu

Quoted 45 lines · Sasha Levin

Quoted 44 lines · Bob Liu

Quoted 9 lines · Sasha Levin

Hi all,

While fuzzing with trinity inside a KVM tools guest running latest -next kernel, I've
stumbled on the following spew.

This seems to be due to commit 0bf598d863e "mbind: add BUG_ON(!vma) in new_vma_page()"
which added that BUG_ON.

Could you take a try with this patch from Wanpeng Li?

Thanks,
-Bob

Subject: [PATCH] mm/mempolicy: fix !vma in new_vma_page()
....

Signed-off-by Wanpeng Li <[email protected]>

index eca4a31..73b5a35 100644

mm/mempolicy.c +8 -6

@@ -1197,14 +1197,16 @@ static struct page *new_vma_page(struct page
 *page, unsigned long private, int *
               break;
           vma = vma->vm_next;
       }
+
+    if (PageHuge(page)) {
+        if (vma)
+            return alloc_huge_page_noerr(vma, address, 1);
+        else
+            return NULL;
+    }
       /*
-     * queue_pages_range() confirms that @page belongs to some vma,
-     * so vma shouldn't be NULL.
+     * if !vma, alloc_page_vma() will use task or system default policy
        */
-    BUG_ON(!vma);
-
-    if (PageHuge(page))
-        return alloc_huge_page_noerr(vma, address, 1);

Quoted 8 lines

Quoted 4 lines

mm/mempolicy.c +0 -0

       return alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, address);
   }

   #else

Hmm... So in essence it's mostly a revert of Naoya's patch, who seemed pretty certain that this
situation shouldn't happen at all. What's the reasoning behind just

I think this assumption may not correct.
Even if
address = __vma_address(page, vma);
and
vma->start < address < vma->end;
page_address_in_vma() may still return -EFAULT because of many other conditions in it.
As a result the while loop in new_vma_page() may end with vma=NULL.

Quoted 3 lines

Quoted 1 line

Naoya, any idea?

Yes, you totally make sense. So please apply Wanpeng's patch.

Shouldn't it just be a revert of Naoya's patch? Otherwise we're changing code paths unnecessarily.

Thanks,
Sasha

--
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/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Hi Sasha,

Quoted 68 lines · Sasha Levin

Quoted 67 lines · Naoya Horiguchi

Hello Bob,

Quoted 64 lines · Bob Liu

Quoted 45 lines · Sasha Levin

Quoted 44 lines · Bob Liu

Quoted 9 lines · Sasha Levin

Hi all,

While fuzzing with trinity inside a KVM tools guest running latest -next kernel, I've
stumbled on the following spew.

This seems to be due to commit 0bf598d863e "mbind: add BUG_ON(!vma) in new_vma_page()"
which added that BUG_ON.

Could you take a try with this patch from Wanpeng Li?

Thanks,
-Bob

Subject: [PATCH] mm/mempolicy: fix !vma in new_vma_page()
....

Signed-off-by Wanpeng Li <[email protected]>

index eca4a31..73b5a35 100644

mm/mempolicy.c +8 -6

@@ -1197,14 +1197,16 @@ static struct page *new_vma_page(struct page
 *page, unsigned long private, int *
                break;
            vma = vma->vm_next;
        }
+
+    if (PageHuge(page)) {
+        if (vma)
+            return alloc_huge_page_noerr(vma, address, 1);
+        else
+            return NULL;
+    }
        /*
-     * queue_pages_range() confirms that @page belongs to some vma,
-     * so vma shouldn't be NULL.
+     * if !vma, alloc_page_vma() will use task or system default policy
         */
-    BUG_ON(!vma);
-
-    if (PageHuge(page))
-        return alloc_huge_page_noerr(vma, address, 1);

Quoted 8 lines

Quoted 4 lines

mm/mempolicy.c +0 -0

        return alloc_page_vma(GFP_HIGHUSER_MOVABLE, vma, address);
    }

    #else

Hmm... So in essence it's mostly a revert of Naoya's patch, who seemed pretty certain that this
situation shouldn't happen at all. What's the reasoning behind just

I think this assumption may not correct.
Even if
address = __vma_address(page, vma);
and
vma->start < address < vma->end;
page_address_in_vma() may still return -EFAULT because of many other conditions in it.
As a result the while loop in new_vma_page() may end with vma=NULL.

Quoted 7 lines

Quoted 3 lines

Quoted 1 line

Naoya, any idea?

Yes, you totally make sense. So please apply Wanpeng's patch.

Shouldn't it just be a revert of Naoya's patch? Otherwise we're changing code paths unnecessarily.

Actually, the original target of Naoya's patch is try to fix potential dereference NULL pointer by Dan. http://marc.info/?l=linux-mm&m=137689530323257&w=2

This patch fix both the regression and potential dereference NULL pointer reported by Dan. http://marc.info/?l=linux-kernel&m=138726268626705&w=2