From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.1 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3DF26C433E0 for ; Tue, 12 Jan 2021 11:44:11 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id B4F6923104 for ; Tue, 12 Jan 2021 11:44:10 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B4F6923104 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 37CF28D009D; Tue, 12 Jan 2021 06:44:10 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 32D058D0090; Tue, 12 Jan 2021 06:44:10 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1F56A8D009D; Tue, 12 Jan 2021 06:44:10 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0079.hostedemail.com [216.40.44.79]) by kanga.kvack.org (Postfix) with ESMTP id 08A188D0090 for ; Tue, 12 Jan 2021 06:44:10 -0500 (EST) Received: from smtpin29.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id A6A68362B for ; Tue, 12 Jan 2021 11:44:09 +0000 (UTC) X-FDA: 77696939418.29.day45_310e24227515 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin29.hostedemail.com (Postfix) with ESMTP id 7F2AF180868EB for ; Tue, 12 Jan 2021 11:44:09 +0000 (UTC) X-HE-Tag: day45_310e24227515 X-Filterd-Recvd-Size: 8103 Received: from so254-31.mailgun.net (so254-31.mailgun.net [198.61.254.31]) by imf11.hostedemail.com (Postfix) with ESMTP for ; Tue, 12 Jan 2021 11:44:08 +0000 (UTC) DKIM-Signature: a=rsa-sha256; v=1; c=relaxed/relaxed; d=mg.codeaurora.org; q=dns/txt; s=smtp; t=1610451848; h=Content-Transfer-Encoding: Content-Type: In-Reply-To: MIME-Version: Date: Message-ID: From: References: Cc: To: Subject: Sender; bh=pDD5+cOOV+kq8coSojkQ9ER1nl8CFfyd8Ps/ttb8Gl0=; b=PGvvyo04j/RccDtVEWAndVvxC6GDexftTUsIF+dqkejdbNkQVXQB6658YWrv/Vr6upyDi7qz Q//w7fz1MgfQ21s1Q89v7eiXGApdG1AKQr6ZBiQHQJHM20oB1SwWzlbA98V58QEIYnl8iX26 0183STZZOu6PPyM2QI66y+NF28A= X-Mailgun-Sending-Ip: 198.61.254.31 X-Mailgun-Sid: WyIwY2Q3OCIsICJsaW51eC1tbUBrdmFjay5vcmciLCAiYmU5ZTRhIl0= Received: from smtp.codeaurora.org (ec2-35-166-182-171.us-west-2.compute.amazonaws.com [35.166.182.171]) by smtp-out-n07.prod.us-east-1.postgun.com with SMTP id 5ffd8b884dcca12475d32cdc (version=TLS1.2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256); Tue, 12 Jan 2021 11:44:08 GMT Received: by smtp.codeaurora.org (Postfix, from userid 1001) id 322BCC43461; Tue, 12 Jan 2021 11:44:07 +0000 (UTC) Received: from [192.168.1.4] (unknown [183.83.147.228]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) (Authenticated sender: vinmenon) by smtp.codeaurora.org (Postfix) with ESMTPSA id 454D0C433ED; Tue, 12 Jan 2021 11:43:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 454D0C433ED Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: aws-us-west-2-caf-mail-1.web.codeaurora.org; spf=fail smtp.mailfrom=vinmenon@codeaurora.org Subject: Re: [PATCH] mm/userfaultfd: fix memory corruption due to writeprotect To: Peter Zijlstra , Linus Torvalds Cc: Andy Lutomirski , Peter Xu , Nadav Amit , Yu Zhao , Andrea Arcangeli , linux-mm , lkml , Pavel Emelyanov , Mike Kravetz , Mike Rapoport , stable , Minchan Kim , Will Deacon , ldufour@linux.vnet.ibm.com, surenb@google.com References: <9E301C7C-882A-4E0F-8D6D-1170E792065A@gmail.com> <1FCC8F93-FF29-44D3-A73A-DF943D056680@gmail.com> <20201221223041.GL6640@xz-x1> <20210105153727.GK3040@hirez.programming.kicks-ass.net> From: Vinayak Menon Message-ID: Date: Tue, 12 Jan 2021 17:13:57 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: <20210105153727.GK3040@hirez.programming.kicks-ass.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-GB 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: On 1/5/2021 9:07 PM, Peter Zijlstra wrote: > On Mon, Dec 21, 2020 at 08:16:11PM -0800, Linus Torvalds wrote: > >> So I think the basic rule is that "if you hold mmap_sem for writing, >> you're always safe". And that really should be considered the >> "default" locking. >> >> ANY time you make a modification to the VM layer, you should basically >> always treat it as a write operation, and get the mmap_sem for >> writing. >> >> Yeah, yeah, that's a bit simplified, and it ignores various special >> cases (and the hardware page table walkers that obviously take no >> locks at all), but if you hold the mmap_sem for writing you won't >> really race with anything else - not page faults, and not other >> "modify this VM". >> To a first approximation, everybody that changes the VM should take >> the mmap_sem for writing, and the readers should just be just about >> page fault handling (and I count GUP as "page fault handling" too - >> it's kind of the same "look up page" rather than "modify vm" kind of >> operation). >> >> And there are just a _lot_ more page faults than there are things that >> modify the page tables and the vma's. >> >> So having that mental model of "lookup of pages in a VM take mmap_semn >> for reading, any modification of the VM uses it for writing" makes >> sense both from a performance angle and a logical standpoint. It's the >> correct model. >> And it's worth noting that COW is still "lookup of pages", even though >> it might modify the page tables in the process. The same way lookup >> can modify the page tables to mark things accessed or dirty. >> >> So COW is still a lookup operation, in ways that "change the >> writabiility of this range" very much is not. COW is "lookup for >> write", and the magic we do to copy to make that write valid is still >> all about the lookup of the page. > (your other email clarified this point; the COW needs to copy while > holding the PTL and we need TLBI under PTL if we're to change this) > >> Which brings up another mental mistake I saw earlier in this thread: >> you should not think "mmap_sem is for vma, and the page table lock is >> for the page table changes". >> >> mmap_sem is the primary lock for any modifications to the VM layout, >> whether it be in the vma's or in the page tables. >> >> Now, the page table lock does exist _in_addition_to_ the mmap_sem, but >> it is partly because >> >> (a) we have things that historically walked the page tables _without_ >> walking the vma's (notably the virtual memory scanning) >> >> (b) we do allow concurrent page faults, so we then need a lower-level >> lock to serialize the parallelism we _do_ have. > And I'm thinking the speculative page fault series steps right into all > this, it fundamentally avoids mmap_sem and entirely relies on the PTL. > > Which opens it up to exactly these races explored here. > > The range lock approach does not suffer this, but I'm still worried > about the actual performance of that thing. Some thoughts on why there may not be an issue with speculative page fault. Adding Laurent as well. Possibility of race against other PTE modifiers 1) Fork - We have seen a case of SPF racing with fork marking PTEs RO and that is described and fixed here https://lore.kernel.org/patchwork/patch/1062672/ 2) mprotect - change_protection in mprotect which does the deferred flush is marked under vm_write_begin/vm_write_end, thus SPF bails out on faults on those VMAs. 3) userfaultfd - mwriteprotect_range is not protected unlike in (2) above. But SPF does not take UFFD faults. 4) hugetlb - hugetlb_change_protection - called from mprotect and covered by (2) above. 5) Concurrent faults - SPF does not handle all faults. Only anon page faults. Of which do_anonymous_page and do_swap_page are NONE/NON-PRESENT->PRESENT transitions without tlb flush. And I hope do_wp_page with RO->RW is fine as well. I could not see a case where speculative path cannot see a PTE update done via a fault on another CPU. -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation