[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 2/2] xen/mm: Fix offlining pages only make aligned buddies, fixes Xen crash



reserve_offline_pages() is missing an alignment check and thus
has a relatively high probability of growing unaligned buddies.

Fix this by checking alignment before growing spans to the next order.
Update the test case to verify the fix and prevent future regressions.

Fixes: e4865c2315 ('Page offline support in Xen side')
Signed-off-by: Bernhard Kaindl <bernhard.kaindl@xxxxxxxxxx>
---
 tools/tests/native/offline-unaligned.c | 92 --------------------------
 xen/common/page_alloc.c                |  5 ++
 2 files changed, 5 insertions(+), 92 deletions(-)

diff --git a/tools/tests/native/offline-unaligned.c 
b/tools/tests/native/offline-unaligned.c
index 1186b1763bef..593135722a3f 100644
--- a/tools/tests/native/offline-unaligned.c
+++ b/tools/tests/native/offline-unaligned.c
@@ -17,38 +17,6 @@
  *
  * Copyright (C) 2026 Cloud Software Group
  */
-#include "harness/common.h"
-
-/* test_bss_start must be first in the BSS segment */
-void __aligned(PAGE_SIZE) *test_bss_start;
-
-/* Include xen/mm.h so we can wrap page_list_del() to assert the corruption. */
-#define TEST_WRAP_XEN_INCLUDE_XEN_MM_H
-#include "harness/mm-wrapper.h"
-
-static bool expect_free_list_corruption;
-
-/*
- * Wrap page_list_del() to not fail the test by virtue of the prepared
- * free list state but continue the test like a running Xen instance
- * would in many cases. Assert and expect the corruption, and continue.
- */
-static inline void wrap_page_list_del(struct page_info *page,
-                                      struct page_list_head *head)
-{
-    printf("page_list_del: page MFN %lu, order %u\n",
-           mfn_x(page_to_mfn(page)), PFN_ORDER(page));
-
-    if ( expect_free_list_corruption )
-        EXPECT_FAIL_BEGIN();
-    CHECK(page->list.next && page->list.prev, "The free list is corrupt now!");
-    if ( expect_free_list_corruption )
-        EXPECT_FAIL_END(1);
-
-    if ( page->list.next && page->list.prev )
-        page_list_del(page, head);
-}
-#define page_list_del(page, head) wrap_page_list_del(page, head)
 
 /*
  * Include the main test library that sets up scenarios, asserts
@@ -84,78 +52,18 @@ static void test_unaligned_buddy_merge(int start_mfn)
      * | offlined page | single page     |    head page with a tail page    |
      * +---------------+-----------------+-----------------+----------------+
      */
-    EXPECT_FAIL_BEGIN();
-    /*
-     * Due to a bug in reserve_offlined_page(), we get an unaligned buddy:
-     * +---------------+-----------------+-----------------+----------------+
-     * | offlined page |     head page with a tail page    | single page    |
-     * +---------------+-----------------+-----------------+----------------+
-     */
     CHECK(page_aligned(pg + 1), "The buddy #%lu is not aligned to order-%d",
           mfn_x(page_to_mfn(pg + 1)), PFN_ORDER(pg + 1));
-    EXPECT_FAIL_END(1);
 
     /* Allocate and free a page to trigger buddy merging on free. */
-
-    /*
-     * After allocating and freeing MFN 7, we get a double-freed MFN 6 due
-     * to aligned predecessor merging in free_heap_pages():
-     *
-     *         MFN 4             MFN 5             MFN 6            MFN 7
-     *   +---------------+-----------------+-----------------+
-     *   | offlined page |    head page         tail page    |
-     *   |               |       Unaligned buddies are       |
-     *   |               |      an invariant violation!      |
-     *   +---------------+-----------------+-----------------+----------------+
-     *                                     |    head page        tail page    |
-     *                                     +-----------------+----------------+
-     */
-    expect_free_list_corruption = true;
     free_domheap_pages(alloc_domheap_pages(dom1, order0, 0), order0);
-
-    /*
-     * At this point, the free list is already corrupt. In free_heap_pages(),
-     * the tail of the unaligned buddy was added to the free list a 2nd time
-     * as the page of an overlapping aligned buddy. This is per design of the
-     * algorithm: These pages are free and thus the merging occurs as expected.
-     *
-     * The next allocation allocates the tail of the unaligned buddy, which
-     * is now, due to the merge, also the head of the new aligned buddy.
-     */
     CHECK((pg = alloc_domheap_pages(dom1, order1, 0)), "Alloc the order-1 pg");
 
     /* Inspect the predecessor (pg is the tail of the unaligned buddy) */
-    EXPECT_FAIL_BEGIN();
-    /*
-     * After allocating two more pages, MFN 6 is free AND in-use:
-     *
-     *         MFN 4             MFN 5             MFN 6            MFN 7
-     *   +---------------+-----------------+-----------------+
-     *   | offlined page |    head page         tail page    |
-     *   +---------------+-----------------+-----------------+----------------+
-     *                                     |    in-use page      in-use page  |
-     *                                     +-----------------+----------------+
-     */
     CHECK(page_aligned(pg - 1), "The buddy #%lu is not aligned to order-%d!",
           mfn_x(page_to_mfn(pg - 1)), PFN_ORDER(pg - 1));
-    EXPECT_FAIL_END(1);
 
     /* Allocate the remaining page; a clean heap should not hit BUG(). */
-    testcase_assert_expect_to_hit_bug = true;
-    /*
-     * As described above, if pg is the tail of an unaligned order-1 buddy,
-     * the unaligned buddy is still on the free list and this allocation will
-     * remove it from the free list and check alloc_heap_pages() checks the
-     * buddies to have a reference count of zero, and the already allocated
-     * page is returned as the tail of the unaligned buddy, causing the BUG().
-     *
-     *         MFN 4             MFN 5             MFN 6            MFN 7
-     *   +---------------+-----------------+-----------------+
-     *   | offlined page |    head page         tail page    | <- panic's Xen
-     *   +---------------+-----------------+-----------------+----------------+
-     *                                     |    in-use page      in-use page  |
-     *                                     +-----------------+----------------+
-     */
     alloc_domheap_pages(dom1, order0, 0); /* Triggers BUG() */
 }
 
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 46c01a9fca2a..d923ae02ade0 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1434,6 +1434,11 @@ static int reserve_offlined_page(struct page_info *head)
             if ( (cur_head + (1 << next_order)) >= (head + ( 1 << head_order)) 
)
                 goto merge;
 
+            /* Do not grow to next_order if cur_head is not aligned to it. */
+            if ( (mfn_x(page_to_mfn(cur_head)) & ((1UL << next_order) - 1)) )
+                goto merge;
+
+            /* Check if any page in the next_order range is offlined. */
             for ( i = (1 << cur_order), pg = cur_head + (1 << cur_order );
                   i < (1 << next_order);
                   i++, pg++ )
-- 
2.39.5




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.