| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20180709102825.GA21147@hori1.linux.bs1.fc.nec.co.jp>
Date: Mon, 9 Jul 2018 10:28:26 +0000
From: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
To: 裘稀石(稀石) <xishi.qiuxishi@...baba-inc.com>
CC: linux-mm <linux-mm@...ck.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
陈义全 <zy.zhengyi@...baba-inc.com>
Subject: Re: Re：[RFC] a question about reuse hwpoison page in soft_offline_page()
On Mon, Jul 09, 2018 at 01:43:35PM +0800, 裘稀石(稀石) wrote:
> Hi Naoya,
>
> Shall we fix this path too? It also will set hwpoison before
> dissolve_free_huge_page().
>
> soft_offline_huge_page
> migrate_pages
> unmap_and_move_huge_page
> if (reason == MR_MEMORY_FAILURE && !test_set_page_hwpoison(hpage))
> dissolve_free_huge_page
Thank you Xishi, I added it to the current (still draft) version below.
I start feeling that current code is broken about behavior of PageHWPoison
(at least) in soft offline. And this patch might not cover all of the issues.
My current questions/concerns are:
- does the same issue happens on soft offlining of normal pages?
- does hard offling of free (huge) page have the similar issue?
I'll try to clarify these next and will update the patch if necessary.
I'm happy if I get some comment around these.
Thanks,
Naoya Horiguchi
---
From 9ce4df899f4c859001571958be6a281cdaf5a58f Mon Sep 17 00:00:00 2001
From: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
Date: Mon, 9 Jul 2018 13:07:46 +0900
Subject: [PATCH] mm: fix race on soft-offlining free huge pages
There's a race condition between soft offline and hugetlb_fault which
causes unexpected process killing and/or hugetlb allocation failure.
The process killing is caused by the following flow:
CPU 0 CPU 1 CPU 2
soft offline
get_any_page
// find the hugetlb is free
mmap a hugetlb file
page fault
...
hugetlb_fault
hugetlb_no_page
alloc_huge_page
// succeed
soft_offline_free_page
// set hwpoison flag
mmap the hugetlb file
page fault
...
hugetlb_fault
hugetlb_no_page
find_lock_page
return VM_FAULT_HWPOISON
mm_fault_error
do_sigbus
// kill the process
The hugetlb allocation failure comes from the following flow:
CPU 0 CPU 1
mmap a hugetlb file
// reserve all free page but don't fault-in
soft offline
get_any_page
// find the hugetlb is free
soft_offline_free_page
// set hwpoison flag
dissolve_free_huge_page
// fail because all free hugepages are reserved
page fault
...
hugetlb_fault
hugetlb_no_page
alloc_huge_page
...
dequeue_huge_page_node_exact
// ignore hwpoisoned hugepage
// and finally fail due to no-mem
The root cause of this is that current soft-offline code is written
based on an assumption that PageHWPoison flag should beset at first to
avoid accessing the corrupted data. This makes sense for memory_failure()
or hard offline, but does not for soft offline because soft offline is
not about corrected error and is safe from data lost.
This patch changes soft offline semantics where it sets PageHWPoison flag
only after containment of the error page completes succesfully.
Reported-by: Xishi Qiu <xishi.qiuxishi@...baba-inc.com>
Suggested-by: Xishi Qiu <xishi.qiuxishi@...baba-inc.com>
Signed-off-by: Naoya Horiguchi <n-horiguchi@...jp.nec.com>
---
mm/hugetlb.c | 11 +++++------
mm/memory-failure.c | 13 +++++++------
mm/migrate.c | 2 --
3 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index d34225c1cb5b..3c9ce4c05f1b 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1479,22 +1479,20 @@ static int free_pool_huge_page(struct hstate *h, nodemask_t *nodes_allowed,
/*
* Dissolve a given free hugepage into free buddy pages. This function does
* nothing for in-use (including surplus) hugepages. Returns -EBUSY if the
- * number of free hugepages would be reduced below the number of reserved
- * hugepages.
+ * dissolution fails because a give page is not a free hugepage, or because
+ * free hugepages are fully reserved.
*/
int dissolve_free_huge_page(struct page *page)
{
- int rc = 0;
+ int rc = -EBUSY;
spin_lock(&hugetlb_lock);
if (PageHuge(page) && !page_count(page)) {
struct page *head = compound_head(page);
struct hstate *h = page_hstate(head);
int nid = page_to_nid(head);
- if (h->free_huge_pages - h->resv_huge_pages == 0) {
- rc = -EBUSY;
+ if (h->free_huge_pages - h->resv_huge_pages == 0)
goto out;
- }
/*
* Move PageHWPoison flag from head page to the raw error page,
* which makes any subpages rather than the error page reusable.
@@ -1508,6 +1506,7 @@ int dissolve_free_huge_page(struct page *page)
h->free_huge_pages_node[nid]--;
h->max_huge_pages--;
update_and_free_page(h, head);
+ rc = 0;
}
out:
spin_unlock(&hugetlb_lock);
diff --git a/mm/memory-failure.c b/mm/memory-failure.c
index 9d142b9b86dc..7a519d947408 100644
--- a/mm/memory-failure.c
+++ b/mm/memory-failure.c
@@ -1598,8 +1598,9 @@ static int soft_offline_huge_page(struct page *page, int flags)
if (ret > 0)
ret = -EIO;
} else {
- if (PageHuge(page))
- dissolve_free_huge_page(page);
+ ret = dissolve_free_huge_page(page);
+ if (!ret)
+ num_poisoned_pages_inc();
}
return ret;
}
@@ -1715,13 +1716,13 @@ static int soft_offline_in_use_page(struct page *page, int flags)
static void soft_offline_free_page(struct page *page)
{
+ int rc = 0;
struct page *head = compound_head(page);
- if (!TestSetPageHWPoison(head)) {
+ if (PageHuge(head))
+ rc = dissolve_free_huge_page(page);
+ if (!rc && !TestSetPageHWPoison(page))
num_poisoned_pages_inc();
- if (PageHuge(head))
- dissolve_free_huge_page(page);
- }
}
/**
diff --git a/mm/migrate.c b/mm/migrate.c
index 198af4289f9b..3ae213b799a1 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1318,8 +1318,6 @@ static int unmap_and_move_huge_page(new_page_t get_new_page,
out:
if (rc != -EAGAIN)
putback_active_hugepage(hpage);
- if (reason == MR_MEMORY_FAILURE && !test_set_page_hwpoison(hpage))
- num_poisoned_pages_inc();
/*
* If migration was not successful and there's a freeing callback, use
--
2.7.4
Powered by blists - more mailing lists