linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: akpm@linux-foundation.org, axelrasmussen@google.com,
	hughd@google.com, linux-mm@kvack.org, mm-commits@vger.kernel.org,
	peterx@redhat.com, stable@vger.kernel.org,
	torvalds@linux-foundation.org
Subject: [patch 06/13] userfaultfd: release page in error path to avoid BUG_ON
Date: Fri, 14 May 2021 17:27:19 -0700	[thread overview]
Message-ID: <20210515002719.QX9ZRXcYB%akpm@linux-foundation.org> (raw)
In-Reply-To: <20210514172634.9018621171d5334ceee97e95@linux-foundation.org>

From: Axel Rasmussen <axelrasmussen@google.com>
Subject: userfaultfd: release page in error path to avoid BUG_ON

Consider the following sequence of events:

1. Userspace issues a UFFD ioctl, which ends up calling into
   shmem_mfill_atomic_pte(). We successfully account the blocks, we
   shmem_alloc_page(), but then the copy_from_user() fails. We return
   -ENOENT. We don't release the page we allocated.
2. Our caller detects this error code, tries the copy_from_user() after
   dropping the mmap_lock, and retries, calling back into
   shmem_mfill_atomic_pte().
3. Meanwhile, let's say another process filled up the tmpfs being used.
4. So shmem_mfill_atomic_pte() fails to account blocks this time, and
   immediately returns - without releasing the page.

This triggers a BUG_ON in our caller, which asserts that the page
should always be consumed, unless -ENOENT is returned.

To fix this, detect if we have such a "dangling" page when accounting
fails, and if so, release it before returning.

Link: https://lkml.kernel.org/r/20210428230858.348400-1-axelrasmussen@google.com
Fixes: cb658a453b93 ("userfaultfd: shmem: avoid leaking blocks and used blocks in UFFDIO_COPY")
Signed-off-by: Axel Rasmussen <axelrasmussen@google.com>
Reported-by: Hugh Dickins <hughd@google.com>
Acked-by: Hugh Dickins <hughd@google.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/shmem.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

--- a/mm/shmem.c~userfaultfd-release-page-in-error-path-to-avoid-bug_on
+++ a/mm/shmem.c
@@ -2361,8 +2361,18 @@ static int shmem_mfill_atomic_pte(struct
 	pgoff_t offset, max_off;
 
 	ret = -ENOMEM;
-	if (!shmem_inode_acct_block(inode, 1))
+	if (!shmem_inode_acct_block(inode, 1)) {
+		/*
+		 * We may have got a page, returned -ENOENT triggering a retry,
+		 * and now we find ourselves with -ENOMEM. Release the page, to
+		 * avoid a BUG_ON in our caller.
+		 */
+		if (unlikely(*pagep)) {
+			put_page(*pagep);
+			*pagep = NULL;
+		}
 		goto out;
+	}
 
 	if (!*pagep) {
 		page = shmem_alloc_page(gfp, info, pgoff);
_


  parent reply	other threads:[~2021-05-15  0:27 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-15  0:26 incoming Andrew Morton
2021-05-15  0:27 ` [patch 01/13] mm/hugetlb: fix F_SEAL_FUTURE_WRITE Andrew Morton
2021-05-15  0:27 ` [patch 02/13] mm/hugetlb: fix cow where page writtable in child Andrew Morton
2021-05-15  0:27 ` [patch 03/13] mm, slub: move slub_debug static key enabling outside slab_mutex Andrew Morton
2021-05-15  0:27 ` [patch 04/13] kernel/resource: fix return code check in __request_free_mem_region Andrew Morton
2021-05-15  0:27 ` [patch 05/13] squashfs: fix divide error in calculate_skip() Andrew Morton
2021-05-15  0:27 ` Andrew Morton [this message]
2021-05-15  0:27 ` [patch 07/13] ksm: revert "use GET_KSM_PAGE_NOLOCK to get ksm page in remove_rmap_item_from_tree()" Andrew Morton
2021-05-15  0:27 ` [patch 08/13] mm: fix struct page layout on 32-bit systems Andrew Morton
2021-05-15  0:27 ` [patch 09/13] kasan: fix unit tests with CONFIG_UBSAN_LOCAL_BOUNDS enabled Andrew Morton
2021-05-15  0:27 ` [patch 10/13] mm/filemap: fix readahead return types Andrew Morton
2021-05-15  0:27 ` [patch 11/13] hfsplus: prevent corruption in shrinking truncate Andrew Morton
2021-05-15  0:27 ` [patch 12/13] docs: admin-guide: update description for kernel.modprobe sysctl Andrew Morton
2021-05-15  0:27 ` [patch 13/13] mm/ioremap: fix iomap_max_page_shift Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210515002719.QX9ZRXcYB%akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=axelrasmussen@google.com \
    --cc=hughd@google.com \
    --cc=linux-mm@kvack.org \
    --cc=mm-commits@vger.kernel.org \
    --cc=peterx@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).