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 8E5AEC77B7F for ; Wed, 3 May 2023 11:01:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CA13F900002; Wed, 3 May 2023 07:01:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C2A696B0072; Wed, 3 May 2023 07:01:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AA20F900002; Wed, 3 May 2023 07:01:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by kanga.kvack.org (Postfix) with ESMTP id 6D5E86B0071 for ; Wed, 3 May 2023 07:01:47 -0400 (EDT) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 77D2A22652; Wed, 3 May 2023 11:01:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1683111706; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=z8jwtQHJUM/AAWsjtHIjrtk50jYN3yDn+0AjU75Gl8I=; b=YnHHB9XleIkwOgka1vGP8aJr57QABiFIYqyQyRc190pWEehlsHxHHfLtXMcwYmaPOpl+wN PF7SwfH0/Fl3otDZ0ORMKPi1ufHEJI+u1wXNthvFYPJ19qsDYxRolPgvwjfiYKrU2fop0C HIwPUQffZm/HAW8bbxEbbw1BFC9A3qE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1683111706; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=z8jwtQHJUM/AAWsjtHIjrtk50jYN3yDn+0AjU75Gl8I=; b=LbUksBymq3p4Gn1YivZps62g/RWguxmOUm3+x9IlBwCcn/cItgSEOeSSPICJnwGxuZWzIC oNqeocndKtvnM6AA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 60A45139F8; Wed, 3 May 2023 11:01:46 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id MroXFxo/UmRuSgAAMHmgww (envelope-from ); Wed, 03 May 2023 11:01:46 +0000 Received: by quack3.suse.cz (Postfix, from userid 1000) id DC334A0744; Wed, 3 May 2023 13:01:45 +0200 (CEST) Date: Wed, 3 May 2023 13:01:45 +0200 From: Jan Kara To: Lorenzo Stoakes Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Jason Gunthorpe , 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 , Jason Gunthorpe , John Hubbard , Jan Kara , "Kirill A . Shutemov" , Pavel Begunkov , Mika Penttila , David Hildenbrand , Dave Chinner , Theodore Ts'o , Peter Xu , Matthew Rosato , "Paul E . McKenney" , Christian Borntraeger Subject: Re: [PATCH v8 3/3] mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings Message-ID: <20230503110145.74q5sm5psv7o7nrd@quack3> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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 Tue 02-05-23 23:51:35, Lorenzo Stoakes wrote: > Writing to file-backed dirty-tracked mappings via GUP is inherently broken > as we cannot rule out folios being cleaned and then a GUP user writing to > them again and possibly marking them dirty unexpectedly. > > This is especially egregious for long-term mappings (as indicated by the > use of the FOLL_LONGTERM flag), so we disallow this case in GUP-fast as > we have already done in the slow path. > > We have access to less information in the fast path as we cannot examine > the VMA containing the mapping, however we can determine whether the folio > is anonymous or belonging to a whitelisted filesystem - specifically > hugetlb and shmem mappings. > > We take special care to ensure that both the folio and mapping are safe to > access when performing these checks and document folio_fast_pin_allowed() > accordingly. > > It's important to note that there are no APIs allowing users to specify > FOLL_FAST_ONLY for a PUP-fast let alone with FOLL_LONGTERM, so we can > always rely on the fact that if we fail to pin on the fast path, the code > will fall back to the slow path which can perform the more thorough check. > > Suggested-by: David Hildenbrand > Suggested-by: Kirill A . Shutemov > Suggested-by: Peter Zijlstra > Signed-off-by: Lorenzo Stoakes The patch looks good to me now. Feel free to add: Reviewed-by: Jan Kara Honza > --- > mm/gup.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 102 insertions(+) > > diff --git a/mm/gup.c b/mm/gup.c > index 0ea9ebec9547..1ab369b5d889 100644 > --- a/mm/gup.c > +++ b/mm/gup.c > @@ -18,6 +18,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -95,6 +96,83 @@ static inline struct folio *try_get_folio(struct page *page, int refs) > return folio; > } > > +/* > + * Used in the GUP-fast path to determine whether a pin is permitted for a > + * specific folio. > + * > + * This call assumes the caller has pinned the folio, that the lowest page table > + * level still points to this folio, and that interrupts have been disabled. > + * > + * Writing to pinned file-backed dirty tracked folios is inherently problematic > + * (see comment describing the writable_file_mapping_allowed() function). We > + * therefore try to avoid the most egregious case of a long-term mapping doing > + * so. > + * > + * This function cannot be as thorough as that one as the VMA is not available > + * in the fast path, so instead we whitelist known good cases and if in doubt, > + * fall back to the slow path. > + */ > +static bool folio_fast_pin_allowed(struct folio *folio, unsigned int flags) > +{ > + struct address_space *mapping; > + unsigned long mapping_flags; > + > + /* > + * If we aren't pinning then no problematic write can occur. A long term > + * pin is the most egregious case so this is the one we disallow. > + */ > + if ((flags & (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE)) != > + (FOLL_PIN | FOLL_LONGTERM | FOLL_WRITE)) > + return true; > + > + /* The folio is pinned, so we can safely access folio fields. */ > + > + /* Neither of these should be possible, but check to be sure. */ > + if (unlikely(folio_test_slab(folio) || folio_test_swapcache(folio))) > + return false; > + > + /* hugetlb mappings do not require dirty-tracking. */ > + if (folio_test_hugetlb(folio)) > + return true; > + > + /* > + * GUP-fast disables IRQs. When IRQS are disabled, RCU grace periods > + * cannot proceed, which means no actions performed under RCU can > + * proceed either. > + * > + * inodes and thus their mappings are freed under RCU, which means the > + * mapping cannot be freed beneath us and thus we can safely dereference > + * it. > + */ > + lockdep_assert_irqs_disabled(); > + > + /* > + * However, there may be operations which _alter_ the mapping, so ensure > + * we read it once and only once. > + */ > + mapping = READ_ONCE(folio->mapping); > + > + /* > + * The mapping may have been truncated, in any case we cannot determine > + * if this mapping is safe - fall back to slow path to determine how to > + * proceed. > + */ > + if (!mapping) > + return false; > + > + /* Anonymous folios are fine, other non-file backed cases are not. */ > + mapping_flags = (unsigned long)mapping & PAGE_MAPPING_FLAGS; > + if (mapping_flags) > + return mapping_flags == PAGE_MAPPING_ANON; > + > + /* > + * At this point, we know the mapping is non-null and points to an > + * address_space object. The only remaining whitelisted file system is > + * shmem. > + */ > + return shmem_mapping(mapping); > +} > + > /** > * try_grab_folio() - Attempt to get or pin a folio. > * @page: pointer to page to be grabbed > @@ -2464,6 +2542,11 @@ static int gup_pte_range(pmd_t pmd, pmd_t *pmdp, unsigned long addr, > goto pte_unmap; > } > > + if (!folio_fast_pin_allowed(folio, flags)) { > + gup_put_folio(folio, 1, flags); > + goto pte_unmap; > + } > + > if (!pte_write(pte) && gup_must_unshare(NULL, flags, page)) { > gup_put_folio(folio, 1, flags); > goto pte_unmap; > @@ -2656,6 +2739,11 @@ static int gup_hugepte(pte_t *ptep, unsigned long sz, unsigned long addr, > return 0; > } > > + if (!folio_fast_pin_allowed(folio, flags)) { > + gup_put_folio(folio, refs, flags); > + return 0; > + } > + > if (!pte_write(pte) && gup_must_unshare(NULL, flags, &folio->page)) { > gup_put_folio(folio, refs, flags); > return 0; > @@ -2722,6 +2810,10 @@ static int gup_huge_pmd(pmd_t orig, pmd_t *pmdp, unsigned long addr, > return 0; > } > > + if (!folio_fast_pin_allowed(folio, flags)) { > + gup_put_folio(folio, refs, flags); > + return 0; > + } > if (!pmd_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) { > gup_put_folio(folio, refs, flags); > return 0; > @@ -2762,6 +2854,11 @@ static int gup_huge_pud(pud_t orig, pud_t *pudp, unsigned long addr, > return 0; > } > > + if (!folio_fast_pin_allowed(folio, flags)) { > + gup_put_folio(folio, refs, flags); > + return 0; > + } > + > if (!pud_write(orig) && gup_must_unshare(NULL, flags, &folio->page)) { > gup_put_folio(folio, refs, flags); > return 0; > @@ -2797,6 +2894,11 @@ static int gup_huge_pgd(pgd_t orig, pgd_t *pgdp, unsigned long addr, > return 0; > } > > + if (!folio_fast_pin_allowed(folio, flags)) { > + gup_put_folio(folio, refs, flags); > + return 0; > + } > + > *nr += refs; > folio_set_referenced(folio); > return 1; > -- > 2.40.1 > -- Jan Kara SUSE Labs, CR