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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4A79FC77B7F for ; Fri, 28 Apr 2023 23:43:54 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A71166B0075; Fri, 28 Apr 2023 19:43:53 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id A20706B0078; Fri, 28 Apr 2023 19:43:53 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8A5056B007B; Fri, 28 Apr 2023 19:43:53 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 744586B0075 for ; Fri, 28 Apr 2023 19:43:53 -0400 (EDT) Received: from smtpin17.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 50C07140371 for ; Fri, 28 Apr 2023 23:43:53 +0000 (UTC) X-FDA: 80732429946.17.3064557 Received: from wnew4-smtp.messagingengine.com (wnew4-smtp.messagingengine.com [64.147.123.18]) by imf17.hostedemail.com (Postfix) with ESMTP id DE7B240009 for ; Fri, 28 Apr 2023 23:43:49 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=shutemov.name header.s=fm1 header.b="C pVvEI/"; dkim=pass header.d=messagingengine.com header.s=fm3 header.b=JIg+gSGQ; dmarc=none; spf=pass (imf17.hostedemail.com: domain of kirill@shutemov.name designates 64.147.123.18 as permitted sender) smtp.mailfrom=kirill@shutemov.name ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1682725430; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=G9a2ROJTdxIxZkt3BlJK7s92qsI2BITvO+vfX3e3InE=; b=haolBT+1JqnbTr9xtobBs8L1eDx40TuWnscVzOS67K1uj0+AL6R7r5IJh+ldQ+136JKQ7c 7zLCGXnn4ROD71dKY+yoG1zDe8eyX8G3RD8Eub1m0olyQi2Bbt2ovx0KEIgSIUklcjqunm T90KR4gOnfiCld9WLLAwg72Auz1KjQI= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=shutemov.name header.s=fm1 header.b="C pVvEI/"; dkim=pass header.d=messagingengine.com header.s=fm3 header.b=JIg+gSGQ; dmarc=none; spf=pass (imf17.hostedemail.com: domain of kirill@shutemov.name designates 64.147.123.18 as permitted sender) smtp.mailfrom=kirill@shutemov.name ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1682725430; a=rsa-sha256; cv=none; b=Su2XmVcOiCHHAY8zC1ps+8NQfuP1NzluzaiCE9G5oJ6Vzo93CAOcJ6MgTESCfQ27mtzEzv bMAexgNltpuQV9x9QLt0/nNOIrw4f6w8ZT1u20Kfp43H2Xfd+KmSMNdPxCHqWxwRy0vu34 pncyOk/ccRuLRcGFIob9VoG6bOlSFuk= Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailnew.west.internal (Postfix) with ESMTP id 4DA8D2B067DB; Fri, 28 Apr 2023 19:43:40 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Fri, 28 Apr 2023 19:43:43 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=shutemov.name; h=cc:cc:content-type:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm1; t=1682725419; x= 1682732619; bh=G9a2ROJTdxIxZkt3BlJK7s92qsI2BITvO+vfX3e3InE=; b=C pVvEI/V43AJCH6FgH++L+zCJDslVrr7DgTbuZU1ndHFJuGnBdH/2RHpjEAHIRwXn aRhtxvt7IDMAZSo4peFT8zZtXc72b7x3ACNmZrSM3q9cNT3Uyz2IFcYoSBEIGwky ERoFdKZpyXDzMjC3PcVyd0NbnSwOcYnc/MenGf6IGvB6yn0JEKqOmcKYN4QOopln M7fqbuA888lLl80rrjNXNwFepdfbbUaidNHkB0rgGZyj7U3oscjUX66Mz+beMGaX mcUPzffcMGiFGY/7D+rMkpySyM797zY8ZeRRjVWsAf8+funfSh6+YZRb8ae54RXx r7T3voVmvokFm2ha9Ij7A== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; t=1682725419; x=1682732619; bh=G9a2ROJTdxIxZ kt3BlJK7s92qsI2BITvO+vfX3e3InE=; b=JIg+gSGQEBmHT/iJQcAHitI+659Qt 9+vwssAc+7GVHEOHh3kESJoBlsJ2C/YJhKk0+S83cY1gcF2ytRAbCRWAOs9A13CA ujwa20AfOP1dZo3zvPamz4EE47oPHTl5wcCGNp0chb8w2Wfy98iK0m2pt955gA7X lnVWjBvsWgcGRzP/Rh3p0xUupDS38AWkN0vJigfWdordc3sTIF0Mb+ntqH0Zzb8c RvPw2Q4p1LdkSnzcHrDOGqiwxfxjRjX6CcTOZgiudD4UTmk/NeEzt9HTothXBcTq lB8vmCNL2QgOv6eL3uJC/PtbQAPRpX/ZUPZwA93506fg8kPzj8shgFj1g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrfeduledgvdeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesthdttddttddtvdenucfhrhhomhepfdfmihhr ihhllhcutecurdcuufhhuhhtvghmohhvfdcuoehkihhrihhllhesshhhuhhtvghmohhvrd hnrghmvgeqnecuggftrfgrthhtvghrnhepgfdtveeugeethfffffeklefgkeelgfekfedt heeileetuefhkeefleduvddtkeevnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrg hmpehmrghilhhfrhhomhepkhhirhhilhhlsehshhhuthgvmhhovhdrnhgrmhgv X-ME-Proxy: Feedback-ID: ie3994620:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 28 Apr 2023 19:43:36 -0400 (EDT) Received: by box.shutemov.name (Postfix, from userid 1000) id E682E1041AE; Sat, 29 Apr 2023 02:43:32 +0300 (+03) Date: Sat, 29 Apr 2023 02:43:32 +0300 From: "Kirill A . Shutemov" To: David Hildenbrand Cc: Peter Xu , Lorenzo Stoakes , Jason Gunthorpe , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Jens Axboe , Matthew Wilcox , Dennis Dalessandro , Leon Romanovsky , Christian Benvenuti , Nelson Escobar , Bernard Metzler , Peter Zijlstra , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , Ian Rogers , Adrian Hunter , Bjorn Topel , Magnus Karlsson , Maciej Fijalkowski , Jonathan Lemon , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Christian Brauner , Richard Cochran , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , linux-fsdevel@vger.kernel.org, linux-perf-users@vger.kernel.org, netdev@vger.kernel.org, bpf@vger.kernel.org, Oleg Nesterov , John Hubbard , Jan Kara , Pavel Begunkov , Mika Penttila , David Howells , Christoph Hellwig Subject: Re: [PATCH v5] mm/gup: disallow GUP writing to file-backed mappings by default Message-ID: <20230428234332.2vhprztuotlqir4x@box.shutemov.name> References: <077c4b21-8806-455f-be98-d7052a584259@lucifer.local> <62ec50da-5f73-559c-c4b3-bde4eb215e08@redhat.com> <6ddc7ac4-4091-632a-7b2c-df2005438ec4@redhat.com> <20230428160925.5medjfxkyvmzfyhq@box.shutemov.name> <39cc0f26-8fc2-79dd-2e84-62238d27fd98@redhat.com> <20230428162207.o3ejmcz7rzezpt6n@box.shutemov.name> <173337c0-14f4-3246-15ff-7fbf03861c94@redhat.com> <20230428165623.pqchgi5gtfhxd5b5@box.shutemov.name> <1039c830-acec-d99b-b315-c2a6e26c34ca@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1039c830-acec-d99b-b315-c2a6e26c34ca@redhat.com> X-Rspam-User: X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: DE7B240009 X-Stat-Signature: frhcmxnwn6adzgir85oci35ubefqhohh X-HE-Tag: 1682725429-839166 X-HE-Meta: U2FsdGVkX18vnHkdX3Qi3EhRZNP4nnaFOzR8EFWjzS37rx/TQ7xrQ+0uIvwTVnWGzhHc/ivXGUu/CsrTzIa+sOcUQAovDB3qecKqBokx13MlNoYwbASVlz9lFN7s8vD0GrKbTwY/wyyzaMPc9oGWYSpxHW+PqHA0Ai4qoSFBiKdqXR26amkM2IYjTBF2pANlkYgApMZ7fzTvPAxMZtguKOEtiCl1HTJyy2r/e9OFuZjeyVRmsHg0DMnTtaC9Tm/+xqL6RQfJ4Red8G0uANFuo0T8pYrkGDo0s6mNpYsLBdhDxUPtjwQDZtnpJPCFoDQkNFwRW/wtTzMY6flmUcNUpv+MIFCIaTVi0sVmTXTnKLbW9SwMB2zVOCLUDYUHv9ZSdh01bHmumxQUCQXYMrJqJayUAZn2zOhWtgpup6a/MuK4EdchXQKCXQBsMyA2ls6GgaiqhDLqXNbS0TQ7H3Gc6h9vDPAlcm+lKluTkmyNdfUKpVP3kg2233K7D+OGWipOP75aHrn5Ft2ZV/nmoGtT/NS12ArN+5LEv+cJ6th85pHbBzK/+eU5awndGrELlNonFxaFh7FVmb8LFCDgkdoK9H6w6mEnlRttgBUraOguELyzD9NEqIRAL/zNddMUYjQo+GBBhV+0+Heju0kHB/ZC2kb6D0JJX8d8EI8GW4dOU1JYoWcoAHiUlhVWrH98/sieJBFGImllBuVL4EmjcqKhL5KmG94RW3vup/Lv0EwV13T97CV7yyo+QSID1tNlamlHODd5/mrXoEEm6FJzRNgwNqJzNkVMFsHFST/unqoFCCiZluthRd87sjPas01DfapXwem9c6Hxam9oy4FZ7OFi00zx+nqi6H/yZtnbUZ2NGzEuGrE1mRYra27ikLCejcuZOv1JN0Q/KDLfCdKPXtWqdpPTa48nqy9H1S0Pg0LOFg2Kudx4WMbw4SRUMHefKwXHfJ3ZBBUizjz9b2b0trM hUgqzbZP TCpXJ0hruWhnTaAnN8welOMtOY13LbGaqgwYopr874KB5BdUS7Tpns6MAIT+15lQPPQ9AhQmtAjxTwfYCgC44pSHFe3Lix/MwUSSaxKLeJ4j8fRgpsDAxPKIpMfkYuD3pbp3H3BxiGjICSec0Rnz82Br5nwWyfUfIkWqUvgATjQkKnA0BOBNzL/BSJuZhq0hg8zEIS5KFIuwZYHHJEti7Uu6ny8+7x8iFyKfVHXIMGG55Mh8ZMUOU8wxBeW+OBBwbAzqufxnJhxjYm3cd5AlTpLf+7SIuVyMDZFIRQR6JFvD/o2BgkGvY9O7AnsLs6FGunDH5/2Qk7Zsct7sY9bpnQJPTLmfCbH03K3WF0vQz7puItwH9MsErAuCE8eBXsm7kUq/H 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 Fri, Apr 28, 2023 at 07:02:22PM +0200, David Hildenbrand wrote: > On 28.04.23 18:56, Kirill A . Shutemov wrote: > > On Fri, Apr 28, 2023 at 06:51:46PM +0200, David Hildenbrand wrote: > > > On 28.04.23 18:39, Peter Xu wrote: > > > > On Fri, Apr 28, 2023 at 07:22:07PM +0300, Kirill A . Shutemov wrote: > > > > > On Fri, Apr 28, 2023 at 06:13:03PM +0200, David Hildenbrand wrote: > > > > > > On 28.04.23 18:09, Kirill A . Shutemov wrote: > > > > > > > On Fri, Apr 28, 2023 at 05:43:52PM +0200, David Hildenbrand wrote: > > > > > > > > On 28.04.23 17:34, David Hildenbrand wrote: > > > > > > > > > On 28.04.23 17:33, Lorenzo Stoakes wrote: > > > > > > > > > > On Fri, Apr 28, 2023 at 05:23:29PM +0200, David Hildenbrand wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > Security is the primary case where we have historically closed uAPI > > > > > > > > > > > > > items. > > > > > > > > > > > > > > > > > > > > > > > > As this patch > > > > > > > > > > > > > > > > > > > > > > > > 1) Does not tackle GUP-fast > > > > > > > > > > > > 2) Does not take care of !FOLL_LONGTERM > > > > > > > > > > > > > > > > > > > > > > > > I am not convinced by the security argument in regard to this patch. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > If we want to sells this as a security thing, we have to block it > > > > > > > > > > > > *completely* and then CC stable. > > > > > > > > > > > > > > > > > > > > > > Regarding GUP-fast, to fix the issue there as well, I guess we could do > > > > > > > > > > > something similar as I did in gup_must_unshare(): > > > > > > > > > > > > > > > > > > > > > > If we're in GUP-fast (no VMA), and want to pin a !anon page writable, > > > > > > > > > > > fallback to ordinary GUP. IOW, if we don't know, better be safe. > > > > > > > > > > > > > > > > > > > > How do we determine it's non-anon in the first place? The check is on the > > > > > > > > > > VMA. We could do it by following page tables down to folio and checking > > > > > > > > > > folio->mapping for PAGE_MAPPING_ANON I suppose? > > > > > > > > > > > > > > > > > > PageAnon(page) can be called from GUP-fast after grabbing a reference. > > > > > > > > > See gup_must_unshare(). > > > > > > > > > > > > > > > > IIRC, PageHuge() can also be called from GUP-fast and could special-case > > > > > > > > hugetlb eventually, as it's table while we hold a (temporary) reference. > > > > > > > > Shmem might be not so easy ... > > > > > > > > > > > > > > page->mapping->a_ops should be enough to whitelist whatever fs you want. > > > > > > > > > > > > > > > > > > > The issue is how to stabilize that from GUP-fast, such that we can safely > > > > > > dereference the mapping. Any idea? > > > > > > > > > > > > At least for anon page I know that page->mapping only gets cleared when > > > > > > freeing the page, and we don't dereference the mapping but only check a > > > > > > single flag stored alongside the mapping. Therefore, PageAnon() is fine in > > > > > > GUP-fast context. > > > > > > > > > > What codepath you are worry about that clears ->mapping on pages with > > > > > non-zero refcount? > > > > > > > > > > I can only think of truncate (and punch hole). READ_ONCE(page->mapping) > > > > > and fail GUP_fast if it is NULL should be fine, no? > > > > > > > > > > I guess we should consider if the inode can be freed from under us and the > > > > > mapping pointer becomes dangling. But I think we should be fine here too: > > > > > VMA pins inode and VMA cannot go away from under GUP. > > > > > > > > Can vma still go away if during a fast-gup? > > > > > > > > > > So, after we grabbed the page and made sure the the PTE didn't change (IOW, > > > the PTE was stable while we processed it), the page can get unmapped (but > > > not freed, because we hold a reference) and the VMA can theoretically go > > > away (and as far as I understand, nothing stops the file from getting > > > deleted, truncated etc). > > > > > > So we might be looking at folio->mapping and the VMA is no longer there. > > > Maybe even the file is no longer there. > > > > No. VMA cannot get away before PTEs are unmapped and TLB is flushed. And > > TLB flushing is serialized against GUP_fast(). > > > The whole CONFIG_MMU_GATHER_RCU_TABLE_FREE handling makes the situation more > complicated. I think I found relevant snippet of code that solves similar issue. get_futex_key() uses RCU to stabilize page->mapping after GUP_fast: /* * The associated futex object in this case is the inode and * the page->mapping must be traversed. Ordinarily this should * be stabilised under page lock but it's not strictly * necessary in this case as we just want to pin the inode, not * update the radix tree or anything like that. * * The RCU read lock is taken as the inode is finally freed * under RCU. If the mapping still matches expectations then the * mapping->host can be safely accessed as being a valid inode. */ rcu_read_lock(); if (READ_ONCE(page->mapping) != mapping) { rcu_read_unlock(); put_page(page); goto again; } inode = READ_ONCE(mapping->host); if (!inode) { rcu_read_unlock(); put_page(page); goto again; } I think something similar can be used inside GUP_fast too. -- Kiryl Shutsemau / Kirill A. Shutemov