[ciqlts9_6] crypto: rng - Fix spurious EFAULT in the FIPS per-CPU DRBG

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)))
@@ -439,6 +453,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 {
@@ -452,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) {
@@ -476,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);
@@ -513,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) {
@@ -560,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);
 		}
 
 		/*
@@ -576,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;
@@ -612,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;
 }

include/linux/mm.h

@@ -2571,8 +2571,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);

mm/gup.c

@@ -3359,20 +3359,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
  *