| From 7ed9d238c7dbb1fdb63ad96a6184985151b0171c Mon Sep 17 00:00:00 2001 |
| From: Axel Rasmussen <axelrasmussen@google.com> |
| Date: Fri, 14 May 2021 17:27:19 -0700 |
| Subject: userfaultfd: release page in error path to avoid BUG_ON |
| |
| From: Axel Rasmussen <axelrasmussen@google.com> |
| |
| commit 7ed9d238c7dbb1fdb63ad96a6184985151b0171c upstream. |
| |
| 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> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| mm/shmem.c | 12 +++++++++++- |
| 1 file changed, 11 insertions(+), 1 deletion(-) |
| |
| --- a/mm/shmem.c |
| +++ b/mm/shmem.c |
| @@ -2378,8 +2378,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); |