lib/iov_iter: fix bugs found via cross-function consistency review

A systematic review of lib/iov_iter.c checking that each iterator type is handled consistently across all functions turned up seven issues:

- Two can cause kernel oops: a missing allocation failure check in iov_iter_extract_bvec_pages() (NULL deref under memory pressure), and a missing NULL check in iov_iter_folioq_revert() (NULL deref when reverting past the head of a folio_queue chain).

- Two return wrong values for partially consumed iterators:
iov_iter_single_seg_count() ignores iov_offset for folioq, and iov_iter_gap_alignment() ignores iov_offset for the first iovec segment.

- One fires a spurious WARN_ON_ONCE for kvec iterators in iov_iter_restore() due to a misplaced parenthesis.

- One can read out of bounds: iov_iter_alignment_iovec/bvec enter their do-while loops unconditionally even when count is zero.

- One is a testing gap: copy_to_user_iter_mc() lacks the should_fail_usercopy() check present in all other copy helpers.

All found through code review; no runtime failures were needed to discover them. Each patch is independent and can be applied separately.

Josh Law (7):
lib/iov_iter: fix missing allocation failure check in
iov_iter_extract_bvec_pages()
lib/iov_iter: add NULL check on folioq->prev in
iov_iter_folioq_revert()
lib/iov_iter: fix misplaced parenthesis in iov_iter_restore() kvec
check
lib/iov_iter: account for iov_offset in iov_iter_single_seg_count()
folioq path
lib/iov_iter: account for iov_offset in iov_iter_gap_alignment() lib/iov_iter: guard iov_iter_alignment() against zero-count iovec/bvec
iterators
lib/iov_iter: add missing should_fail_usercopy() in

    copy_to_user_iter_mc()

 lib/iov_iter.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

2.34.1

When iterating bvec pages for direct I/O submission on a block device under heavy memory pressure, want_pages_array() can return 0 if the internal kvmalloc_objs() call fails. Every other call site in the file (iter_folioq_get_pages, iter_xarray_get_pages, iov_iter_extract_kvec_pages, iov_iter_extract_user_pages, __iov_iter_get_pages_alloc) checks for this and returns -ENOMEM, but iov_iter_extract_bvec_pages() does not.

When the allocation fails, *pages is left NULL and maxpages is set to 0, but the while loop still enters because bi.bi_size is nonzero. The subsequent (*pages)[k++] = bv.bv_page dereferences the NULL pointer, causing a kernel oops.

This was found through code review comparing the error handling patterns across all want_pages_array() call sites in the file and noticing the one that was inconsistent.

Add the same !maxpages check present at every other call site.

---
 lib/iov_iter.c | 2 ++
 1 file changed, 2 insertions(+)

index 0a63c7fba313..852f9ed40e5c 100644
@@ -1628,6 +1628,8 @@ static ssize_t iov_iter_extract_bvec_pages(struct iov_iter *i,
 	bi.bi_bvec_done = skip;
 
 	maxpages = want_pages_array(pages, maxsize, skip, maxpages);
+	if (!maxpages)
+		return -ENOMEM;
 
 	while (bi.bi_size && bi.bi_idx < i->nr_segs) {
 		struct bio_vec bv = bvec_iter_bvec(i->bvec, bi);

2.34.1

Don't think you can object to the Developer’s Certificate of Origin if you want your patches in the kernel.

Best,
Caleb

Hello Caleb, I have to ask, is this a joke? Or did I genuinely do something wrong

V/R

Josh Law

Is that your actual email address? objecting.org doesn't look like a real website, so I assumed you were indicating that you objected to the DCO.

Best,
Caleb

When a network filesystem using folio_queue-backed iterators (e.g. ceph or NFS with folioq read paths) hits a short read and calls iov_iter_revert() to wind the iterator back, the revert walks backwards through the folio_queue linked list via folioq->prev. If the unroll amount exceeds the data preceding the current position — possible when a splice or sendfile operation miscalculates the revert distance after a partial transfer — the walk reaches the head of the queue where prev is NULL, and the subsequent folioq_nr_slots(NULL) dereferences it.

This was found through code review examining the revert paths: the bvec and iovec revert loops have the same class of unbounded backward walk, but the folioq case is the easiest to reach in practice because folio_queue chains have an explicit NULL-terminated prev pointer.

Add a NULL check and early return when folioq->prev is NULL to prevent the oops.

---
 lib/iov_iter.c | 2 ++
 1 file changed, 2 insertions(+)

index 852f9ed40e5c..325669b103ed 100644
@@ -600,6 +600,8 @@ static void iov_iter_folioq_revert(struct iov_iter *i, size_t unroll)
 		size_t fsize;
 
 		if (slot == 0) {
+			if (!folioq->prev)
+				return;
 			folioq = folioq->prev;
 			slot = folioq_nr_slots(folioq);
 		}

2.34.1

When restoring a kvec-backed iterator after a short write in a kernel socket sendmsg path (e.g. an NFS client resending an RPC call), the WARN_ON_ONCE guard fires a spurious warning on every first call.

The closing parenthesis of WARN_ON_ONCE is after !iter_is_ubuf(i), leaving !iov_iter_is_kvec(i) as a separate && operand outside the macro:

  WARN_ON_ONCE(!bvec && !iovec && !ubuf) && !kvec

For kvec iterators this evaluates as WARN_ON_ONCE(true) && false — the warning fires but the return is skipped, so execution proceeds correctly. The warning is the only visible symptom, but it is confusing and may mask real issues.

Found through code review of the operator precedence in the condition expression and confirmed by tracing the macro expansion.

Move the closing parenthesis to include the kvec check inside WARN_ON_ONCE so the warning only fires for genuinely unsupported iterator types.

---
 lib/iov_iter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 325669b103ed..6a7959bfc849 100644
@@ -1471,7 +1471,7 @@ EXPORT_SYMBOL_GPL(import_ubuf);
 void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state)
 {
 	if (WARN_ON_ONCE(!iov_iter_is_bvec(i) && !iter_is_iovec(i) &&
-			 !iter_is_ubuf(i)) && !iov_iter_is_kvec(i))
+			 !iter_is_ubuf(i) && !iov_iter_is_kvec(i)))
 		return;
 	i->iov_offset = state->iov_offset;
 	i->count = state->count;

2.34.1

1: i am a real human
2: For code written by AI, i use Assisted-by: , i wrote the commit description with ai (thats why its so long)

Sorry for wasting your time

V/R

Josh Law

When the block layer calls iov_iter_single_seg_count() to size a segment for a folioq-backed iterator — as happens during large buffered writes through cachefiles or fscache — the function returns the full folio size without subtracting iov_offset. After a partial transfer has advanced iov_offset into the middle of a folio, this over-reports the remaining data in the current segment.

The iovec and bvec paths both subtract iov_offset (e.g. iter_iov(i)->iov_len - i->iov_offset), but the folioq path omits it.

Found by reviewing the function for consistency across iterator types and comparing each branch's treatment of iov_offset.

Subtract iov_offset from the folio size to match the behavior of the other iterator types.

---
 lib/iov_iter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 6a7959bfc849..375512beefc5 100644
@@ -682,7 +682,8 @@ size_t iov_iter_single_seg_count(const struct iov_iter *i)
 	}
 	if (unlikely(iov_iter_is_folioq(i)))
 		return !i->count ? 0 :
-			umin(folioq_folio_size(i->folioq, i->folioq_slot), i->count);
+			umin(folioq_folio_size(i->folioq, i->folioq_slot) - i->iov_offset,
+			     i->count);
 	return i->count;
 }
 EXPORT_SYMBOL(iov_iter_single_seg_count);

2.34.1

When the block layer checks gap alignment constraints for a partially consumed iovec iterator — as can happen during large direct I/O submissions that get split across multiple bio's — iov_iter_gap_alignment() uses the raw iov_base and iov_len of the first segment without adjusting for iov_offset. This reports the alignment of already-consumed data rather than the remaining portion, which can cause the block layer to apply incorrect gap alignment restrictions.

Found by comparing the first-segment handling in iov_iter_gap_alignment() against iov_iter_alignment_iovec(), which correctly applies iov_offset via its skip variable.

Apply iov_offset to the base address and length of the first segment so that gap alignment reflects the actual remaining data.

---
 lib/iov_iter.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

index 375512beefc5..18561108aa3a 100644
@@ -883,14 +883,16 @@ unsigned long iov_iter_gap_alignment(const struct iov_iter *i)
 
 	for (k = 0; k < i->nr_segs; k++) {
 		const struct iovec *iov = iter_iov(i) + k;
-		if (iov->iov_len) {
-			unsigned long base = (unsigned long)iov->iov_base;
+		size_t skip = k ? 0 : i->iov_offset;
+		size_t len = iov->iov_len - skip;
+		if (len) {
+			unsigned long base = (unsigned long)iov->iov_base + skip;
 			if (v) // if not the first one
 				res |= base | v; // this start | previous end
-			v = base + iov->iov_len;
-			if (size <= iov->iov_len)
+			v = base + len;
+			if (size <= len)
 				break;
-			size -= iov->iov_len;
+			size -= len;
 		}
 	}
 	return res;

2.34.1

When a direct I/O path calls iov_iter_alignment() on an iterator that has already been fully consumed (count == 0) — which can occur when the block layer re-checks alignment after draining all bytes — the iov_iter_alignment_iovec() and iov_iter_alignment_bvec() helpers enter their do-while loops unconditionally. With count == 0 and a non-zero first segment length, the subtraction size -= len wraps the unsigned size_t to a large value, causing the loop to walk off the end of the iov/bvec array and read garbage memory.

The ubuf path already guards against this (checking if (size) before accessing the buffer), but the iovec and bvec paths do not.

Found by reviewing the function for consistent handling of edge cases across all iterator types, noting the ubuf path's explicit zero check had no equivalent in the other branches.

Add count == 0 guards in iov_iter_alignment() before calling into the iovec and bvec helpers, matching the ubuf path's behavior.

---
 lib/iov_iter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 18561108aa3a..a5bb4fbd9d38 100644
@@ -853,10 +853,10 @@ unsigned long iov_iter_alignment(const struct iov_iter *i)
 
 	/* iovec and kvec have identical layouts */
 	if (likely(iter_is_iovec(i) || iov_iter_is_kvec(i)))
-		return iov_iter_alignment_iovec(i);
+		return i->count ? iov_iter_alignment_iovec(i) : 0;
 
 	if (iov_iter_is_bvec(i))
-		return iov_iter_alignment_bvec(i);
+		return i->count ? iov_iter_alignment_bvec(i) : 0;
 
 	/* With both xarray and folioq types, we're dealing with whole folios. */
 	if (iov_iter_is_folioq(i))

2.34.1

When running fault injection tests (CONFIG_FAULT_INJECTION_USERCOPY) against DAX/pmem read paths, the machine-check variant copy_to_user_iter_mc() is not covered because it skips the should_fail_usercopy() check that the normal copy_to_user_iter() has.

This means the dax_copy_to_iter() code path used for pmem reads cannot be exercised by the usercopy fault injector, leaving a gap in test coverage for a path that has its own distinct error handling (it must not retry on #MC).

Found by auditing all copy_to_user/copy_from_user helper variants in the file for consistent use of the fault injection hook and noticing copy_to_user_iter_mc() was the only one without it.

Add the same should_fail_usercopy() check that copy_to_user_iter() has.

---
 lib/iov_iter.c | 2 ++
 1 file changed, 2 insertions(+)

index a5bb4fbd9d38..89e04066f737 100644
@@ -204,6 +204,8 @@ static __always_inline
 size_t copy_to_user_iter_mc(void __user *iter_to, size_t progress,
 			    size_t len, void *from, void *priv2)
 {
+	if (should_fail_usercopy())
+		return len;
 	if (access_ok(iter_to, len)) {
 		from += progress;
 		instrument_copy_to_user(iter_to, from, len);

2.34.1