lkml.org 
[lkml]   [2023]   [May]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v8 3/3] mm/gup: disallow FOLL_LONGTERM GUP-fast writing to file-backed mappings
    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 <david@redhat.com>
    > Suggested-by: Kirill A . Shutemov <kirill@shutemov.name>
    > Suggested-by: Peter Zijlstra <peterz@infradead.org>
    > Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>

    The patch looks good to me now. Feel free to add:

    Reviewed-by: Jan Kara <jack@suse.cz>

    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 <linux/migrate.h>
    > #include <linux/mm_inline.h>
    > #include <linux/sched/mm.h>
    > +#include <linux/shmem_fs.h>
    >
    > #include <asm/mmu_context.h>
    > #include <asm/tlbflush.h>
    > @@ -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 <jack@suse.com>
    SUSE Labs, CR

    \
     
     \ /
      Last update: 2023-05-03 13:03    [W:4.094 / U:0.232 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site