From patchwork Fri May 28 00:50:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mina Almasry X-Patchwork-Id: 12285917 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=-25.2 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,MISSING_HEADERS, SPF_HELO_NONE,SPF_PASS,URIBL_RED,USER_AGENT_GIT,USER_IN_DEF_DKIM_WL autolearn=ham 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 844A0C4708B for ; Fri, 28 May 2021 00:50:34 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id F3D0A61358 for ; Fri, 28 May 2021 00:50:33 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F3D0A61358 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 5A2466B006C; Thu, 27 May 2021 20:50:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 52BF76B006E; Thu, 27 May 2021 20:50:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 37E3E8D0001; Thu, 27 May 2021 20:50:33 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0091.hostedemail.com [216.40.44.91]) by kanga.kvack.org (Postfix) with ESMTP id 01BE16B006C for ; Thu, 27 May 2021 20:50:32 -0400 (EDT) Received: from smtpin38.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay03.hostedemail.com (Postfix) with ESMTP id 8F9A18249980 for ; Fri, 28 May 2021 00:50:32 +0000 (UTC) X-FDA: 78188809104.38.3612CC6 Received: from mail-yb1-f202.google.com (mail-yb1-f202.google.com [209.85.219.202]) by imf21.hostedemail.com (Postfix) with ESMTP id 7A4CEE007A7E for ; Fri, 28 May 2021 00:50:21 +0000 (UTC) Received: by mail-yb1-f202.google.com with SMTP id d89-20020a25a3620000b02904dc8d0450c6so2443997ybi.2 for ; Thu, 27 May 2021 17:50:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:message-id:mime-version:subject:from:cc; bh=IxmGmY7n5KWBKt5FosIwPuRpVYTAXeIP+zmHpHVCHdk=; b=DohjESoP/dFt/2ZPtV2i7cUhEPE6LzZ381mXb86OWnZImSWL9ctrRVgf3qcYJvZjGf h8P+MmBanZuPLs7WQNs8wFZJKe8qBhkFgPFiYinMYp3SA63A4357FHeEyAqlhh3pxVSD tDVmbkOLTaPdQRsHHXYb4fcvxbXysIC2JSgEd5KkfUalQ+Pq4sgBjI4gJqJXQiZPUsmF 6Oh9NyHvjiOMIop3+kJUKjc6iX6qdE+iDnWCcZVuQfIk5ndSOwc9Jf1rtEACh6oKNfxD BoDGr2tiDPQmhy0ctLm/QF0Wmow4P5WhotB6PWaRv8h6DNZzwWpc2oP8C8N9N27SXBDx gHDA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:message-id:mime-version:subject:from:cc; bh=IxmGmY7n5KWBKt5FosIwPuRpVYTAXeIP+zmHpHVCHdk=; b=WlXYyboduMCX7yMvXYeqVnCNCoDKfokpNh9kVuVS7XXPYujHmQzIgSiZ53ssVfFIxT cjzX4gAL1CeIFh6pw+0Xu4qmcn4zBGi77R4omDJIYqSmLNchhGJxDBET9EHOClpOEc3b SNvMslp2ZxScyJg/QKMfwxEGkEtvoPeDAcjBNgqhPmxreQXMhRFOGnYM0fLoqUAqXB79 OIJ+Kt6HGS4S/bfOFlJHMRmoqDvsZJ2G9OFR8zmZdcINSUcgqAtq1LsdDyhJkpojp98C ajcN7PL3i1smjO6Uv7EJUax+A973Gzs1t37ObmLZjpTJoA31yuZaDthQqWt/ILR+POsy 1v5Q== X-Gm-Message-State: AOAM533RFqsWJWw6VIsEY/57zMg3Hb9hK2lt+RhqTlDband9kp9ZTa36 +Yx6U7HnYI+w6Tc2JJMDuQvrgtk/EQcWnrpBgw== X-Google-Smtp-Source: ABdhPJxSNpWMjGmtXQc7/7c+BN5g4mcCDdtgX5Qdjko26lrpUplmm0xvqZ2DgvAtTbdJNyCZKm1luVIrrJv7nuraSg== X-Received: from almasrymina.svl.corp.google.com ([2620:15c:2cd:202:b35:38bd:7e0f:3b1d]) (user=almasrymina job=sendgmr) by 2002:a25:80d4:: with SMTP id c20mr8667284ybm.345.1622163031545; Thu, 27 May 2021 17:50:31 -0700 (PDT) Date: Thu, 27 May 2021 17:50:29 -0700 Message-Id: <20210528005029.88088-1-almasrymina@google.com> Mime-Version: 1.0 X-Mailer: git-send-email 2.32.0.rc0.204.g9fa02ecfa5-goog Subject: [PATCH v4] mm, hugetlb: fix racy resv_huge_pages underflow on UFFDIO_COPY From: Mina Almasry Cc: Mina Almasry , Axel Rasmussen , Peter Xu , linux-mm@kvack.org, Mike Kravetz , Andrew Morton , linux-kernel@vger.kernel.org Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=DohjESoP; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf21.hostedemail.com: domain of 3Vz6wYAsKCKAALMASRYMINAGOOGLE.COMLINUX-MMKVACK.ORG@flex--almasrymina.bounces.google.com designates 209.85.219.202 as permitted sender) smtp.mailfrom=3Vz6wYAsKCKAALMASRYMINAGOOGLE.COMLINUX-MMKVACK.ORG@flex--almasrymina.bounces.google.com X-Stat-Signature: 67yz9guduj8wcyogorofekf7cbs45qww X-Rspamd-Queue-Id: 7A4CEE007A7E X-Rspamd-Server: rspam02 X-HE-Tag: 1622163021-299896 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 UFFDIO_COPY, if we fail to copy the page contents while holding the hugetlb_fault_mutex, we will drop the mutex and return to the caller after allocating a page that consumed a reservation. In this case there may be a fault that double consumes the reservation. To handle this, we free the allocated page, fix the reservations, and allocate a temporary hugetlb page and return that to the caller. When the caller does the copy outside of the lock, we again check the cache, and allocate a page consuming the reservation, and copy over the contents. Test: Hacked the code locally such that resv_huge_pages underflows produce a warning and the copy_huge_page_from_user() always fails, then: ./tools/testing/selftests/vm/userfaultfd hugetlb_shared 10 2 /tmp/kokonut_test/huge/userfaultfd_test && echo test success ./tools/testing/selftests/vm/userfaultfd hugetlb 10 2 /tmp/kokonut_test/huge/userfaultfd_test && echo test success Both tests succeed and produce no warnings. After the test runs number of free/resv hugepages is correct. Signed-off-by: Mina Almasry Cc: Axel Rasmussen Cc: Peter Xu Cc: linux-mm@kvack.org Cc: Mike Kravetz Cc: Andrew Morton Cc: linux-mm@kvack.org Cc: linux-kernel@vger.kernel.org --- include/linux/migrate.h | 4 ++++ mm/hugetlb.c | 48 +++++++++++++++++++++++++++++++++-------- mm/migrate.c | 4 ++-- mm/userfaultfd.c | 48 +---------------------------------------- 4 files changed, 46 insertions(+), 58 deletions(-) -- 2.32.0.rc0.204.g9fa02ecfa5-goog diff --git a/include/linux/migrate.h b/include/linux/migrate.h index 4bb4e519e3f5..4164c9ddd86e 100644 --- a/include/linux/migrate.h +++ b/include/linux/migrate.h @@ -51,6 +51,7 @@ extern int migrate_huge_page_move_mapping(struct address_space *mapping, struct page *newpage, struct page *page); extern int migrate_page_move_mapping(struct address_space *mapping, struct page *newpage, struct page *page, int extra_count); +extern void migrate_copy_huge_page(struct page *dst, struct page *src); #else static inline void putback_movable_pages(struct list_head *l) {} @@ -77,6 +78,9 @@ static inline int migrate_huge_page_move_mapping(struct address_space *mapping, return -ENOSYS; } +static inline void migrate_copy_huge_page(struct page *dst, struct page *src) +{ +} #endif /* CONFIG_MIGRATION */ #ifdef CONFIG_COMPACTION diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 76e2a6efc165..6072c9f82794 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -30,6 +30,7 @@ #include #include #include +#include #include #include @@ -4905,20 +4906,17 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm, struct page **pagep) { bool is_continue = (mode == MCOPY_ATOMIC_CONTINUE); - struct address_space *mapping; - pgoff_t idx; + struct hstate *h = hstate_vma(dst_vma); + struct address_space *mapping = dst_vma->vm_file->f_mapping; + pgoff_t idx = vma_hugecache_offset(h, dst_vma, dst_addr); unsigned long size; int vm_shared = dst_vma->vm_flags & VM_SHARED; - struct hstate *h = hstate_vma(dst_vma); pte_t _dst_pte; spinlock_t *ptl; - int ret; + int ret = -ENOMEM; struct page *page; int writable; - mapping = dst_vma->vm_file->f_mapping; - idx = vma_hugecache_offset(h, dst_vma, dst_addr); - if (is_continue) { ret = -EFAULT; page = find_lock_page(mapping, idx); @@ -4947,12 +4945,44 @@ int hugetlb_mcopy_atomic_pte(struct mm_struct *dst_mm, /* fallback to copy_from_user outside mmap_lock */ if (unlikely(ret)) { ret = -ENOENT; + /* Free the allocated page which may have + * consumed a reservation. + */ + restore_reserve_on_error(h, dst_vma, dst_addr, page); + put_page(page); + + /* Allocate a temporary page to hold the copied + * contents. + */ + page = alloc_huge_page_vma(h, dst_vma, dst_addr); + if (IS_ERR(page)) { + ret = -ENOMEM; + goto out; + } *pagep = page; - /* don't free the page */ + /* Set the outparam pagep and return to the caller to + * copy the contents outside the lock. Don't free the + * page. + */ goto out; } } else { - page = *pagep; + if (vm_shared && + hugetlbfs_pagecache_present(h, dst_vma, dst_addr)) { + put_page(*pagep); + ret = -EEXIST; + *pagep = NULL; + goto out; + } + + page = alloc_huge_page(dst_vma, dst_addr, 0); + if (IS_ERR(page)) { + ret = -ENOMEM; + *pagep = NULL; + goto out; + } + migrate_copy_huge_page(page, *pagep); + put_page(*pagep); *pagep = NULL; } diff --git a/mm/migrate.c b/mm/migrate.c index b234c3f3acb7..3bfe1f7d127d 100644 --- a/mm/migrate.c +++ b/mm/migrate.c @@ -550,7 +550,7 @@ static void __copy_gigantic_page(struct page *dst, struct page *src, } } -static void copy_huge_page(struct page *dst, struct page *src) +void migrate_copy_huge_page(struct page *dst, struct page *src) { int i; int nr_pages; @@ -652,7 +652,7 @@ EXPORT_SYMBOL(migrate_page_states); void migrate_page_copy(struct page *newpage, struct page *page) { if (PageHuge(page) || PageTransHuge(page)) - copy_huge_page(newpage, page); + migrate_copy_huge_page(newpage, page); else copy_highpage(newpage, page); diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c index 2d6a3a36f6ce..e13a0492b7ba 100644 --- a/mm/userfaultfd.c +++ b/mm/userfaultfd.c @@ -346,54 +346,8 @@ static __always_inline ssize_t __mcopy_atomic_hugetlb(struct mm_struct *dst_mm, out_unlock: mmap_read_unlock(dst_mm); out: - if (page) { - /* - * We encountered an error and are about to free a newly - * allocated huge page. - * - * Reservation handling is very subtle, and is different for - * private and shared mappings. See the routine - * restore_reserve_on_error for details. Unfortunately, we - * can not call restore_reserve_on_error now as it would - * require holding mmap_lock. - * - * If a reservation for the page existed in the reservation - * map of a private mapping, the map was modified to indicate - * the reservation was consumed when the page was allocated. - * We clear the HPageRestoreRsvCnt flag now so that the global - * reserve count will not be incremented in free_huge_page. - * The reservation map will still indicate the reservation - * was consumed and possibly prevent later page allocation. - * This is better than leaking a global reservation. If no - * reservation existed, it is still safe to clear - * HPageRestoreRsvCnt as no adjustments to reservation counts - * were made during allocation. - * - * The reservation map for shared mappings indicates which - * pages have reservations. When a huge page is allocated - * for an address with a reservation, no change is made to - * the reserve map. In this case HPageRestoreRsvCnt will be - * set to indicate that the global reservation count should be - * incremented when the page is freed. This is the desired - * behavior. However, when a huge page is allocated for an - * address without a reservation a reservation entry is added - * to the reservation map, and HPageRestoreRsvCnt will not be - * set. When the page is freed, the global reserve count will - * NOT be incremented and it will appear as though we have - * leaked reserved page. In this case, set HPageRestoreRsvCnt - * so that the global reserve count will be incremented to - * match the reservation map entry which was created. - * - * Note that vm_alloc_shared is based on the flags of the vma - * for which the page was originally allocated. dst_vma could - * be different or NULL on error. - */ - if (vm_alloc_shared) - SetHPageRestoreRsvCnt(page); - else - ClearHPageRestoreRsvCnt(page); + if (page) put_page(page); - } BUG_ON(copied < 0); BUG_ON(err > 0); BUG_ON(!copied && !err);