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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:	Mon, 7 Jan 2013 17:08:15 +0000
From:	Mel Gorman <mgorman@...e.de>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Hugh Dickins <hughd@...gle.com>, Ingo Molnar <mingo@...nel.org>,
	Andrea Arcangeli <aarcange@...hat.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: [PATCH] mm: migrate: Check page_count of THP before migrating

Hugh Dickins pointed out that migrate_misplaced_transhuge_page() does not
check page_count before migrating like base page migration and khugepage. He
could not see why this was safe and he is right.

The potential impact of the bug is avoided due to the limitations of NUMA
balancing.  The page_mapcount() check ensures that only a single address
space is using this page and as THPs are typically private it should not be
possible for another address space to fault it in parallel. If the address
space has one associated task then it's difficult to have both a GUP pin
and be referencing the page at the same time. If there are multiple tasks
then a buggy scenario requires that another thread be accessing the page
while the direct IO is in flight. This is dodgy behaviour as there is
a possibility of corruption with or without THP migration. It would be
difficult to identify the corruption as being a migration bug.

While we happen to be safe for the most part it is shoddy to depend on
such "safety" so this patch checks the page count similar to anonymous
pages. Note that this does not mean that the page_mapcount() check can go
away. If we were to remove the page_mapcount() check the the THP would
have to be unmapped from all referencing PTEs, replaced with migration
PTEs and restored properly afterwards.

Reported-by: Hugh Dickins <hughd@...gle.com>
Signed-off-by: Mel Gorman <mgorman@...e.de>
---
 mm/migrate.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 3b676b0..f466827 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -1679,9 +1679,18 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm,
 	page_xchg_last_nid(new_page, page_last_nid(page));
 
 	isolated = numamigrate_isolate_page(pgdat, page);
-	if (!isolated) {
+
+	/*
+	 * Failing to isolate or a GUP pin prevents migration. The expected
+	 * page count is 2. 1 for anonymous pages without a mapping and 1
+	 * for the callers pin. If the page was isolated, the page will
+	 * need to be put back on the LRU.
+	 */
+	if (!isolated || page_count(page) != 2) {
 		count_vm_events(PGMIGRATE_FAIL, HPAGE_PMD_NR);
 		put_page(new_page);
+		if (isolated)
+			putback_lru_page(page);
 		goto out_keep_locked;
 	}
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ