Skip to content

Commit 9556772

Browse files
xzpeterakpm00
authored andcommitted
mm/userfaultfd: fix uninitialized output field for -EAGAIN race
While discussing some userfaultfd relevant issues recently, Andrea noticed a potential ABI breakage with -EAGAIN on almost all userfaultfd ioctl()s. Quote from Andrea, explaining how -EAGAIN was processed, and how this should fix it (taking example of UFFDIO_COPY ioctl): The "mmap_changing" and "stale pmd" conditions are already reported as -EAGAIN written in the copy field, this does not change it. This change removes the subnormal case that left copy.copy uninitialized and required apps to explicitly set the copy field to get deterministic behavior (which is a requirement contrary to the documentation in both the manpage and source code). In turn there's no alteration to backwards compatibility as result of this change because userland will find the copy field consistently set to -EAGAIN, and not anymore sometime -EAGAIN and sometime uninitialized. Even then the change only can make a difference to non cooperative users of userfaultfd, so when UFFD_FEATURE_EVENT_* is enabled, which is not true for the vast majority of apps using userfaultfd or this unintended uninitialized field may have been noticed sooner. Meanwhile, since this bug existed for years, it also almost affects all ioctl()s that was introduced later. Besides UFFDIO_ZEROPAGE, these also get affected in the same way: - UFFDIO_CONTINUE - UFFDIO_POISON - UFFDIO_MOVE This patch should have fixed all of them. Link: https://lkml.kernel.org/r/20250424215729.194656-2-peterx@redhat.com Fixes: df2cc96 ("userfaultfd: prevent non-cooperative events vs mcopy_atomic races") Fixes: f619147 ("userfaultfd: add UFFDIO_CONTINUE ioctl") Fixes: fc71884 ("mm: userfaultfd: add new UFFDIO_POISON ioctl") Fixes: adef440 ("userfaultfd: UFFDIO_MOVE uABI") Signed-off-by: Peter Xu <peterx@redhat.com> Reported-by: Andrea Arcangeli <aarcange@redhat.com> Suggested-by: Andrea Arcangeli <aarcange@redhat.com> Reviewed-by: David Hildenbrand <david@redhat.com> Cc: Mike Rapoport <rppt@kernel.org> Cc: Axel Rasmussen <axelrasmussen@google.com> Cc: Suren Baghdasaryan <surenb@google.com> Cc: <stable@vger.kernel.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
1 parent ab00ddd commit 9556772

File tree

1 file changed

+22
-6
lines changed

1 file changed

+22
-6
lines changed

fs/userfaultfd.c

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,8 +1585,11 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
15851585
user_uffdio_copy = (struct uffdio_copy __user *) arg;
15861586

15871587
ret = -EAGAIN;
1588-
if (atomic_read(&ctx->mmap_changing))
1588+
if (unlikely(atomic_read(&ctx->mmap_changing))) {
1589+
if (unlikely(put_user(ret, &user_uffdio_copy->copy)))
1590+
return -EFAULT;
15891591
goto out;
1592+
}
15901593

15911594
ret = -EFAULT;
15921595
if (copy_from_user(&uffdio_copy, user_uffdio_copy,
@@ -1641,8 +1644,11 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
16411644
user_uffdio_zeropage = (struct uffdio_zeropage __user *) arg;
16421645

16431646
ret = -EAGAIN;
1644-
if (atomic_read(&ctx->mmap_changing))
1647+
if (unlikely(atomic_read(&ctx->mmap_changing))) {
1648+
if (unlikely(put_user(ret, &user_uffdio_zeropage->zeropage)))
1649+
return -EFAULT;
16451650
goto out;
1651+
}
16461652

16471653
ret = -EFAULT;
16481654
if (copy_from_user(&uffdio_zeropage, user_uffdio_zeropage,
@@ -1744,8 +1750,11 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg)
17441750
user_uffdio_continue = (struct uffdio_continue __user *)arg;
17451751

17461752
ret = -EAGAIN;
1747-
if (atomic_read(&ctx->mmap_changing))
1753+
if (unlikely(atomic_read(&ctx->mmap_changing))) {
1754+
if (unlikely(put_user(ret, &user_uffdio_continue->mapped)))
1755+
return -EFAULT;
17481756
goto out;
1757+
}
17491758

17501759
ret = -EFAULT;
17511760
if (copy_from_user(&uffdio_continue, user_uffdio_continue,
@@ -1801,8 +1810,11 @@ static inline int userfaultfd_poison(struct userfaultfd_ctx *ctx, unsigned long
18011810
user_uffdio_poison = (struct uffdio_poison __user *)arg;
18021811

18031812
ret = -EAGAIN;
1804-
if (atomic_read(&ctx->mmap_changing))
1813+
if (unlikely(atomic_read(&ctx->mmap_changing))) {
1814+
if (unlikely(put_user(ret, &user_uffdio_poison->updated)))
1815+
return -EFAULT;
18051816
goto out;
1817+
}
18061818

18071819
ret = -EFAULT;
18081820
if (copy_from_user(&uffdio_poison, user_uffdio_poison,
@@ -1870,8 +1882,12 @@ static int userfaultfd_move(struct userfaultfd_ctx *ctx,
18701882

18711883
user_uffdio_move = (struct uffdio_move __user *) arg;
18721884

1873-
if (atomic_read(&ctx->mmap_changing))
1874-
return -EAGAIN;
1885+
ret = -EAGAIN;
1886+
if (unlikely(atomic_read(&ctx->mmap_changing))) {
1887+
if (unlikely(put_user(ret, &user_uffdio_move->move)))
1888+
return -EFAULT;
1889+
goto out;
1890+
}
18751891

18761892
if (copy_from_user(&uffdio_move, user_uffdio_move,
18771893
/* don't copy "move" last field */

0 commit comments

Comments
 (0)