From 6d6b2c3d73bf205f5ffe6a23d290ee9430934e82 Mon Sep 17 00:00:00 2001 From: Sultan Alsawaf Date: Mon, 22 Jun 2026 18:40:11 -0700 Subject: [PATCH 1/3] crypto: rng - Skip leading zero-length iovec segments The fast per-CPU DRBG path computes its initial user destination address straight from the iov_iter. For an ITER_IOVEC iter it reads iter_iov_addr() and iter_iov_len() of the current segment, but when the iovec leads with one or more zero-length segments, the current segment is one of those empty entries. iter_iov_addr() then hands back the base of an empty segment, which is whatever userspace put there: its base can be NULL or some other unwritable address, since a zero-length segment is never actually touched. Right after the setup, that address is prefaulted, and on a bogus base it fails. A failed prefault on the very first address is treated as fatal, so the whole read bails out to -EFAULT even though there are perfectly good non-empty segments later in the iovec. This is reachable with something as simple as readv() on /dev/urandom where the first iovec entry is {NULL, 0}. Fix it by advancing the iterator by zero before reading the first address. The iovec advance loop walks past every leading empty segment and stops at the first non-empty one, and there's guaranteed to be such a segment because iov_iter_count() is nonzero at this point. Empty segments that crop up mid-stream are already skipped by the per-copy advance, so this only needs to run once during setup. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Sultan Alsawaf --- crypto/rng.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crypto/rng.c b/crypto/rng.c index 011a510492580..8024df3d24d3c 100644 --- a/crypto/rng.c +++ b/crypto/rng.c @@ -439,6 +439,16 @@ static ssize_t crypto_devrandom_read_iter(struct iov_iter *iter, bool reseed) uaddr = iter->ubuf + iter->iov_offset; ulen = iov_iter_count(iter); } else if (iter_is_iovec(iter)) { + /* + * Skip any leading zero-length segments first, since + * uaddr would otherwise point at an empty segment's + * base (which may be NULL or unwritable). Advancing by + * zero walks past every leading empty segment (the + * iovec advance loop stops at the first non-empty one), + * and there's guaranteed to be a non-empty one because + * iov_iter_count() is nonzero above. + */ + iov_iter_advance(iter, 0); uaddr = iter_iov_addr(iter); ulen = iter_iov_len(iter); } else { From 6186194900c6215ee5f401e14701ecb15dce29ac Mon Sep 17 00:00:00 2001 From: Sultan Alsawaf Date: Mon, 22 Jun 2026 23:47:14 -0700 Subject: [PATCH 2/3] crypto: rng - Fix spurious EFAULT when the destination PTE is zapped While GUP pinning makes it possible to pin the page _backing_ a user address, it *doesn't* pin the page table entry (PTE) for that mapping. This means the pinned physical page can be separated from the user address it was backing, and even back a _different_ user address within the same process. PTE zapping naturally happens during memory reclaim when memory pressure is elevated, and can even be done directly by userspace via madvise(MADV_DONTNEED). Since the optimized per-CPU DRBG loop assumes copy_to_user_nofault() will always succeed on a GUP-pinned page, it immediately bails out when the nofault copy actually *does* fail for the reasons described above. This results in either fewer than requested random bytes copied or, more seriously, a spurious EFAULT returned to userspace when no random bytes were copied. As it turns out, there's no way to pin a PTE. That means it's not possible to guarantee a 100% success rate for the copy_to_user_nofault() attempt. Fix this by handling copy_to_user_nofault() errors correctly with a fall back to a faultable copy attempt outside of the RNG lock. In order to guarantee forward progress for the caller, an on-stack bounce buffer is used to copy up to 256 bytes of the generated random bytes whenever this happens rather than discarding the whole thing. There's no need to use GUP pinning anymore since there's no use for having a page pinned without pinning a PTE to go along with that page, hence the page pinning is eliminated which saves a software page table walk that was performed for _at least_ every destination page. Reported-by: Kun Yi Signed-off-by: Sultan Alsawaf --- crypto/rng.c | 146 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 95 insertions(+), 51 deletions(-) diff --git a/crypto/rng.c b/crypto/rng.c index 8024df3d24d3c..df499460a053e 100644 --- a/crypto/rng.c +++ b/crypto/rng.c @@ -418,16 +418,30 @@ clear_rng_page(struct cpu_rng_inst *cri, size_t count) count < PAGE_SIZE ? memset(cri->page, 0, count) : clear_page(cri->page); } +static __always_inline void +no_reseed_clear_unlock(struct cpu_rng_inst *cri, size_t *page_dirty_len) +{ + /* + * Clear the buffer of our latest random bytes before unlocking and + * potentially migrating CPUs, in which case we wouldn't have the same + * `cri` anymore. + */ + clear_rng_page(cri, *page_dirty_len); + unlock_local_rng(cri, false); + *page_dirty_len = 0; +} + static ssize_t crypto_devrandom_read_iter(struct iov_iter *iter, bool reseed) { /* lock_local_rng() puts us in atomic context for !reseed on non-RT */ const bool atomic = !reseed && !IS_ENABLED(CONFIG_PREEMPT_RT); const bool user_no_reseed = !reseed && user_backed_iter(iter); + /* Align the bounce buffer to a word to prevent alignment traps */ + u8 bounce[SZ_256] __aligned(sizeof(long)); size_t ulen, page_dirty_len = 0; struct cpu_rng_inst *cri; struct crypto_rng *rng; - void __user *uaddr; - struct page *upage; + u8 __user *uaddr; ssize_t ret = 0; if (unlikely(!iov_iter_count(iter))) @@ -462,21 +476,39 @@ static ssize_t crypto_devrandom_read_iter(struct iov_iter *iter, bool reseed) restart: /* - * Pin the user page backing the current user destination address, - * potentially prefaulting to allocate a page for the destination. By - * prefaulting without the RNG lock held, the DRBG won't be blocked by - * time spent on page faults for this task, and thus the DRBG can still - * be used by other tasks. + * Prefault the current user destination address in case the page for + * that virtual address is not resident. This is done by simply writing + * a single zero byte to the destination, which forces a COW if the page + * isn't resident, leaving us with a writable mapping. By prefaulting + * without the RNG lock held, the DRBG won't be blocked by time spent on + * page faults for this task, and thus the DRBG can still be used by + * other tasks. + * + * Notably, this doesn't keep the page from getting evicted later on; + * it's just an optimistic prefault under the assumption that the page + * won't be evicted before we copy random bytes into it, which is often + * but _not always_ the case. + * + * While GUP pinning makes it possible to pin the page _backing_ a user + * address, it *doesn't* pin the page table entry (PTE) for that + * mapping. This means the pinned physical page can be separated from + * the user address it was backing, and even back a _different_ user + * address within the same process. PTE zapping naturally happens during + * memory reclaim when memory pressure is elevated, and can even be done + * directly by userspace via madvise(MADV_DONTNEED). Since there's no + * way to pin a PTE, that means it's not possible to guarantee a page + * will be resident when copy_to_user_nofault() runs under the RNG lock + * below; in other words, there's no way to guarantee a 100% success + * rate for the copy_to_user_nofault() attempt. */ - if (user_no_reseed && pin_user_pages_fast((unsigned long)uaddr, 1, - FOLL_WRITE, &upage) != 1) + if (user_no_reseed && put_user((u8)0, uaddr)) goto exit; cri = lock_local_rng(&rng, reseed); if (IS_ERR(cri)) { if (!ret) ret = PTR_ERR(cri); - goto unpin_upage; + goto exit; } while (1) { @@ -486,7 +518,7 @@ static ssize_t crypto_devrandom_read_iter(struct iov_iter *iter, bool reseed) /* * Generate up to one page at a time, and align to a page - * boundary so we only need to pin one user page at a time. + * boundary to track page residence one page at a time. */ if (user_no_reseed) i = min3(i, PAGE_SIZE - offset_in_page(uaddr), ulen); @@ -523,33 +555,56 @@ static ssize_t crypto_devrandom_read_iter(struct iov_iter *iter, bool reseed) * the entire call; this is why a mutex is used instead of a * local lock for the reseed RNG, to permit sleeping without * yielding the DRBG instance. + * + * From this point on, err != 0 means the nofault copy failed. */ page_dirty_len = max(i, page_dirty_len); - if (user_no_reseed) { - err = copy_to_user_nofault(uaddr, cri->page, i); - if (err >= 0) { - iov_iter_advance(iter, i - err); - ret += i - err; - } - if (err) - break; + if (user_no_reseed && + !(err = copy_to_user_nofault(uaddr, cri->page, i))) { + iov_iter_advance(iter, i); + copied = i; } else { + const void *src = cri->page; + /* - * We know that copying from cri->page is safe, so use - * _copy_to_iter() directly to skip check_copy_size(). + * Fall back to a faultable copy attempt when the + * non-faulting attempt failed, likely because the PTE + * was zapped. To ensure forward progress, some of the + * generated bytes are retained in an on-stack bounce + * buffer and copied out rather than just discarding all + * of it and going back to `restart`. */ - copied = _copy_to_iter(cri->page, i, iter); - ret += copied; - if (copied != i) - break; + if (unlikely(err)) { + i = min(i, sizeof(bounce)); + memcpy(bounce, cri->page, i); + no_reseed_clear_unlock(cri, &page_dirty_len); + src = bounce; + } + + /* + * We know that copying from cri->page (or the bounce + * buffer) is safe, so use _copy_to_iter() directly to + * skip check_copy_size(). + */ + copied = _copy_to_iter(src, i, iter); + + /* Sanitize the on-stack bounce buffer if it was used */ + if (err) + memzero_explicit(bounce, i); } + ret += copied; /* - * Quit when either the requested number of bytes have been - * generated or there is a pending signal. + * Quit when the copy fell short, the requested number of bytes + * have been generated, or there is a pending signal. */ - if (!iov_iter_count(iter) || signal_pending(current)) + if (copied != i || !iov_iter_count(iter) || + signal_pending(current)) { + /* Skip the unlock when already unlocked */ + if (unlikely(err)) + goto exit; break; + } /* Compute the next user destination address and length */ if (user_no_reseed) { @@ -570,8 +625,6 @@ static ssize_t crypto_devrandom_read_iter(struct iov_iter *iter, bool reseed) uaddr = iter_iov(iter)->iov_base; ulen = iter_iov(iter)->iov_len; } - - unpin_user_page(upage); } /* @@ -586,33 +639,27 @@ static ssize_t crypto_devrandom_read_iter(struct iov_iter *iter, bool reseed) } /* - * Optimistically try to pin the next user page without - * faulting, so we don't need to clear cri->page and drop the - * lock on every iteration. If this fails, we fall back to - * pinning with the option to prefault. + * Optimistically check if the next user page is resident and + * writable with a dummy copy_to_user_nofault() attempt to write + * a single zero byte. If this fails, fall back to prefaulting + * outside of the lock. */ - if (user_no_reseed && !resched_without_lock && - pin_user_pages_fast_only((unsigned long)uaddr, 1, - FOLL_WRITE, &upage) == 1) + if (!err && user_no_reseed && !resched_without_lock && + !copy_to_user_nofault(uaddr, &(u8){ 0 }, 1)) continue; /* * Restart if either rescheduling is needed (and requires - * dropping the lock since we're atomic) or the optimistic page - * pinning attempt failed. + * dropping the lock since we're atomic) or the destination page + * wasn't resident when copy_to_user_nofault() was attempted. * * This always implies `reseed == false`, so unlock_local_rng() - * can just be passed `false` for reseed to eliminate a branch. + * can just be passed `false` for reseed to eliminate a branch, + * hence using no_reseed_clear_unlock(). */ if (resched_without_lock || user_no_reseed) { - /* - * Clear the buffer of our latest random bytes before - * unlocking and potentially migrating CPUs, in which - * case we wouldn't have the same `cri` anymore. - */ - clear_rng_page(cri, page_dirty_len); - unlock_local_rng(cri, false); - page_dirty_len = 0; + if (!err) + no_reseed_clear_unlock(cri, &page_dirty_len); if (resched_without_lock) cond_resched(); goto restart; @@ -622,9 +669,6 @@ static ssize_t crypto_devrandom_read_iter(struct iov_iter *iter, bool reseed) if (page_dirty_len) clear_rng_page(cri, page_dirty_len); unlock_local_rng(cri, reseed); -unpin_upage: - if (user_no_reseed) - unpin_user_page(upage); exit: return ret ? ret : -EFAULT; } From eacd8fecb7d05286cedfe1e052bd28a434e64df3 Mon Sep 17 00:00:00 2001 From: Sultan Alsawaf Date: Tue, 23 Jun 2026 11:33:01 -0700 Subject: [PATCH 3/3] Revert "mm/gup: reintroduce pin_user_pages_fast_only()" This reverts commit ef467f3c43338f72f4173c98b15bc8021f79c3a1. This helper is no longer used by the FIPS-mode RNG, which was the motivation for reintroducing it. Remove it. Signed-off-by: Sultan Alsawaf --- include/linux/mm.h | 2 -- mm/gup.c | 14 -------------- 2 files changed, 16 deletions(-) diff --git a/include/linux/mm.h b/include/linux/mm.h index 05e7a1a6afb1c..74fe0873e3518 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -2577,8 +2577,6 @@ int get_user_pages_fast(unsigned long start, int nr_pages, unsigned int gup_flags, struct page **pages); int pin_user_pages_fast(unsigned long start, int nr_pages, unsigned int gup_flags, struct page **pages); -int pin_user_pages_fast_only(unsigned long start, int nr_pages, - unsigned int gup_flags, struct page **pages); void folio_add_pin(struct folio *folio); int account_locked_vm(struct mm_struct *mm, unsigned long pages, bool inc); diff --git a/mm/gup.c b/mm/gup.c index c85a7b052f92b..4ef41429be155 100644 --- a/mm/gup.c +++ b/mm/gup.c @@ -3491,20 +3491,6 @@ int pin_user_pages_fast(unsigned long start, int nr_pages, } EXPORT_SYMBOL_GPL(pin_user_pages_fast); -/* - * This is the FOLL_PIN equivalent of get_user_pages_fast_only(). Behavior is - * the same, except that this one sets FOLL_PIN instead of FOLL_GET. - */ -int pin_user_pages_fast_only(unsigned long start, int nr_pages, - unsigned int gup_flags, struct page **pages) -{ - if (!is_valid_gup_args(pages, NULL, &gup_flags, - FOLL_PIN | FOLL_FAST_ONLY)) - return -EINVAL; - return internal_get_user_pages_fast(start, nr_pages, gup_flags, pages); -} -EXPORT_SYMBOL_GPL(pin_user_pages_fast_only); - /** * pin_user_pages_remote() - pin pages of a remote process *