From patchwork Fri Oct 13 22:58:25 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Morton X-Patchwork-Id: 115832 Delivered-To: patch@linaro.org Received: by 10.140.22.163 with SMTP id 32csp1235710qgn; Fri, 13 Oct 2017 15:58:28 -0700 (PDT) X-Google-Smtp-Source: AOwi7QCXmaDCPQ6e1FdtzThmlM3U+rZ0rXZUfYghCQ1mlSo/mezYaj1QeYC4PinBfA12PX4lrKgU X-Received: by 10.99.167.12 with SMTP id d12mr2448559pgf.414.1507935508577; Fri, 13 Oct 2017 15:58:28 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1507935508; cv=none; d=google.com; s=arc-20160816; b=cbWXotQNuPN8fBtuGaXptaW+USltdcQPID4lWaIU+ibLfAfJ5IkxHodBZSRxZSIwVW qshlfFmBTyWhg1/68S1ig+u4U+NK5w5HNsObi1GNvBumMInSqzABQTxWG1c7VXwZP7hI h+2+sXvV8c2JQT2XnTmBlpYH+t+C55c61jFqldeKDMjyAVLM+Bjjl6Nz7FYGTMYJneMK M5M5fZHn4fyqRSyLfdjcb0igg5RlnMonEGtW0n1t31EkDOkrKek6VfFhAermKUhKNPit pGHTvAljiPfciajU/4qMREubSbTxsHDbwNVrrBEwuvO328yzsNGChK/l+gh+Nj7p/+vZ hOkg== 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:message-id:subject:to:from:date :arc-authentication-results; bh=+UMK59oWB82xQ0mb7HrADgxblJBq8HmT6j6iy2qRvCM=; b=DHzoh8mFJAWF7MjtB3Iiuzvs+wcqHNCOkwZb8syxdooshMXNnbInOMEbqcge4gizVf 3X0WCEAf6zJfOQyXlF2QCeX+uP1AP9ovR5kLxZ3VMbaiH0HcA3WNuE/jwbbBumYUp1Kv DTgmlH4f+/iIelTbwwuDCowarv0hXusG8qgN2pfnGEANkmJ4PLA2xz+Sia4jvaYODZ2g QKlTocDy27AjE8R/ZavoZXjXdR+SP9+Fr8pG+M+aA2vAfw/dCfopWu/dBaLR41nnT/w/ B/WAMnDsZIPNiReOCq/vDsd7/WFTmYFgYF8fM2lblqGsdsFceg/5ZUkIunQV3ZO3QYSZ pfmg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-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 a65si1090525pgc.699.2017.10.13.15.58.28; Fri, 13 Oct 2017 15:58:28 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of stable-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=stable-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753135AbdJMW61 (ORCPT + 9 others); Fri, 13 Oct 2017 18:58:27 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:40284 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753089AbdJMW61 (ORCPT ); Fri, 13 Oct 2017 18:58:27 -0400 Received: from akpm3.svl.corp.google.com (unknown [104.133.9.92]) by mail.linuxfoundation.org (Postfix) with ESMTPSA id 62AF0ACB; Fri, 13 Oct 2017 22:58:26 +0000 (UTC) Date: Fri, 13 Oct 2017 15:58:25 -0700 From: akpm@linux-foundation.org To: torvalds@linux-foundation.org, mm-commits@vger.kernel.org, akpm@linux-foundation.org, will.deacon@arm.com, kirill.shutemov@linux.intel.com, paulmck@linux.vnet.ibm.com, peterz@infradead.org, rruigrok@codeaurora.org, stable@vger.kernel.org, ynorov@caviumnetworks.com Subject: [patch 17/18] mm: page_vma_mapped: ensure pmd is loaded with READ_ONCE outside of lock Message-ID: <59e14511.eLRptBeL+6bECu9A%akpm@linux-foundation.org> User-Agent: Heirloom mailx 12.5 6/20/10 MIME-Version: 1.0 Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org From: Will Deacon Subject: mm: page_vma_mapped: ensure pmd is loaded with READ_ONCE outside of lock Loading the pmd without holding the pmd_lock exposes us to races with concurrent updaters of the page tables but, worse still, it also allows the compiler to cache the pmd value in a register and reuse it later on, even if we've performed a READ_ONCE in between and seen a more recent value. In the case of page_vma_mapped_walk, this leads to the following crash when the pmd loaded for the initial pmd_trans_huge check is all zeroes and a subsequent valid table entry is loaded by check_pmd. We then proceed into map_pte, but the compiler re-uses the zero entry inside pte_offset_map, resulting in a junk pointer being installed in pvmw->pte: [ 254.032812] PC is at check_pte+0x20/0x170 [ 254.032948] LR is at page_vma_mapped_walk+0x2e0/0x540 [...] [ 254.036114] Process doio (pid: 2463, stack limit = 0xffff00000f2e8000) [ 254.036361] Call trace: [ 254.038977] [] check_pte+0x20/0x170 [ 254.039137] [] page_vma_mapped_walk+0x2e0/0x540 [ 254.039332] [] page_mkclean_one+0xac/0x278 [ 254.039489] [] rmap_walk_file+0xf0/0x238 [ 254.039642] [] rmap_walk+0x64/0xa0 [ 254.039784] [] page_mkclean+0x90/0xa8 [ 254.040029] [] clear_page_dirty_for_io+0x84/0x2a8 [ 254.040311] [] mpage_submit_page+0x34/0x98 [ 254.040518] [] mpage_process_page_bufs+0x164/0x170 [ 254.040743] [] mpage_prepare_extent_to_map+0x134/0x2b8 [ 254.040969] [] ext4_writepages+0x484/0xe30 [ 254.041175] [] do_writepages+0x44/0xe8 [ 254.041372] [] __filemap_fdatawrite_range+0xbc/0x110 [ 254.041568] [] file_write_and_wait_range+0x48/0xd8 [ 254.041739] [] ext4_sync_file+0x80/0x4b8 [ 254.041907] [] vfs_fsync_range+0x64/0xc0 [ 254.042106] [] SyS_msync+0x194/0x1e8 This patch fixes the problem by ensuring that READ_ONCE is used before the initial checks on the pmd, and this value is subsequently used when checking whether or not the pmd is present. pmd_check is removed and the pmd_present check is inlined directly. Link: http://lkml.kernel.org/r/1507222630-5839-1-git-send-email-will.deacon@arm.com Fixes: f27176cfc363 ("mm: convert page_mkclean_one() to use page_vma_mapped_walk()") Signed-off-by: Will Deacon Tested-by: Yury Norov Tested-by: Richard Ruigrok Acked-by: Kirill A. Shutemov Cc: "Paul E. McKenney" Cc: Peter Zijlstra Cc: Signed-off-by: Andrew Morton --- mm/page_vma_mapped.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff -puN mm/page_vma_mapped.c~mm-page_vma_mapped-ensure-pmd-is-loaded-with-read_once-outside-of-lock mm/page_vma_mapped.c --- a/mm/page_vma_mapped.c~mm-page_vma_mapped-ensure-pmd-is-loaded-with-read_once-outside-of-lock +++ a/mm/page_vma_mapped.c @@ -6,17 +6,6 @@ #include "internal.h" -static inline bool check_pmd(struct page_vma_mapped_walk *pvmw) -{ - pmd_t pmde; - /* - * Make sure we don't re-load pmd between present and !trans_huge check. - * We need a consistent view. - */ - pmde = READ_ONCE(*pvmw->pmd); - return pmd_present(pmde) && !pmd_trans_huge(pmde); -} - static inline bool not_found(struct page_vma_mapped_walk *pvmw) { page_vma_mapped_walk_done(pvmw); @@ -116,6 +105,7 @@ bool page_vma_mapped_walk(struct page_vm pgd_t *pgd; p4d_t *p4d; pud_t *pud; + pmd_t pmde; /* The only possible pmd mapping has been handled on last iteration */ if (pvmw->pmd && !pvmw->pte) @@ -148,7 +138,13 @@ restart: if (!pud_present(*pud)) return false; pvmw->pmd = pmd_offset(pud, pvmw->address); - if (pmd_trans_huge(*pvmw->pmd) || is_pmd_migration_entry(*pvmw->pmd)) { + /* + * Make sure the pmd value isn't cached in a register by the + * compiler and used as a stale value after we've observed a + * subsequent update. + */ + pmde = READ_ONCE(*pvmw->pmd); + if (pmd_trans_huge(pmde) || is_pmd_migration_entry(pmde)) { pvmw->ptl = pmd_lock(mm, pvmw->pmd); if (likely(pmd_trans_huge(*pvmw->pmd))) { if (pvmw->flags & PVMW_MIGRATION) @@ -174,9 +170,8 @@ restart: spin_unlock(pvmw->ptl); pvmw->ptl = NULL; } - } else { - if (!check_pmd(pvmw)) - return false; + } else if (!pmd_present(pmde)) { + return false; } if (!map_pte(pvmw)) goto next_pte;