Hi Jann,
On 2024/8/13 00:42, Jann Horn wrote:
The following race can occur:
mfill_atomic other thread ============ ============ <zap PMD> pmdp_get_lockless() [reads none pmd]
<bail if trans_huge> <if none:> <pagefault creates transhuge zeropage> __pte_alloc [no-op] <zap PMD> <bail if pmd_trans_huge(*dst_pmd)> BUG_ON(pmd_none(*dst_pmd))
I have experimentally verified this in a kernel with extra mdelay() calls; the BUG_ON(pmd_none(*dst_pmd)) triggers.
On kernels newer than commit 0d940a9b270b ("mm/pgtable: allow pte_offset_map[_lock]() to fail"), this can't lead to anything worse than a BUG_ON(), since the page table access helpers are actually designed to deal with page tables concurrently disappearing; but on older kernels (<=6.4), I think we could probably theoretically race past the two BUG_ON() checks and end up treating a hugepage as a page table.
Cc: stable@vger.kernel.org Fixes: c1a4de99fada ("userfaultfd: mcopy_atomic|mfill_zeropage: UFFDIO_COPY|UFFDIO_ZEROPAGE preparation") Signed-off-by: Jann Horn jannh@google.com
mm/userfaultfd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c index e54e5c8907fa..ec3750467aa5 100644 --- a/mm/userfaultfd.c +++ b/mm/userfaultfd.c @@ -801,7 +801,8 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx, break; } /* If an huge pmd materialized from under us fail */
if (unlikely(pmd_trans_huge(*dst_pmd))) {
dst_pmdval = pmdp_get_lockless(dst_pmd);if (unlikely(pmd_none(dst_pmdval) || pmd_trans_huge(dst_pmdval))) {
Before commit 0d940a9b270b, should we also check for is_pmd_migration_entry(), pmd_devmap() and pmd_bad() here?
Thanks, Qi
err = -EFAULT; break; }