From patchwork Mon Mar 14 02:13:37 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naoya Horiguchi X-Patchwork-Id: 12779495 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2DD07C433EF for ; Mon, 14 Mar 2022 02:13:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 8C9858D0002; Sun, 13 Mar 2022 22:13:54 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 851628D0001; Sun, 13 Mar 2022 22:13:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 718E38D0002; Sun, 13 Mar 2022 22:13:54 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0111.hostedemail.com [216.40.44.111]) by kanga.kvack.org (Postfix) with ESMTP id 5E5F18D0001 for ; Sun, 13 Mar 2022 22:13:54 -0400 (EDT) Received: from smtpin16.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 0B6278249980 for ; Mon, 14 Mar 2022 02:13:54 +0000 (UTC) X-FDA: 79241371188.16.BD0C708 Received: from out0.migadu.com (out0.migadu.com [94.23.1.103]) by imf15.hostedemail.com (Postfix) with ESMTP id 37F7CA001C for ; Mon, 14 Mar 2022 02:13:53 +0000 (UTC) X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1647224030; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=HmZJDaJz/YW+RAaAgW5cxomg9hhNVz7JL5wG+y4Nka8=; b=ayYPlc5reDvT7sBTmPkxVknC1FpX8Xt/V7RonQorERM1EvaMK011KX0bY/jQamw+IMsDvc MGZJDRsodrOYsoa+JAqDsWrMjo79CJBT+yL6Vrr7+dT0CPmyfl6BmdpfwCWXyIk6zkg/fb x4a3JXbZ5biE33XQqHO+QR/ROXm2270= From: Naoya Horiguchi To: linux-mm@kvack.org Cc: Andrew Morton , Mike Kravetz , Miaohe Lin , Yang Shi , Naoya Horiguchi , linux-kernel@vger.kernel.org Subject: [PATCH v2] mm/hwpoison: set PageHWPoison after taking page lock in memory_failure_hugetlb() Date: Mon, 14 Mar 2022 11:13:37 +0900 Message-Id: <20220314021337.333781-1-naoya.horiguchi@linux.dev> MIME-Version: 1.0 X-Migadu-Flow: FLOW_OUT X-Migadu-Auth-User: linux.dev X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 37F7CA001C X-Stat-Signature: kg6ja3bbw99by6hfrfw87im8y15ijk57 Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b=ayYPlc5r; spf=pass (imf15.hostedemail.com: domain of naoya.horiguchi@linux.dev designates 94.23.1.103 as permitted sender) smtp.mailfrom=naoya.horiguchi@linux.dev; dmarc=pass (policy=none) header.from=linux.dev X-Rspam-User: X-HE-Tag: 1647224033-959096 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: From: Naoya Horiguchi There is a race condition between memory_failure_hugetlb() and hugetlb free/demotion, which causes setting PageHWPoison flag on the wrong page (which was a hugetlb when memory_failure() was called, but was removed or demoted when memory_failure_hugetlb() is called). This results in killing wrong processes. So set PageHWPoison flag with holding page lock, The actual user-visible effect might be obscure because even if memory_failure() works as expected, some random process could be killed. Even worse, the actual error is left unhandled, so no one prevents later access to it, which might lead to more serious results like consuming corrupted data. This patch depends on Miaohe's patch titled "mm/memory-failure.c: fix race with changing page compound again". Reported-by: Mike Kravetz Signed-off-by: Naoya Horiguchi Cc: --- ChangeLog v1 -> v2: - pass subpage to get_hwpoison_huge_page() instead of head page. - call compound_head() in hugetlb_lock to avoid race with hugetlb demotion/free. --- mm/hugetlb.c | 8 +++++--- mm/memory-failure.c | 33 +++++++++++++++------------------ 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/mm/hugetlb.c b/mm/hugetlb.c index f294db835f4b..345fed90842e 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -6761,14 +6761,16 @@ bool isolate_huge_page(struct page *page, struct list_head *list) int get_hwpoison_huge_page(struct page *page, bool *hugetlb) { + struct page *head; int ret = 0; *hugetlb = false; spin_lock_irq(&hugetlb_lock); - if (PageHeadHuge(page)) { + head = compound_head(page); + if (PageHeadHuge(head)) { *hugetlb = true; - if (HPageFreed(page) || HPageMigratable(page)) - ret = get_page_unless_zero(page); + if (HPageFreed(head) || HPageMigratable(head)) + ret = get_page_unless_zero(head); else ret = -EBUSY; } diff --git a/mm/memory-failure.c b/mm/memory-failure.c index a9bfd04d2a3c..c40c00c3a261 100644 --- a/mm/memory-failure.c +++ b/mm/memory-failure.c @@ -1193,7 +1193,7 @@ static int __get_hwpoison_page(struct page *page, unsigned long flags) int ret = 0; bool hugetlb = false; - ret = get_hwpoison_huge_page(head, &hugetlb); + ret = get_hwpoison_huge_page(page, &hugetlb); if (hugetlb) return ret; @@ -1280,11 +1280,10 @@ static int get_any_page(struct page *p, unsigned long flags) static int __get_unpoison_page(struct page *page) { - struct page *head = compound_head(page); int ret = 0; bool hugetlb = false; - ret = get_hwpoison_huge_page(head, &hugetlb); + ret = get_hwpoison_huge_page(page, &hugetlb); if (hugetlb) return ret; @@ -1503,24 +1502,11 @@ static int memory_failure_hugetlb(unsigned long pfn, int flags) int res; unsigned long page_flags; - if (TestSetPageHWPoison(head)) { - pr_err("Memory failure: %#lx: already hardware poisoned\n", - pfn); - res = -EHWPOISON; - if (flags & MF_ACTION_REQUIRED) - res = kill_accessing_process(current, page_to_pfn(head), flags); - return res; - } - - num_poisoned_pages_inc(); - if (!(flags & MF_COUNT_INCREASED)) { res = get_hwpoison_page(p, flags); if (!res) { lock_page(head); if (hwpoison_filter(p)) { - if (TestClearPageHWPoison(head)) - num_poisoned_pages_dec(); unlock_page(head); return -EOPNOTSUPP; } @@ -1553,13 +1539,16 @@ static int memory_failure_hugetlb(unsigned long pfn, int flags) page_flags = head->flags; if (hwpoison_filter(p)) { - if (TestClearPageHWPoison(head)) - num_poisoned_pages_dec(); put_page(p); res = -EOPNOTSUPP; goto out; } + if (TestSetPageHWPoison(head)) + goto already_hwpoisoned; + + num_poisoned_pages_inc(); + /* * TODO: hwpoison for pud-sized hugetlb doesn't work right now, so * simply disable it. In order to make it work properly, we need @@ -1585,6 +1574,14 @@ static int memory_failure_hugetlb(unsigned long pfn, int flags) out: unlock_page(head); return res; +already_hwpoisoned: + put_page(p); + unlock_page(head); + pr_err("Memory failure: %#lx: already hardware poisoned\n", pfn); + res = -EHWPOISON; + if (flags & MF_ACTION_REQUIRED) + res = kill_accessing_process(current, page_to_pfn(head), flags); + return res; } static int memory_failure_dev_pagemap(unsigned long pfn, int flags,