Received: by 2002:a25:7ec1:0:0:0:0:0 with SMTP id z184csp5996088ybc; Wed, 27 Nov 2019 13:06:37 -0800 (PST) X-Google-Smtp-Source: APXvYqyctO36tPMC4LmPg6P3Z2kDa6KEwyzLtuBpS0M8CAGTyL9NfR6Gjs9FCQJMwUKJ5PsV7sSF X-Received: by 2002:a05:6402:12ca:: with SMTP id k10mr34189170edx.255.1574888797685; Wed, 27 Nov 2019 13:06:37 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1574888797; cv=none; d=google.com; s=arc-20160816; b=rI308FeC/tvtiSgpVPladXip6p4+Q7ifFcpxGbPOKGYQ8Eg56roNjrr8RV8YoBZarE 49wZJq4h/IRI4OcjnRmBkdSVQj4r4qzeexqbGhLHuBcbHw8RGTwlW9HQammBwcTCsZav V9oXspcIT0AI/Wty76wPRchdQlfyBNXGcVoUrMS3Zm7qQtCp1Q5woJnNsRymEB+PMm2u MIt84G8k0VA+SArvZ8syjT26rk2OuLWwyWPIr4J12Vxet445E0rxEvwE3pGUpqvtWCpk ba2/WROLrhVGnPj7vXDf+qA0X+grCGHRgXFQxCH+a2LqeWirnGb8G08ByCO7C0sFjqGr wcPw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=f02RpSQnQ2XpY+6v4yenzJ0YuudBcVcKaoihh/ou5nY=; b=LP98+5RICOrKVIga580/TbkSdQ5vXDX++V9GdWluSk3cTU473qKEwIEZ9bo4GwwWlg EqGn6DZhiG+7zx9s317dng815DrN+CXy8bZ1We72ipMNKOhzg+Z0Ef3wy58VxBK3K+VQ AzA4ELgdLcgvzYWU8K/5USwE/45lY8gkgJqw/GIGtlwowY6oUzQ3L4LLHX4IYejVu4Y4 SmIv0HUZfHUx4hde8FyDJSICmkTW/yZ7Ss59JOuIH6ED9N1m7zjaSHadKwcg8f2Piq3v tU+fvz9mW+CgXMNJ0WzR+2aWKYC+H0y968XR1YZlhX8U1U8Sh4xjk+bkD18uigMif7CD FnvQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b="CDp2g3/s"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id cw24si884322edb.382.2019.11.27.13.06.13; Wed, 27 Nov 2019 13:06:37 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b="CDp2g3/s"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732000AbfK0VDI (ORCPT + 99 others); Wed, 27 Nov 2019 16:03:08 -0500 Received: from mail.kernel.org ([198.145.29.99]:56200 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731995AbfK0VDH (ORCPT ); Wed, 27 Nov 2019 16:03:07 -0500 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id C22C121771; Wed, 27 Nov 2019 21:03:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1574888587; bh=RgYEdoLQTvVEYewfCzVEnGtA8NhBcnTFTDtXu/wCQdE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CDp2g3/shN+/tr78AaOVPPJsDaSYRzAUAHpOLKpTeuaJQGaZ66YQ7CLVwyUr6rDd0 3qWmhG5A3g0c22LelK1OQ65NtanAEBZg0jSvg5QZWbbckHdY8DuupL1np+H/6voOx/ g/m4WYnsw2V3AduUOvITnRKL4gWXt5WqJeWV+buQ= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Andrea Arcangeli , Aaron Tomlin , Mel Gorman , "Kirill A. Shutemov" , Jerome Glisse , Andrew Morton , Linus Torvalds , Sasha Levin Subject: [PATCH 4.19 150/306] mm: thp: fix MADV_DONTNEED vs migrate_misplaced_transhuge_page race condition Date: Wed, 27 Nov 2019 21:30:00 +0100 Message-Id: <20191127203126.514055248@linuxfoundation.org> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191127203114.766709977@linuxfoundation.org> References: <20191127203114.766709977@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Andrea Arcangeli [ Upstream commit d7c3393413fe7e7dc54498ea200ea94742d61e18 ] Patch series "migrate_misplaced_transhuge_page race conditions". Aaron found a new instance of the THP MADV_DONTNEED race against pmdp_clear_flush* variants, that was apparently left unfixed. While looking into the race found by Aaron, I may have found two more issues in migrate_misplaced_transhuge_page. These race conditions would not cause kernel instability, but they'd corrupt userland data or leave data non zero after MADV_DONTNEED. I did only minor testing, and I don't expect to be able to reproduce this (especially the lack of ->invalidate_range before migrate_page_copy, requires the latest iommu hardware or infiniband to reproduce). The last patch is noop for x86 and it needs further review from maintainers of archs that implement flush_cache_range() (not in CC yet). To avoid confusion, it's not the first patch that introduces the bug fixed in the second patch, even before removing the pmdp_huge_clear_flush_notify, that _notify suffix was called after migrate_page_copy already run. This patch (of 3): This is a corollary of ced108037c2aa ("thp: fix MADV_DONTNEED vs. numa balancing race"), 58ceeb6bec8 ("thp: fix MADV_DONTNEED vs. MADV_FREE race") and 5b7abeae3af8c ("thp: fix MADV_DONTNEED vs clear soft dirty race). When the above three fixes where posted Dave asked https://lkml.kernel.org/r/929b3844-aec2-0111-fef7-8002f9d4e2b9@intel.com but apparently this was missed. The pmdp_clear_flush* in migrate_misplaced_transhuge_page() was introduced in a54a407fbf7 ("mm: Close races between THP migration and PMD numa clearing"). The important part of such commit is only the part where the page lock is not released until the first do_huge_pmd_numa_page() finished disarming the pagenuma/protnone. The addition of pmdp_clear_flush() wasn't beneficial to such commit and there's no commentary about such an addition either. I guess the pmdp_clear_flush() in such commit was added just in case for safety, but it ended up introducing the MADV_DONTNEED race condition found by Aaron. At that point in time nobody thought of such kind of MADV_DONTNEED race conditions yet (they were fixed later) so the code may have looked more robust by adding the pmdp_clear_flush(). This specific race condition won't destabilize the kernel, but it can confuse userland because after MADV_DONTNEED the memory won't be zeroed out. This also optimizes the code and removes a superfluous TLB flush. [akpm@linux-foundation.org: reflow comment to 80 cols, fix grammar and typo (beacuse)] Link: http://lkml.kernel.org/r/20181013002430.698-2-aarcange@redhat.com Signed-off-by: Andrea Arcangeli Reported-by: Aaron Tomlin Acked-by: Mel Gorman Acked-by: Kirill A. Shutemov Cc: Jerome Glisse Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds Signed-off-by: Sasha Levin --- mm/migrate.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/mm/migrate.c b/mm/migrate.c index 0c48191a90368..4d3588c012034 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -2048,15 +2048,26 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm, entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma); /* - * Clear the old entry under pagetable lock and establish the new PTE. - * Any parallel GUP will either observe the old page blocking on the - * page lock, block on the page table lock or observe the new page. - * The SetPageUptodate on the new page and page_add_new_anon_rmap - * guarantee the copy is visible before the pagetable update. + * Overwrite the old entry under pagetable lock and establish + * the new PTE. Any parallel GUP will either observe the old + * page blocking on the page lock, block on the page table + * lock or observe the new page. The SetPageUptodate on the + * new page and page_add_new_anon_rmap guarantee the copy is + * visible before the pagetable update. */ flush_cache_range(vma, mmun_start, mmun_end); page_add_anon_rmap(new_page, vma, mmun_start, true); - pmdp_huge_clear_flush_notify(vma, mmun_start, pmd); + /* + * At this point the pmd is numa/protnone (i.e. non present) and the TLB + * has already been flushed globally. So no TLB can be currently + * caching this non present pmd mapping. There's no need to clear the + * pmd before doing set_pmd_at(), nor to flush the TLB after + * set_pmd_at(). Clearing the pmd here would introduce a race + * condition against MADV_DONTNEED, because MADV_DONTNEED only holds the + * mmap_sem for reading. If the pmd is set to NULL at any given time, + * MADV_DONTNEED won't wait on the pmd lock and it'll skip clearing this + * pmd. + */ set_pmd_at(mm, mmun_start, pmd, entry); update_mmu_cache_pmd(vma, address, &entry); @@ -2070,7 +2081,7 @@ int migrate_misplaced_transhuge_page(struct mm_struct *mm, * No need to double call mmu_notifier->invalidate_range() callback as * the above pmdp_huge_clear_flush_notify() did already call it. */ - mmu_notifier_invalidate_range_only_end(mm, mmun_start, mmun_end); + mmu_notifier_invalidate_range_end(mm, mmun_start, mmun_end); /* Take an "isolate" reference and put new page on the LRU. */ get_page(new_page); -- 2.20.1