mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Fix length calculation bug in extract_kvec_to_sg
@ 2026-03-24 20:34 Christian A. Ehrhardt
  2026-03-24 20:34 ` [PATCH v2 1/3] lib: kunit_iov_iter: Improve error detection Christian A. Ehrhardt
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Christian A. Ehrhardt @ 2026-03-24 20:34 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton, Josh Law, David Howells
  Cc: Christian A. Ehrhardt, Kees Cook, Petr Mladek, David Gow

There is a bug in extract_kvec_to_sg() where the length
of a scatterlist segment is miscalculated. The actual fix
is a one-liner and it is quite obvious from reading the
code that this is what was intened.

As this is a core library function this series first adds
test cases to the kunit_iov_iter test that demonstrate that
there is a bug before actually fixing it in the last commit.

The bug was orignally introduced into kernel v6.3 where the
function lived in fs/netfs/iterator.c. It was later moved
to lib/scatterlist.c in v6.5. Thus the actual fix is only
marked for backports to v6.5+.

---
Changes in v2:
Addresss valid issues raised by AI review
https://sashiko.dev/#/patchset/20260323212350.807118-1-lk@c--e.de:
- Add kunit assertions for OOM conditions in the test
- Reorder commits.
- Fix sg_max == 0 case.
- Fix return value if we run out of sg entries.
- Adjust tests to catch these cases, too.
---

Christian A. Ehrhardt (3):
  lib: kunit_iov_iter: Improve error detection
  lib: kunit_iov_iter: Add tests for extract_iter_to_sg
  lib: Fix length calculation in extract_kvec_to_sg

 lib/scatterlist.c          |   2 +-
 lib/tests/kunit_iov_iter.c | 147 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 147 insertions(+), 2 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/3] lib: kunit_iov_iter: Improve error detection
  2026-03-24 20:34 [PATCH v2 0/3] Fix length calculation bug in extract_kvec_to_sg Christian A. Ehrhardt
@ 2026-03-24 20:34 ` Christian A. Ehrhardt
  2026-03-24 20:50   ` Josh Law
  2026-03-24 23:18   ` David Howells
  2026-03-24 20:34 ` [PATCH v2 2/3] lib: Fix length calculations in extract_kvec_to_sg Christian A. Ehrhardt
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 12+ messages in thread
From: Christian A. Ehrhardt @ 2026-03-24 20:34 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton, Josh Law, David Howells
  Cc: Christian A. Ehrhardt, Kees Cook, Petr Mladek, David Gow

In the kunit_iov_iter test prevent the kernel buffer from
being a single physically contiguous region.

Additionally, make sure that the test pattern written to
a page in the buffer depends on the offset of the page within
the buffer.

Cc: David Howells <dhowells@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Christian A. Ehrhardt <lk@c--e.de>
---
 lib/tests/kunit_iov_iter.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/lib/tests/kunit_iov_iter.c b/lib/tests/kunit_iov_iter.c
index bb847e5010eb..c42b9235f57a 100644
--- a/lib/tests/kunit_iov_iter.c
+++ b/lib/tests/kunit_iov_iter.c
@@ -13,6 +13,7 @@
 #include <linux/uio.h>
 #include <linux/bvec.h>
 #include <linux/folio_queue.h>
+#include <linux/minmax.h>
 #include <kunit/test.h>
 
 MODULE_DESCRIPTION("iov_iter testing");
@@ -37,7 +38,7 @@ static const struct kvec_test_range kvec_test_ranges[] = {
 
 static inline u8 pattern(unsigned long x)
 {
-	return x & 0xff;
+	return (u8)x + (u8)(x >> 8) + (u8)(x >> 16);
 }
 
 static void iov_kunit_unmap(void *data)
@@ -52,6 +53,7 @@ static void *__init iov_kunit_create_buffer(struct kunit *test,
 	struct page **pages;
 	unsigned long got;
 	void *buffer;
+	unsigned int i;
 
 	pages = kunit_kcalloc(test, npages, sizeof(struct page *), GFP_KERNEL);
         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pages);
@@ -62,6 +64,9 @@ static void *__init iov_kunit_create_buffer(struct kunit *test,
 		release_pages(pages, got);
 		KUNIT_ASSERT_EQ(test, got, npages);
 	}
+	/* Make sure that we don't get a physically contiguous buffer. */
+	for (i = 0; i < npages / 4; ++i)
+		swap(pages[i], pages[i + npages/2]);
 
 	buffer = vmap(pages, npages, VM_MAP | VM_MAP_PUT_PAGES, PAGE_KERNEL);
         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buffer);
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 2/3] lib: Fix length calculations in extract_kvec_to_sg
  2026-03-24 20:34 [PATCH v2 0/3] Fix length calculation bug in extract_kvec_to_sg Christian A. Ehrhardt
  2026-03-24 20:34 ` [PATCH v2 1/3] lib: kunit_iov_iter: Improve error detection Christian A. Ehrhardt
@ 2026-03-24 20:34 ` Christian A. Ehrhardt
  2026-03-24 20:54   ` Josh Law
  2026-03-24 20:34 ` [PATCH v2 3/3] lib: kunit_iov_iter: Add tests for extract_iter_to_sg Christian A. Ehrhardt
  2026-03-24 20:41 ` [PATCH v2 0/3] Fix length calculation bug in extract_kvec_to_sg Josh Law
  3 siblings, 1 reply; 12+ messages in thread
From: Christian A. Ehrhardt @ 2026-03-24 20:34 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton, Josh Law, David Howells
  Cc: Christian A. Ehrhardt, Kees Cook, Petr Mladek, David Gow

When extracting from a kvec to a scatterlist, do not
cross page boundaries. The required length is already
calculated but not used as intended.

Adjust the copied length if the loop runs out auf
sglist entries.

While there return immediately from extract_iter_to_sg
if there are no sglist entries at all.

The changes to the kunit_iov_iter.c in the next commit
demonstrate that the patch is necessary.

Cc: David Howells <dhowells@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: stable@vger.kernel.org # v6.5+
Fixes: 018584697533 ("netfs: Add a function to extract an iterator into a scatterlist")
Signed-off-by: Christian A. Ehrhardt <lk@c--e.de>
---
 lib/scatterlist.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/scatterlist.c b/lib/scatterlist.c
index d773720d11bf..befdc4b9c11d 100644
--- a/lib/scatterlist.c
+++ b/lib/scatterlist.c
@@ -1247,7 +1247,7 @@ static ssize_t extract_kvec_to_sg(struct iov_iter *iter,
 			else
 				page = virt_to_page((void *)kaddr);
 
-			sg_set_page(sg, page, len, off);
+			sg_set_page(sg, page, seg, off);
 			sgtable->nents++;
 			sg++;
 			sg_max--;
@@ -1256,6 +1256,7 @@ static ssize_t extract_kvec_to_sg(struct iov_iter *iter,
 			kaddr += PAGE_SIZE;
 			off = 0;
 		} while (len > 0 && sg_max > 0);
+		ret -= len;
 
 		if (maxsize <= 0 || sg_max == 0)
 			break;
@@ -1409,7 +1410,7 @@ ssize_t extract_iter_to_sg(struct iov_iter *iter, size_t maxsize,
 			   struct sg_table *sgtable, unsigned int sg_max,
 			   iov_iter_extraction_t extraction_flags)
 {
-	if (maxsize == 0)
+	if (maxsize == 0 || sg_max == 0)
 		return 0;
 
 	switch (iov_iter_type(iter)) {
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 3/3] lib: kunit_iov_iter: Add tests for extract_iter_to_sg
  2026-03-24 20:34 [PATCH v2 0/3] Fix length calculation bug in extract_kvec_to_sg Christian A. Ehrhardt
  2026-03-24 20:34 ` [PATCH v2 1/3] lib: kunit_iov_iter: Improve error detection Christian A. Ehrhardt
  2026-03-24 20:34 ` [PATCH v2 2/3] lib: Fix length calculations in extract_kvec_to_sg Christian A. Ehrhardt
@ 2026-03-24 20:34 ` Christian A. Ehrhardt
  2026-03-24 20:58   ` Josh Law
  2026-03-24 23:14   ` David Howells
  2026-03-24 20:41 ` [PATCH v2 0/3] Fix length calculation bug in extract_kvec_to_sg Josh Law
  3 siblings, 2 replies; 12+ messages in thread
From: Christian A. Ehrhardt @ 2026-03-24 20:34 UTC (permalink / raw)
  To: linux-kernel, Andrew Morton, Josh Law, David Howells
  Cc: Christian A. Ehrhardt, Kees Cook, Petr Mladek, David Gow

Add test cases that test extract_iter_to_sg.

For each iterator type an iterator is loaded with a kernel
buffer. The iterator is then extracted to a scatterlist with
multiple calls to extract_iter_to_sg. The final scatterlist
is copied into a scratch buffer.

The test passes if the scratch buffer compares equal to the
original buffer.

The new tests demostrate bugs in extract_iter_to_sg
for kvec iterators that are fixed by the previous
commit.

Cc: David Howells <dhowells@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Christian A. Ehrhardt <lk@c--e.de>
---
 lib/tests/kunit_iov_iter.c | 152 +++++++++++++++++++++++++++++++++++++
 1 file changed, 152 insertions(+)

diff --git a/lib/tests/kunit_iov_iter.c b/lib/tests/kunit_iov_iter.c
index c42b9235f57a..159f61c9a30f 100644
--- a/lib/tests/kunit_iov_iter.c
+++ b/lib/tests/kunit_iov_iter.c
@@ -13,6 +13,7 @@
 #include <linux/uio.h>
 #include <linux/bvec.h>
 #include <linux/folio_queue.h>
+#include <linux/scatterlist.h>
 #include <linux/minmax.h>
 #include <kunit/test.h>
 
@@ -1014,6 +1015,153 @@ static void __init iov_kunit_extract_pages_xarray(struct kunit *test)
 	KUNIT_SUCCEED(test);
 }
 
+struct iov_kunit_iter_to_sg_data {
+	struct sg_table sgt;
+	u8 *buffer, *scratch;
+	struct page **pages;
+	size_t npages;
+};
+
+static void __init
+iov_kunit_iter_to_sg_init(struct kunit *test, size_t bufsize,
+			  struct iov_kunit_iter_to_sg_data *data)
+{
+	struct page **spages;
+	struct scatterlist *sg;
+	size_t i;
+
+	data->npages = bufsize / PAGE_SIZE;
+	sg = kunit_kmalloc_array(test, data->npages, sizeof(*sg), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sg);
+	sg_init_table(sg, data->npages);
+	memset(&data->sgt, 0, sizeof(data->sgt));
+	data->sgt.orig_nents = data->npages;
+	data->sgt.sgl = sg;
+
+	data->buffer = iov_kunit_create_buffer(test, &data->pages,
+					       data->npages);
+	data->scratch = iov_kunit_create_buffer(test, &spages, data->npages);
+	for (i = 0; i < bufsize; ++i)
+		data->buffer[i] = pattern(i);
+	memset(data->scratch, 0, bufsize);
+}
+
+static void __init
+iov_kunit_iter_to_sg_check(struct kunit *test, struct iov_iter *iter,
+			   size_t bufsize,
+			   struct iov_kunit_iter_to_sg_data *data)
+{
+	size_t i;
+
+	i = extract_iter_to_sg(iter, bufsize, &data->sgt, 0, 0);
+	KUNIT_EXPECT_EQ(test, i, 0);
+	KUNIT_EXPECT_EQ(test, data->sgt.nents, 0);
+
+	i = extract_iter_to_sg(iter, bufsize, &data->sgt, 1, 0);
+	KUNIT_EXPECT_LE(test, i, bufsize);
+	KUNIT_EXPECT_EQ(test, data->sgt.nents, 1);
+
+	i += extract_iter_to_sg(iter, bufsize - i, &data->sgt,
+				data->npages - data->sgt.nents, 0);
+
+	KUNIT_EXPECT_EQ(test, i, bufsize);
+	KUNIT_EXPECT_LE(test, data->sgt.nents, data->npages);
+	sg_mark_end(&data->sgt.sgl[data->sgt.nents - 1]);
+
+
+	i = sg_copy_to_buffer(data->sgt.sgl, data->sgt.nents,
+			      data->scratch, bufsize);
+	KUNIT_EXPECT_EQ(test, i, bufsize);
+
+	for (i = 0; i < bufsize; ++i) {
+		KUNIT_EXPECT_EQ_MSG(test, data->buffer[i], data->scratch[i],
+				    "at i=%zx", i);
+		if (data->buffer[i] != data->scratch[i])
+			break;
+	}
+
+	KUNIT_EXPECT_EQ(test, i, bufsize);
+}
+
+static void __init iov_kunit_iter_to_sg_kvec(struct kunit *test)
+{
+	struct iov_kunit_iter_to_sg_data data;
+	struct iov_iter iter;
+	struct kvec kvec;
+	size_t bufsize;
+
+	bufsize = 0x100000;
+	iov_kunit_iter_to_sg_init(test, bufsize, &data);
+
+	kvec.iov_base = data.buffer;
+	kvec.iov_len = bufsize;
+	iov_iter_kvec(&iter, READ, &kvec, 1, bufsize);
+
+	iov_kunit_iter_to_sg_check(test, &iter, bufsize, &data);
+}
+
+static void __init iov_kunit_iter_to_sg_bvec(struct kunit *test)
+{
+	struct iov_kunit_iter_to_sg_data data;
+	struct page *p, *can_merge = NULL;
+	size_t i, k, bufsize;
+	struct bio_vec *bvec;
+	struct iov_iter iter;
+
+	bufsize = 0x100000;
+	iov_kunit_iter_to_sg_init(test, bufsize, &data);
+
+	bvec = kunit_kmalloc_array(test, data.npages, sizeof(*bvec),
+				   GFP_KERNEL);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bvec);
+	k = 0;
+	for (i = 0; i < data.npages; ++i) {
+		p = data.pages[i];
+		if (p == can_merge)
+			bvec[k-1].bv_len += PAGE_SIZE;
+		else
+			bvec_set_page(&bvec[k++], p, PAGE_SIZE, 0);
+		can_merge = p + 1;
+	}
+	iov_iter_bvec(&iter, READ, bvec, k, bufsize);
+
+	iov_kunit_iter_to_sg_check(test, &iter, bufsize, &data);
+}
+
+static void __init iov_kunit_iter_to_sg_folioq(struct kunit *test)
+{
+	struct iov_kunit_iter_to_sg_data data;
+	struct folio_queue *folioq;
+	struct iov_iter iter;
+	size_t bufsize;
+
+	bufsize = 0x100000;
+	iov_kunit_iter_to_sg_init(test, bufsize, &data);
+
+	folioq = iov_kunit_create_folioq(test);
+	iov_kunit_load_folioq(test, &iter, READ, folioq, data.pages,
+			      data.npages);
+
+	iov_kunit_iter_to_sg_check(test, &iter, bufsize, &data);
+}
+
+static void __init iov_kunit_iter_to_sg_xarray(struct kunit *test)
+{
+	struct iov_kunit_iter_to_sg_data data;
+	struct xarray *xarray;
+	struct iov_iter iter;
+	size_t bufsize;
+
+	bufsize = 0x100000;
+	iov_kunit_iter_to_sg_init(test, bufsize, &data);
+
+	xarray = iov_kunit_create_xarray(test);
+	iov_kunit_load_xarray(test, &iter, READ, xarray, data.pages,
+			      data.npages);
+
+	iov_kunit_iter_to_sg_check(test, &iter, bufsize, &data);
+}
+
 static struct kunit_case __refdata iov_kunit_cases[] = {
 	KUNIT_CASE(iov_kunit_copy_to_kvec),
 	KUNIT_CASE(iov_kunit_copy_from_kvec),
@@ -1027,6 +1175,10 @@ static struct kunit_case __refdata iov_kunit_cases[] = {
 	KUNIT_CASE(iov_kunit_extract_pages_bvec),
 	KUNIT_CASE(iov_kunit_extract_pages_folioq),
 	KUNIT_CASE(iov_kunit_extract_pages_xarray),
+	KUNIT_CASE(iov_kunit_iter_to_sg_kvec),
+	KUNIT_CASE(iov_kunit_iter_to_sg_bvec),
+	KUNIT_CASE(iov_kunit_iter_to_sg_folioq),
+	KUNIT_CASE(iov_kunit_iter_to_sg_xarray),
 	{}
 };
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 0/3] Fix length calculation bug in extract_kvec_to_sg
  2026-03-24 20:34 [PATCH v2 0/3] Fix length calculation bug in extract_kvec_to_sg Christian A. Ehrhardt
                   ` (2 preceding siblings ...)
  2026-03-24 20:34 ` [PATCH v2 3/3] lib: kunit_iov_iter: Add tests for extract_iter_to_sg Christian A. Ehrhardt
@ 2026-03-24 20:41 ` Josh Law
  3 siblings, 0 replies; 12+ messages in thread
From: Josh Law @ 2026-03-24 20:41 UTC (permalink / raw)
  To: Christian A. Ehrhardt, linux-kernel, Andrew Morton, David Howells
  Cc: Kees Cook, Petr Mladek, David Gow



On 24 March 2026 20:34:50 GMT, "Christian A. Ehrhardt" <lk@c--e.de> wrote:
>There is a bug in extract_kvec_to_sg() where the length
>of a scatterlist segment is miscalculated. The actual fix
>is a one-liner and it is quite obvious from reading the
>code that this is what was intened.
>
>As this is a core library function this series first adds
>test cases to the kunit_iov_iter test that demonstrate that
>there is a bug before actually fixing it in the last commit.
>
>The bug was orignally introduced into kernel v6.3 where the
>function lived in fs/netfs/iterator.c. It was later moved
>to lib/scatterlist.c in v6.5. Thus the actual fix is only
>marked for backports to v6.5+.
>
>---
>Changes in v2:
>Addresss valid issues raised by AI review
>https://sashiko.dev/#/patchset/20260323212350.807118-1-lk@c--e.de:
>- Add kunit assertions for OOM conditions in the test
>- Reorder commits.
>- Fix sg_max == 0 case.
>- Fix return value if we run out of sg entries.
>- Adjust tests to catch these cases, too.
>---
>
>Christian A. Ehrhardt (3):
>  lib: kunit_iov_iter: Improve error detection
>  lib: kunit_iov_iter: Add tests for extract_iter_to_sg
>  lib: Fix length calculation in extract_kvec_to_sg
>
> lib/scatterlist.c          |   2 +-
> lib/tests/kunit_iov_iter.c | 147 ++++++++++++++++++++++++++++++++++++-
> 2 files changed, 147 insertions(+), 2 deletions(-)
>




Hello again Christian!


Great job fixing those AI reviews! Those are sometimes important, And your code is completely solid, i personally tested it all for you :) 


Cc: Stable is also justified

I will add what was good about this code on the respective threads


Whole series: 
Reviewed-by: Josh Law <objecting@objecting.org >
Tested-By: Josh Law <objecting@objecting.org>



V/R


Josh Law

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] lib: kunit_iov_iter: Improve error detection
  2026-03-24 20:34 ` [PATCH v2 1/3] lib: kunit_iov_iter: Improve error detection Christian A. Ehrhardt
@ 2026-03-24 20:50   ` Josh Law
  2026-03-24 23:18   ` David Howells
  1 sibling, 0 replies; 12+ messages in thread
From: Josh Law @ 2026-03-24 20:50 UTC (permalink / raw)
  To: Christian A. Ehrhardt, linux-kernel, Andrew Morton, David Howells
  Cc: Kees Cook, Petr Mladek, David Gow



On 24 March 2026 20:34:51 GMT, "Christian A. Ehrhardt" <lk@c--e.de> wrote:
>In the kunit_iov_iter test prevent the kernel buffer from
>being a single physically contiguous region.
>
>Additionally, make sure that the test pattern written to
>a page in the buffer depends on the offset of the page within
>the buffer.
>
>Cc: David Howells <dhowells@redhat.com>
>Cc: Andrew Morton <akpm@linux-foundation.org>
>Signed-off-by: Christian A. Ehrhardt <lk@c--e.de>
>---
> lib/tests/kunit_iov_iter.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/lib/tests/kunit_iov_iter.c b/lib/tests/kunit_iov_iter.c
>index bb847e5010eb..c42b9235f57a 100644
>--- a/lib/tests/kunit_iov_iter.c
>+++ b/lib/tests/kunit_iov_iter.c
>@@ -13,6 +13,7 @@
> #include <linux/uio.h>
> #include <linux/bvec.h>
> #include <linux/folio_queue.h>
>+#include <linux/minmax.h>
> #include <kunit/test.h>
> MODULE_DESCRIPTION("iov_iter testing");
>@@ -37,7 +38,7 @@ static const struct kvec_test_range kvec_test_ranges[] = {
> 
> static inline u8 pattern(unsigned long x)
> {
>-	return x & 0xff;
>+	return (u8)x + (u8)(x >> 8) + (u8)(x >> 16);

Suggestion: You could use bitwise XOR if you like
return (x ^ (x >> 8) ^ (x >> 16)) & 0xff;


> }
> 
> static void iov_kunit_unmap(void *data)
>@@ -52,6 +53,7 @@ static void *__init iov_kunit_create_buffer(struct kunit *test,
> 	struct page **pages;
> 	unsigned long got;
> 	void *buffer;
>+	unsigned int i;

Nit: unsigned long is probably better here

> 
> 	pages = kunit_kcalloc(test, npages, sizeof(struct page *), GFP_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, pages);
>@@ -62,6 +64,9 @@ static void *__init iov_kunit_create_buffer(struct kunit *test,
> 		release_pages(pages, got);
> 		KUNIT_ASSERT_EQ(test, got, npages);
> 	}
>+	/* Make sure that we don't get a physically contiguous buffer. */
>+	for (i = 0; i < npages / 4; ++i)

I do really like this method, but

This may not be achievable, but if npages ever pass less than 4, it could evaluate to 0

>+		swap(pages[i], pages[i + npages/2]);

Nit: The spacing is a bit odd, you could change it to
swap(pages[i], pages[i + npages / 2]);

> 
> 	buffer = vmap(pages, npages, VM_MAP | VM_MAP_PUT_PAGES, PAGE_KERNEL);
>         KUNIT_ASSERT_NOT_ERR_OR_NULL(test, buffer);



This doesn't affect my reviewed by,  but just some things to clean up

V/R


Josh Law

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/3] lib: Fix length calculations in extract_kvec_to_sg
  2026-03-24 20:34 ` [PATCH v2 2/3] lib: Fix length calculations in extract_kvec_to_sg Christian A. Ehrhardt
@ 2026-03-24 20:54   ` Josh Law
  0 siblings, 0 replies; 12+ messages in thread
From: Josh Law @ 2026-03-24 20:54 UTC (permalink / raw)
  To: Christian A. Ehrhardt, linux-kernel, Andrew Morton, David Howells
  Cc: Kees Cook, Petr Mladek, David Gow



On 24 March 2026 20:34:52 GMT, "Christian A. Ehrhardt" <lk@c--e.de> wrote:
>When extracting from a kvec to a scatterlist, do not
>cross page boundaries. The required length is already
>calculated but not used as intended.
>
>Adjust the copied length if the loop runs out auf
>sglist entries.

Nit: "auf" > of?? (Probably a typo)

>While there return immediately from extract_iter_to_sg
>if there are no sglist entries at all.

Suggestion: You could change it  to "While there**,** return immediately from extract_iter_to_sg if there are no sglist entries at all." It is something a bit "silly" so it doesn't matter much

>The changes to the kunit_iov_iter.c in the next commit
>demonstrate that the patch is necessary.
>
>Cc: David Howells <dhowells@redhat.com>
>Cc: Andrew Morton <akpm@linux-foundation.org>
>Cc: stable@vger.kernel.org # v6.5+
>Fixes: 018584697533 ("netfs: Add a function to extract an iterator into a scatterlist")
>Signed-off-by: Christian A. Ehrhardt <lk@c--e.de>
>---
> lib/scatterlist.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/lib/scatterlist.c b/lib/scatterlist.c
>index d773720d11bf..befdc4b9c11d 100644
>--- a/lib/scatterlist.c
>+++ b/lib/scatterlist.c
>@@ -1247,7 +1247,7 @@ static ssize_t extract_kvec_to_sg(struct iov_iter *iter,
> 			else
> 				page = virt_to_page((void *)kaddr);
> 
>-			sg_set_page(sg, page, len, off);
>+			sg_set_page(sg, page, seg, off);
> 			sgtable->nents++;
> 			sg++;
> 			sg_max--;
>@@ -1256,6 +1256,7 @@ static ssize_t extract_kvec_to_sg(struct iov_iter *iter,
> 			kaddr += PAGE_SIZE;
> 			off = 0;
> 		} while (len > 0 && sg_max > 0);
>+		ret -= len;
> 
> 		if (maxsize <= 0 || sg_max == 0)
> 			break;
>@@ -1409,7 +1410,7 @@ ssize_t extract_iter_to_sg(struct iov_iter *iter, size_t maxsize,
> 			   struct sg_table *sgtable, unsigned int sg_max,
> 			   iov_iter_extraction_t extraction_flags)
> {
>-	if (maxsize == 0)
>+	if (maxsize == 0 || sg_max == 0)
> 		return 0;
> 
> 	switch (iov_iter_type(iter)) {


This C is absolutely perfect! No nits here


V/R 


Josh Law

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] lib: kunit_iov_iter: Add tests for extract_iter_to_sg
  2026-03-24 20:34 ` [PATCH v2 3/3] lib: kunit_iov_iter: Add tests for extract_iter_to_sg Christian A. Ehrhardt
@ 2026-03-24 20:58   ` Josh Law
  2026-03-24 23:14   ` David Howells
  1 sibling, 0 replies; 12+ messages in thread
From: Josh Law @ 2026-03-24 20:58 UTC (permalink / raw)
  To: Christian A. Ehrhardt, linux-kernel, Andrew Morton, David Howells
  Cc: Kees Cook, Petr Mladek, David Gow



On 24 March 2026 20:34:53 GMT, "Christian A. Ehrhardt" <lk@c--e.de> wrote:
>Add test cases that test extract_iter_to_sg.
>
>For each iterator type an iterator is loaded with a kernel
>buffer. The iterator is then extracted to a scatterlist with
>multiple calls to extract_iter_to_sg. The final scatterlist
>is copied into a scratch buffer.
>
>The test passes if the scratch buffer compares equal to the
>original buffer.
>
>The new tests demostrate bugs in extract_iter_to_sg
>for kvec iterators that are fixed by the previous
>commit.

Nit: just a quick typo here: change demostrate to demonstrate.

>Cc: David Howells <dhowells@redhat.com>
>Cc: Andrew Morton <akpm@linux-foundation.org>
>Signed-off-by: Christian A. Ehrhardt <lk@c--e.de>
>---
> lib/tests/kunit_iov_iter.c | 152 +++++++++++++++++++++++++++++++++++++
> 1 file changed, 152 insertions(+)
>
>diff --git a/lib/tests/kunit_iov_iter.c b/lib/tests/kunit_iov_iter.c
>index c42b9235f57a..159f61c9a30f 100644
>--- a/lib/tests/kunit_iov_iter.c
>+++ b/lib/tests/kunit_iov_iter.c
>@@ -13,6 +13,7 @@
> #include <linux/uio.h>
> #include <linux/bvec.h>
> #include <linux/folio_queue.h>
>+#include <linux/scatterlist.h>
> #include <linux/minmax.h>
> #include <kunit/test.h>
> 
>@@ -1014,6 +1015,153 @@ static void __init iov_kunit_extract_pages_xarray(struct kunit *test)
> 	KUNIT_SUCCEED(test);
> }
> 
>+struct iov_kunit_iter_to_sg_data {
>+	struct sg_table sgt;
>+	u8 *buffer, *scratch;
>+	struct page **pages;
>+	size_t npages;
>+};
>+
>+static void __init
>+iov_kunit_iter_to_sg_init(struct kunit *test, size_t bufsize,
>+			  struct iov_kunit_iter_to_sg_data *data)
>+{
>+	struct page **spages;
>+	struct scatterlist *sg;
>+	size_t i;
>+
>+	data->npages = bufsize / PAGE_SIZE;
>+	sg = kunit_kmalloc_array(test, data->npages, sizeof(*sg), GFP_KERNEL);
>+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sg);
>+	sg_init_table(sg, data->npages);
>+	memset(&data->sgt, 0, sizeof(data->sgt));
>+	data->sgt.orig_nents = data->npages;
>+	data->sgt.sgl = sg;

Good code here

>+
>+	data->buffer = iov_kunit_create_buffer(test, &data->pages,
>+					       data->npages);
>+	data->scratch = iov_kunit_create_buffer(test, &spages, data->npages);
>+	for (i = 0; i < bufsize; ++i)
>+		data->buffer[i] = pattern(i);
>+	memset(data->scratch, 0, bufsize);
>+}
>+
>+static void __init
>+iov_kunit_iter_to_sg_check(struct kunit *test, struct iov_iter *iter,
>+			   size_t bufsize,
>+			   struct iov_kunit_iter_to_sg_data *data)
>+{
>+	size_t i;
>+
>+	i = extract_iter_to_sg(iter, bufsize, &data->sgt, 0, 0);
>+	KUNIT_EXPECT_EQ(test, i, 0);
>+	KUNIT_EXPECT_EQ(test, data->sgt.nents, 0);
>+
>+	i = extract_iter_to_sg(iter, bufsize, &data->sgt, 1, 0);
>+	KUNIT_EXPECT_LE(test, i, bufsize);
>+	KUNIT_EXPECT_EQ(test, data->sgt.nents, 1);
>+
>+	i += extract_iter_to_sg(iter, bufsize - i, &data->sgt,
>+				data->npages - data->sgt.nents, 0);
>+
>+	KUNIT_EXPECT_EQ(test, i, bufsize);
>+	KUNIT_EXPECT_LE(test, data->sgt.nents, data->npages);
>+	sg_mark_end(&data->sgt.sgl[data->sgt.nents - 1]);
>+
>+
>+	i = sg_copy_to_buffer(data->sgt.sgl, data->sgt.nents,
>+			      data->scratch, bufsize);
>+	KUNIT_EXPECT_EQ(test, i, bufsize);
>+
>+	for (i = 0; i < bufsize; ++i) {
>+		KUNIT_EXPECT_EQ_MSG(test, data->buffer[i], data->scratch[i],
>+				    "at i=%zx", i);
>+		if (data->buffer[i] != data->scratch[i])
>+			break;
>+	}

Nit: While KUNIT_EXPECT_EQ_MSG is great, checking byte-by-byte for a 1MB buffer (bufsize = 0x100000) inside a loop can be a bit slow in KUnit if failures occur and it tries to print thousands of messages before breaking. 
The if (...) break; handles the spew perfectly, but you could also just use a memcmp() first, and only drop into the loop to find the exact offset if the memcmp() fails.

    if (memcmp(data->buffer, data->scratch, bufsize)) {
        for (i = 0; i < bufsize; ++i) {
            KUNIT_EXPECT_EQ_MSG(test, data->buffer[i], data->scratch[i],
                        "at i=%zx", i);
            if (data->buffer[i] != data->scratch[i])
                break;
        }
    }

Could be good? (Completely optional!)

>+
>+	KUNIT_EXPECT_EQ(test, i, bufsize);
>+}
>+
>+static void __init iov_kunit_iter_to_sg_kvec(struct kunit *test)
>+{
>+	struct iov_kunit_iter_to_sg_data data;
>+	struct iov_iter iter;
>+	struct kvec kvec;
>+	size_t bufsize;
>+
>+	bufsize = 0x100000;
>+	iov_kunit_iter_to_sg_init(test, bufsize, &data);
>+
>+	kvec.iov_base = data.buffer;
>+	kvec.iov_len = bufsize;
>+	iov_iter_kvec(&iter, READ, &kvec, 1, bufsize);
>+
>+	iov_kunit_iter_to_sg_check(test, &iter, bufsize, &data);
>+}
>+
>+static void __init iov_kunit_iter_to_sg_bvec(struct kunit *test)
>+{
>+	struct iov_kunit_iter_to_sg_data data;
>+	struct page *p, *can_merge = NULL;
>+	size_t i, k, bufsize;
>+	struct bio_vec *bvec;
>+	struct iov_iter iter;
>+
>+	bufsize = 0x100000;
>+	iov_kunit_iter_to_sg_init(test, bufsize, &data);
>+
>+	bvec = kunit_kmalloc_array(test, data.npages, sizeof(*bvec),
>+				   GFP_KERNEL);
>+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bvec);
>+	k = 0;
>+	for (i = 0; i < data.npages; ++i) {
>+		p = data.pages[i];
>+		if (p == can_merge)
>+			bvec[k-1].bv_len += PAGE_SIZE;
>+		else
>+			bvec_set_page(&bvec[k++], p, PAGE_SIZE, 0);
>+		can_merge = p + 1;
>+	}
>+	iov_iter_bvec(&iter, READ, bvec, k, bufsize);
>+
>+	iov_kunit_iter_to_sg_check(test, &iter, bufsize, &data);
>+}
>+
>+static void __init iov_kunit_iter_to_sg_folioq(struct kunit *test)
>+{
>+	struct iov_kunit_iter_to_sg_data data;
>+	struct folio_queue *folioq;
>+	struct iov_iter iter;
>+	size_t bufsize;
>+
>+	bufsize = 0x100000;
>+	iov_kunit_iter_to_sg_init(test, bufsize, &data);
>+
>+	folioq = iov_kunit_create_folioq(test);
>+	iov_kunit_load_folioq(test, &iter, READ, folioq, data.pages,
>+			      data.npages);
>+
>+	iov_kunit_iter_to_sg_check(test, &iter, bufsize, &data);
>+}
>+
>+static void __init iov_kunit_iter_to_sg_xarray(struct kunit *test)
>+{
>+	struct iov_kunit_iter_to_sg_data data;
>+	struct xarray *xarray;
>+	struct iov_iter iter;
>+	size_t bufsize;
>+
>+	bufsize = 0x100000;
>+	iov_kunit_iter_to_sg_init(test, bufsize, &data);
>+
>+	xarray = iov_kunit_create_xarray(test);
>+	iov_kunit_load_xarray(test, &iter, READ, xarray, data.pages,
>+			      data.npages);
>+
>+	iov_kunit_iter_to_sg_check(test, &iter, bufsize, &data);
>+}
>+
> static struct kunit_case __refdata iov_kunit_cases[] = {
> 	KUNIT_CASE(iov_kunit_copy_to_kvec),
> 	KUNIT_CASE(iov_kunit_copy_from_kvec),
>@@ -1027,6 +1175,10 @@ static struct kunit_case __refdata iov_kunit_cases[] = {
> 	KUNIT_CASE(iov_kunit_extract_pages_bvec),
> 	KUNIT_CASE(iov_kunit_extract_pages_folioq),
> 	KUNIT_CASE(iov_kunit_extract_pages_xarray),
>+	KUNIT_CASE(iov_kunit_iter_to_sg_kvec),
>+	KUNIT_CASE(iov_kunit_iter_to_sg_bvec),
>+	KUNIT_CASE(iov_kunit_iter_to_sg_folioq),
>+	KUNIT_CASE(iov_kunit_iter_to_sg_xarray),
> 	{}
> };
> 



Good enough, my reviewed by isn't affected


V/R


Josh Law

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] lib: kunit_iov_iter: Add tests for extract_iter_to_sg
  2026-03-24 20:34 ` [PATCH v2 3/3] lib: kunit_iov_iter: Add tests for extract_iter_to_sg Christian A. Ehrhardt
  2026-03-24 20:58   ` Josh Law
@ 2026-03-24 23:14   ` David Howells
  2026-03-24 23:17     ` Josh Law
  1 sibling, 1 reply; 12+ messages in thread
From: David Howells @ 2026-03-24 23:14 UTC (permalink / raw)
  To: Josh Law
  Cc: dhowells, Christian A. Ehrhardt, linux-kernel, Andrew Morton,
	Kees Cook, Petr Mladek, David Gow

Josh Law <objecting@objecting.org> wrote:

>     if (memcmp(data->buffer, data->scratch, bufsize)) {

If you do this, please do "memcmp(...) != 0".  It's not a boolean function and
the sense is effectively inverted.

David


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 3/3] lib: kunit_iov_iter: Add tests for extract_iter_to_sg
  2026-03-24 23:14   ` David Howells
@ 2026-03-24 23:17     ` Josh Law
  0 siblings, 0 replies; 12+ messages in thread
From: Josh Law @ 2026-03-24 23:17 UTC (permalink / raw)
  To: David Howells
  Cc: dhowells, Christian A. Ehrhardt, linux-kernel, Andrew Morton,
	Kees Cook, Petr Mladek, David Gow



On 24 March 2026 23:14:31 GMT, David Howells <dhowells@redhat.com> wrote:
>Josh Law <objecting@objecting.org> wrote:
>
>>     if (memcmp(data->buffer, data->scratch, bufsize)) {
>
>If you do this, please do "memcmp(...) != 0".  It's not a boolean function and
>the sense is effectively inverted.
>
>David
>


Oh yeah, good catch. My mistake

Yeah, David is right


V/R


Josh Law

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] lib: kunit_iov_iter: Improve error detection
  2026-03-24 20:34 ` [PATCH v2 1/3] lib: kunit_iov_iter: Improve error detection Christian A. Ehrhardt
  2026-03-24 20:50   ` Josh Law
@ 2026-03-24 23:18   ` David Howells
  2026-03-24 23:22     ` Josh Law
  1 sibling, 1 reply; 12+ messages in thread
From: David Howells @ 2026-03-24 23:18 UTC (permalink / raw)
  To: Josh Law
  Cc: dhowells, Christian A. Ehrhardt, linux-kernel, Andrew Morton,
	Kees Cook, Petr Mladek, David Gow

Josh Law <objecting@objecting.org> wrote:

> >+	unsigned int i;
> 
> Nit: unsigned long is probably better here

Why?

> >+	for (i = 0; i < npages / 4; ++i)

I would suggest putting the type here:

	for (int i = 0; ...)

sort of thing.

David


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 1/3] lib: kunit_iov_iter: Improve error detection
  2026-03-24 23:18   ` David Howells
@ 2026-03-24 23:22     ` Josh Law
  0 siblings, 0 replies; 12+ messages in thread
From: Josh Law @ 2026-03-24 23:22 UTC (permalink / raw)
  To: David Howells
  Cc: dhowells, Christian A. Ehrhardt, linux-kernel, Andrew Morton,
	Kees Cook, Petr Mladek, David Gow



On 24 March 2026 23:18:49 GMT, David Howells <dhowells@redhat.com> wrote:
>Josh Law <objecting@objecting.org> wrote:
>
>> >+	unsigned int i;
>> 
>> Nit: unsigned long is probably better here
>
>Why?
>
>> >+	for (i = 0; i < npages / 4; ++i)
>
>I would suggest putting the type here:
>
>	for (int i = 0; ...)
>
>sort of thing.
>
>David
>


Hmmm. You declare unsigned int i; but the total number of pages (npages) and the got counter are usually unsigned long in this context. But its a small nit, nothing that changes my opinion on this being merged :-)


V/R


Josh Law

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-03-24 23:22 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-24 20:34 [PATCH v2 0/3] Fix length calculation bug in extract_kvec_to_sg Christian A. Ehrhardt
2026-03-24 20:34 ` [PATCH v2 1/3] lib: kunit_iov_iter: Improve error detection Christian A. Ehrhardt
2026-03-24 20:50   ` Josh Law
2026-03-24 23:18   ` David Howells
2026-03-24 23:22     ` Josh Law
2026-03-24 20:34 ` [PATCH v2 2/3] lib: Fix length calculations in extract_kvec_to_sg Christian A. Ehrhardt
2026-03-24 20:54   ` Josh Law
2026-03-24 20:34 ` [PATCH v2 3/3] lib: kunit_iov_iter: Add tests for extract_iter_to_sg Christian A. Ehrhardt
2026-03-24 20:58   ` Josh Law
2026-03-24 23:14   ` David Howells
2026-03-24 23:17     ` Josh Law
2026-03-24 20:41 ` [PATCH v2 0/3] Fix length calculation bug in extract_kvec_to_sg Josh Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®