[PATCH 0/7] lib/iov_iter: fix bugs found via cross-function consistency review

[PATCH 0/7] lib/iov_iter: fix bugs found via cross-function consistency review

[Josh Law]

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

[PATCH 1/7] lib/iov_iter: fix missing allocation failure check in iov_iter_extract_bvec_pages()

[Josh Law]

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.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/iov_iter.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 0a63c7fba313..852f9ed40e5c 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -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

Re: [PATCH 1/7] lib/iov_iter: fix missing allocation failure check in iov_iter_extract_bvec_pages()

[Caleb Sander Mateos]

On Fri, Mar 13, 2026 at 11:12 AM Josh Law <objecting@objecting.org> wrote:
>
> 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.
>
> Signed-off-by: Josh Law <objecting@objecting.org>

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

Best,
Caleb

> ---
>  lib/iov_iter.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 0a63c7fba313..852f9ed40e5c 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -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
>
>

Re: [PATCH 1/7] lib/iov_iter: fix missing allocation failure check in iov_iter_extract_bvec_pages()

[Josh Law]

>> Signed-off-by: Josh Law <objecting@objecting.org>
>
>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

Re: [PATCH 1/7] lib/iov_iter: fix missing allocation failure check in iov_iter_extract_bvec_pages()

[Caleb Sander Mateos]

On Fri, Mar 13, 2026 at 11:20 AM Josh Law <objecting@objecting.org> wrote:
>
>
>
> >> Signed-off-by: Josh Law <objecting@objecting.org>
> >
> >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

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

[PATCH 2/7] lib/iov_iter: add NULL check on folioq->prev in iov_iter_folioq_revert()

[Josh Law]

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.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/iov_iter.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 852f9ed40e5c..325669b103ed 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -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

[PATCH 3/7] lib/iov_iter: fix misplaced parenthesis in iov_iter_restore() kvec check

[Josh Law]

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.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/iov_iter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 325669b103ed..6a7959bfc849 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -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

Re: [PATCH 3/7] lib/iov_iter: fix misplaced parenthesis in iov_iter_restore() kvec check

[Christian Brauner]

On Fri, Mar 13, 2026 at 06:10:34PM +0000, Josh Law wrote:
> 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.
> 
> Signed-off-by: Josh Law <objecting@objecting.org>
> ---
>  lib/iov_iter.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 325669b103ed..6a7959bfc849 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -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;

Look... the placement is intentional as is easily identifiable by
looking at the history of the commits. This clearly is AI generated
and you don't have the capacity at the moment to meaningfully argue or
present the technical changes you are proposing - if you are an actual
person at all. As such I see no reason to engage with this.

Re: [PATCH 3/7] lib/iov_iter: fix misplaced parenthesis in iov_iter_restore() kvec check

[Josh Law]

On 23 March 2026 12:39:10 GMT, Christian Brauner <brauner@kernel.org> wrote:
>On Fri, Mar 13, 2026 at 06:10:34PM +0000, Josh Law wrote:
>> 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.
>> 
>> Signed-off-by: Josh Law <objecting@objecting.org>
>> ---
>>  lib/iov_iter.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
>> index 325669b103ed..6a7959bfc849 100644
>> --- a/lib/iov_iter.c
>> +++ b/lib/iov_iter.c
>> @@ -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;
>
>Look... the placement is intentional as is easily identifiable by
>looking at the history of the commits. This clearly is AI generated
>and you don't have the capacity at the moment to meaningfully argue or
>present the technical changes you are proposing - if you are an actual
>person at all. As such I see no reason to engage with this.



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

[PATCH 4/7] lib/iov_iter: account for iov_offset in iov_iter_single_seg_count() folioq path

[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.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/iov_iter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 6a7959bfc849..375512beefc5 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -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

[PATCH 5/7] lib/iov_iter: account for iov_offset in iov_iter_gap_alignment()

[Josh Law]

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.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/iov_iter.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 375512beefc5..18561108aa3a 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -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

[PATCH 6/7] lib/iov_iter: guard iov_iter_alignment() against zero-count iovec/bvec iterators

[Josh Law]

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.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/iov_iter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 18561108aa3a..a5bb4fbd9d38 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -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

[PATCH 7/7] lib/iov_iter: add missing should_fail_usercopy() in copy_to_user_iter_mc()

[Josh Law]

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.

Signed-off-by: Josh Law <objecting@objecting.org>
---
 lib/iov_iter.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index a5bb4fbd9d38..89e04066f737 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -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