userfaultfd: remove (VM_)BUG_ON()s

BUG_ON() is deprecated [1].  Convert all the BUG_ON()s and VM_BUG_ON()s to
use VM_WARN_ON_ONCE().

There are a few additional cases that are converted or modified:

- Convert the printk(KERN_WARNING ...) in handle_userfault() to use
  pr_warn().

- Convert the WARN_ON_ONCE()s in move_pages() to use VM_WARN_ON_ONCE(),
  as the relevant conditions are already checked in validate_range() in
  move_pages()'s caller.

- Convert the VM_WARN_ON()'s in move_pages() to VM_WARN_ON_ONCE(). These
  cases should never happen and are similar to those in mfill_atomic()
  and mfill_atomic_hugetlb(), which were previously BUG_ON()s.
  move_pages() was added later than those functions and makes use of
  VM_WARN_ON() as a replacement for the deprecated BUG_ON(), but.
  VM_WARN_ON_ONCE() is likely a better direct replacement.

- Convert the WARN_ON() for !VM_MAYWRITE in userfaultfd_unregister() and
  userfaultfd_register_range() to VM_WARN_ON_ONCE(). This condition is
  enforced in userfaultfd_register() so it should never happen, and can
  be converted to a debug check.

[1] https://www.kernel.org/doc/html/v6.15/process/coding-style.html#use-warn-rather-than-bug

Link: https://lkml.kernel.org/r/20250619-uffd-fixes-v3-3-a7274d3bd5e4@columbia.edu
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: David Hildenbrand <david@redhat.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Peter Xu <peterx@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
This commit is contained in:
Tal Zussman 2025-06-19 21:24:25 -04:00 committed by Andrew Morton
parent 23ec90eb12
commit 31defc3b01
2 changed files with 62 additions and 65 deletions

View File

@ -165,14 +165,14 @@ static void userfaultfd_ctx_get(struct userfaultfd_ctx *ctx)
static void userfaultfd_ctx_put(struct userfaultfd_ctx *ctx) static void userfaultfd_ctx_put(struct userfaultfd_ctx *ctx)
{ {
if (refcount_dec_and_test(&ctx->refcount)) { if (refcount_dec_and_test(&ctx->refcount)) {
VM_BUG_ON(spin_is_locked(&ctx->fault_pending_wqh.lock)); VM_WARN_ON_ONCE(spin_is_locked(&ctx->fault_pending_wqh.lock));
VM_BUG_ON(waitqueue_active(&ctx->fault_pending_wqh)); VM_WARN_ON_ONCE(waitqueue_active(&ctx->fault_pending_wqh));
VM_BUG_ON(spin_is_locked(&ctx->fault_wqh.lock)); VM_WARN_ON_ONCE(spin_is_locked(&ctx->fault_wqh.lock));
VM_BUG_ON(waitqueue_active(&ctx->fault_wqh)); VM_WARN_ON_ONCE(waitqueue_active(&ctx->fault_wqh));
VM_BUG_ON(spin_is_locked(&ctx->event_wqh.lock)); VM_WARN_ON_ONCE(spin_is_locked(&ctx->event_wqh.lock));
VM_BUG_ON(waitqueue_active(&ctx->event_wqh)); VM_WARN_ON_ONCE(waitqueue_active(&ctx->event_wqh));
VM_BUG_ON(spin_is_locked(&ctx->fd_wqh.lock)); VM_WARN_ON_ONCE(spin_is_locked(&ctx->fd_wqh.lock));
VM_BUG_ON(waitqueue_active(&ctx->fd_wqh)); VM_WARN_ON_ONCE(waitqueue_active(&ctx->fd_wqh));
mmdrop(ctx->mm); mmdrop(ctx->mm);
kmem_cache_free(userfaultfd_ctx_cachep, ctx); kmem_cache_free(userfaultfd_ctx_cachep, ctx);
} }
@ -383,12 +383,12 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason)
if (!ctx) if (!ctx)
goto out; goto out;
BUG_ON(ctx->mm != mm); VM_WARN_ON_ONCE(ctx->mm != mm);
/* Any unrecognized flag is a bug. */ /* Any unrecognized flag is a bug. */
VM_BUG_ON(reason & ~__VM_UFFD_FLAGS); VM_WARN_ON_ONCE(reason & ~__VM_UFFD_FLAGS);
/* 0 or > 1 flags set is a bug; we expect exactly 1. */ /* 0 or > 1 flags set is a bug; we expect exactly 1. */
VM_BUG_ON(!reason || (reason & (reason - 1))); VM_WARN_ON_ONCE(!reason || (reason & (reason - 1)));
if (ctx->features & UFFD_FEATURE_SIGBUS) if (ctx->features & UFFD_FEATURE_SIGBUS)
goto out; goto out;
@ -411,12 +411,11 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason)
* to be sure not to return SIGBUS erroneously on * to be sure not to return SIGBUS erroneously on
* nowait invocations. * nowait invocations.
*/ */
BUG_ON(vmf->flags & FAULT_FLAG_RETRY_NOWAIT); VM_WARN_ON_ONCE(vmf->flags & FAULT_FLAG_RETRY_NOWAIT);
#ifdef CONFIG_DEBUG_VM #ifdef CONFIG_DEBUG_VM
if (printk_ratelimit()) { if (printk_ratelimit()) {
printk(KERN_WARNING pr_warn("FAULT_FLAG_ALLOW_RETRY missing %x\n",
"FAULT_FLAG_ALLOW_RETRY missing %x\n", vmf->flags);
vmf->flags);
dump_stack(); dump_stack();
} }
#endif #endif
@ -602,7 +601,7 @@ static void userfaultfd_event_wait_completion(struct userfaultfd_ctx *ctx,
*/ */
out: out:
atomic_dec(&ctx->mmap_changing); atomic_dec(&ctx->mmap_changing);
VM_BUG_ON(atomic_read(&ctx->mmap_changing) < 0); VM_WARN_ON_ONCE(atomic_read(&ctx->mmap_changing) < 0);
userfaultfd_ctx_put(ctx); userfaultfd_ctx_put(ctx);
} }
@ -710,7 +709,7 @@ void dup_userfaultfd_fail(struct list_head *fcs)
struct userfaultfd_ctx *ctx = fctx->new; struct userfaultfd_ctx *ctx = fctx->new;
atomic_dec(&octx->mmap_changing); atomic_dec(&octx->mmap_changing);
VM_BUG_ON(atomic_read(&octx->mmap_changing) < 0); VM_WARN_ON_ONCE(atomic_read(&octx->mmap_changing) < 0);
userfaultfd_ctx_put(octx); userfaultfd_ctx_put(octx);
userfaultfd_ctx_put(ctx); userfaultfd_ctx_put(ctx);
@ -1317,8 +1316,8 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
do { do {
cond_resched(); cond_resched();
BUG_ON(!!cur->vm_userfaultfd_ctx.ctx ^ VM_WARN_ON_ONCE(!!cur->vm_userfaultfd_ctx.ctx ^
!!(cur->vm_flags & __VM_UFFD_FLAGS)); !!(cur->vm_flags & __VM_UFFD_FLAGS));
/* check not compatible vmas */ /* check not compatible vmas */
ret = -EINVAL; ret = -EINVAL;
@ -1372,7 +1371,7 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
found = true; found = true;
} for_each_vma_range(vmi, cur, end); } for_each_vma_range(vmi, cur, end);
BUG_ON(!found); VM_WARN_ON_ONCE(!found);
ret = userfaultfd_register_range(ctx, vma, vm_flags, start, end, ret = userfaultfd_register_range(ctx, vma, vm_flags, start, end,
wp_async); wp_async);
@ -1464,8 +1463,8 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
do { do {
cond_resched(); cond_resched();
BUG_ON(!!cur->vm_userfaultfd_ctx.ctx ^ VM_WARN_ON_ONCE(!!cur->vm_userfaultfd_ctx.ctx ^
!!(cur->vm_flags & __VM_UFFD_FLAGS)); !!(cur->vm_flags & __VM_UFFD_FLAGS));
/* /*
* Prevent unregistering through a different userfaultfd than * Prevent unregistering through a different userfaultfd than
@ -1487,7 +1486,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
found = true; found = true;
} for_each_vma_range(vmi, cur, end); } for_each_vma_range(vmi, cur, end);
BUG_ON(!found); VM_WARN_ON_ONCE(!found);
vma_iter_set(&vmi, start); vma_iter_set(&vmi, start);
prev = vma_prev(&vmi); prev = vma_prev(&vmi);
@ -1504,7 +1503,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
VM_WARN_ON_ONCE(vma->vm_userfaultfd_ctx.ctx != ctx); VM_WARN_ON_ONCE(vma->vm_userfaultfd_ctx.ctx != ctx);
VM_WARN_ON_ONCE(!vma_can_userfault(vma, vma->vm_flags, wp_async)); VM_WARN_ON_ONCE(!vma_can_userfault(vma, vma->vm_flags, wp_async));
WARN_ON(!(vma->vm_flags & VM_MAYWRITE)); VM_WARN_ON_ONCE(!(vma->vm_flags & VM_MAYWRITE));
if (vma->vm_start > start) if (vma->vm_start > start)
start = vma->vm_start; start = vma->vm_start;
@ -1569,7 +1568,7 @@ static int userfaultfd_wake(struct userfaultfd_ctx *ctx,
* len == 0 means wake all and we don't want to wake all here, * len == 0 means wake all and we don't want to wake all here,
* so check it again to be sure. * so check it again to be sure.
*/ */
VM_BUG_ON(!range.len); VM_WARN_ON_ONCE(!range.len);
wake_userfault(ctx, &range); wake_userfault(ctx, &range);
ret = 0; ret = 0;
@ -1626,7 +1625,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx,
return -EFAULT; return -EFAULT;
if (ret < 0) if (ret < 0)
goto out; goto out;
BUG_ON(!ret); VM_WARN_ON_ONCE(!ret);
/* len == 0 would wake all */ /* len == 0 would wake all */
range.len = ret; range.len = ret;
if (!(uffdio_copy.mode & UFFDIO_COPY_MODE_DONTWAKE)) { if (!(uffdio_copy.mode & UFFDIO_COPY_MODE_DONTWAKE)) {
@ -1681,7 +1680,7 @@ static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx,
if (ret < 0) if (ret < 0)
goto out; goto out;
/* len == 0 would wake all */ /* len == 0 would wake all */
BUG_ON(!ret); VM_WARN_ON_ONCE(!ret);
range.len = ret; range.len = ret;
if (!(uffdio_zeropage.mode & UFFDIO_ZEROPAGE_MODE_DONTWAKE)) { if (!(uffdio_zeropage.mode & UFFDIO_ZEROPAGE_MODE_DONTWAKE)) {
range.start = uffdio_zeropage.range.start; range.start = uffdio_zeropage.range.start;
@ -1793,7 +1792,7 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg)
goto out; goto out;
/* len == 0 would wake all */ /* len == 0 would wake all */
BUG_ON(!ret); VM_WARN_ON_ONCE(!ret);
range.len = ret; range.len = ret;
if (!(uffdio_continue.mode & UFFDIO_CONTINUE_MODE_DONTWAKE)) { if (!(uffdio_continue.mode & UFFDIO_CONTINUE_MODE_DONTWAKE)) {
range.start = uffdio_continue.range.start; range.start = uffdio_continue.range.start;
@ -1850,7 +1849,7 @@ static inline int userfaultfd_poison(struct userfaultfd_ctx *ctx, unsigned long
goto out; goto out;
/* len == 0 would wake all */ /* len == 0 would wake all */
BUG_ON(!ret); VM_WARN_ON_ONCE(!ret);
range.len = ret; range.len = ret;
if (!(uffdio_poison.mode & UFFDIO_POISON_MODE_DONTWAKE)) { if (!(uffdio_poison.mode & UFFDIO_POISON_MODE_DONTWAKE)) {
range.start = uffdio_poison.range.start; range.start = uffdio_poison.range.start;
@ -2111,7 +2110,7 @@ static int new_userfaultfd(int flags)
struct file *file; struct file *file;
int fd; int fd;
BUG_ON(!current->mm); VM_WARN_ON_ONCE(!current->mm);
/* Check the UFFD_* constants for consistency. */ /* Check the UFFD_* constants for consistency. */
BUILD_BUG_ON(UFFD_USER_MODE_ONLY & UFFD_SHARED_FCNTL_FLAGS); BUILD_BUG_ON(UFFD_USER_MODE_ONLY & UFFD_SHARED_FCNTL_FLAGS);

View File

@ -561,7 +561,7 @@ retry:
} }
while (src_addr < src_start + len) { while (src_addr < src_start + len) {
BUG_ON(dst_addr >= dst_start + len); VM_WARN_ON_ONCE(dst_addr >= dst_start + len);
/* /*
* Serialize via vma_lock and hugetlb_fault_mutex. * Serialize via vma_lock and hugetlb_fault_mutex.
@ -602,7 +602,7 @@ retry:
if (unlikely(err == -ENOENT)) { if (unlikely(err == -ENOENT)) {
up_read(&ctx->map_changing_lock); up_read(&ctx->map_changing_lock);
uffd_mfill_unlock(dst_vma); uffd_mfill_unlock(dst_vma);
BUG_ON(!folio); VM_WARN_ON_ONCE(!folio);
err = copy_folio_from_user(folio, err = copy_folio_from_user(folio,
(const void __user *)src_addr, true); (const void __user *)src_addr, true);
@ -614,7 +614,7 @@ retry:
dst_vma = NULL; dst_vma = NULL;
goto retry; goto retry;
} else } else
BUG_ON(folio); VM_WARN_ON_ONCE(folio);
if (!err) { if (!err) {
dst_addr += vma_hpagesize; dst_addr += vma_hpagesize;
@ -635,9 +635,9 @@ out_unlock_vma:
out: out:
if (folio) if (folio)
folio_put(folio); folio_put(folio);
BUG_ON(copied < 0); VM_WARN_ON_ONCE(copied < 0);
BUG_ON(err > 0); VM_WARN_ON_ONCE(err > 0);
BUG_ON(!copied && !err); VM_WARN_ON_ONCE(!copied && !err);
return copied ? copied : err; return copied ? copied : err;
} }
#else /* !CONFIG_HUGETLB_PAGE */ #else /* !CONFIG_HUGETLB_PAGE */
@ -711,12 +711,12 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx,
/* /*
* Sanitize the command parameters: * Sanitize the command parameters:
*/ */
BUG_ON(dst_start & ~PAGE_MASK); VM_WARN_ON_ONCE(dst_start & ~PAGE_MASK);
BUG_ON(len & ~PAGE_MASK); VM_WARN_ON_ONCE(len & ~PAGE_MASK);
/* Does the address range wrap, or is the span zero-sized? */ /* Does the address range wrap, or is the span zero-sized? */
BUG_ON(src_start + len <= src_start); VM_WARN_ON_ONCE(src_start + len <= src_start);
BUG_ON(dst_start + len <= dst_start); VM_WARN_ON_ONCE(dst_start + len <= dst_start);
src_addr = src_start; src_addr = src_start;
dst_addr = dst_start; dst_addr = dst_start;
@ -775,7 +775,7 @@ retry:
while (src_addr < src_start + len) { while (src_addr < src_start + len) {
pmd_t dst_pmdval; pmd_t dst_pmdval;
BUG_ON(dst_addr >= dst_start + len); VM_WARN_ON_ONCE(dst_addr >= dst_start + len);
dst_pmd = mm_alloc_pmd(dst_mm, dst_addr); dst_pmd = mm_alloc_pmd(dst_mm, dst_addr);
if (unlikely(!dst_pmd)) { if (unlikely(!dst_pmd)) {
@ -818,7 +818,7 @@ retry:
up_read(&ctx->map_changing_lock); up_read(&ctx->map_changing_lock);
uffd_mfill_unlock(dst_vma); uffd_mfill_unlock(dst_vma);
BUG_ON(!folio); VM_WARN_ON_ONCE(!folio);
kaddr = kmap_local_folio(folio, 0); kaddr = kmap_local_folio(folio, 0);
err = copy_from_user(kaddr, err = copy_from_user(kaddr,
@ -832,7 +832,7 @@ retry:
flush_dcache_folio(folio); flush_dcache_folio(folio);
goto retry; goto retry;
} else } else
BUG_ON(folio); VM_WARN_ON_ONCE(folio);
if (!err) { if (!err) {
dst_addr += PAGE_SIZE; dst_addr += PAGE_SIZE;
@ -852,9 +852,9 @@ out_unlock:
out: out:
if (folio) if (folio)
folio_put(folio); folio_put(folio);
BUG_ON(copied < 0); VM_WARN_ON_ONCE(copied < 0);
BUG_ON(err > 0); VM_WARN_ON_ONCE(err > 0);
BUG_ON(!copied && !err); VM_WARN_ON_ONCE(!copied && !err);
return copied ? copied : err; return copied ? copied : err;
} }
@ -940,11 +940,11 @@ int mwriteprotect_range(struct userfaultfd_ctx *ctx, unsigned long start,
/* /*
* Sanitize the command parameters: * Sanitize the command parameters:
*/ */
BUG_ON(start & ~PAGE_MASK); VM_WARN_ON_ONCE(start & ~PAGE_MASK);
BUG_ON(len & ~PAGE_MASK); VM_WARN_ON_ONCE(len & ~PAGE_MASK);
/* Does the address range wrap, or is the span zero-sized? */ /* Does the address range wrap, or is the span zero-sized? */
BUG_ON(start + len <= start); VM_WARN_ON_ONCE(start + len <= start);
mmap_read_lock(dst_mm); mmap_read_lock(dst_mm);
@ -1738,15 +1738,13 @@ ssize_t move_pages(struct userfaultfd_ctx *ctx, unsigned long dst_start,
ssize_t moved = 0; ssize_t moved = 0;
/* Sanitize the command parameters. */ /* Sanitize the command parameters. */
if (WARN_ON_ONCE(src_start & ~PAGE_MASK) || VM_WARN_ON_ONCE(src_start & ~PAGE_MASK);
WARN_ON_ONCE(dst_start & ~PAGE_MASK) || VM_WARN_ON_ONCE(dst_start & ~PAGE_MASK);
WARN_ON_ONCE(len & ~PAGE_MASK)) VM_WARN_ON_ONCE(len & ~PAGE_MASK);
goto out;
/* Does the address range wrap, or is the span zero-sized? */ /* Does the address range wrap, or is the span zero-sized? */
if (WARN_ON_ONCE(src_start + len <= src_start) || VM_WARN_ON_ONCE(src_start + len < src_start);
WARN_ON_ONCE(dst_start + len <= dst_start)) VM_WARN_ON_ONCE(dst_start + len < dst_start);
goto out;
err = uffd_move_lock(mm, dst_start, src_start, &dst_vma, &src_vma); err = uffd_move_lock(mm, dst_start, src_start, &dst_vma, &src_vma);
if (err) if (err)
@ -1896,9 +1894,9 @@ out_unlock:
up_read(&ctx->map_changing_lock); up_read(&ctx->map_changing_lock);
uffd_move_unlock(dst_vma, src_vma); uffd_move_unlock(dst_vma, src_vma);
out: out:
VM_WARN_ON(moved < 0); VM_WARN_ON_ONCE(moved < 0);
VM_WARN_ON(err > 0); VM_WARN_ON_ONCE(err > 0);
VM_WARN_ON(!moved && !err); VM_WARN_ON_ONCE(!moved && !err);
return moved ? moved : err; return moved ? moved : err;
} }
@ -1985,10 +1983,10 @@ int userfaultfd_register_range(struct userfaultfd_ctx *ctx,
for_each_vma_range(vmi, vma, end) { for_each_vma_range(vmi, vma, end) {
cond_resched(); cond_resched();
BUG_ON(!vma_can_userfault(vma, vm_flags, wp_async)); VM_WARN_ON_ONCE(!vma_can_userfault(vma, vm_flags, wp_async));
BUG_ON(vma->vm_userfaultfd_ctx.ctx && VM_WARN_ON_ONCE(vma->vm_userfaultfd_ctx.ctx &&
vma->vm_userfaultfd_ctx.ctx != ctx); vma->vm_userfaultfd_ctx.ctx != ctx);
WARN_ON(!(vma->vm_flags & VM_MAYWRITE)); VM_WARN_ON_ONCE(!(vma->vm_flags & VM_MAYWRITE));
/* /*
* Nothing to do: this vma is already registered into this * Nothing to do: this vma is already registered into this
@ -2064,8 +2062,8 @@ void userfaultfd_release_all(struct mm_struct *mm,
prev = NULL; prev = NULL;
for_each_vma(vmi, vma) { for_each_vma(vmi, vma) {
cond_resched(); cond_resched();
BUG_ON(!!vma->vm_userfaultfd_ctx.ctx ^ VM_WARN_ON_ONCE(!!vma->vm_userfaultfd_ctx.ctx ^
!!(vma->vm_flags & __VM_UFFD_FLAGS)); !!(vma->vm_flags & __VM_UFFD_FLAGS));
if (vma->vm_userfaultfd_ctx.ctx != ctx) { if (vma->vm_userfaultfd_ctx.ctx != ctx) {
prev = vma; prev = vma;
continue; continue;