[PATCH 0/2] lib: scatterlist: fix sg_split() partial-coverage geometry + add KUnit tests

[PATCH 0/2] lib: scatterlist: fix sg_split() partial-coverage geometry + add KUnit tests

[Charles Pellegrini]

sg_calculate_split() can decrement its nb_splits counter once too often
and miss the loop-termination check, corrupting the last output split's
geometry. This series fixes that and adds a KUnit suite.

Patch 1 is a one-line fix: the termination check tests `!nb_splits`, but
on partial coverage that ends mid-entry of a non-last input entry the
counter overshoots to -1, so the loop runs one iteration too many and
folds a trailing input entry into the last split (it ends up covering
more bytes than the caller requested). Widening the check to
`nb_splits <= 0` catches the overshoot.

Patch 2 adds a KUnit suite (16 cases) over the (input shape, skip, split
sizes) matrix: full / edge-aligned / mid-entry partial coverage, skip
variants, and multi-split runs. The mid-entry cases fail without patch 1
and pass with it.

Prior art / attribution. I hit this independently via property-based
testing and only afterwards found that Alexander Egorenkov had reported
it in 2021 [1], with an identical root-cause analysis. The independent
rediscovery and the matching diagnosis are good evidence this is a real
defect rather than a misreading of the partial-coverage contract. That
2021 posting went only to linux-kernel@vger, received no review, and was
never applied -- this file has no MAINTAINERS entry, which is likely why,
so I'm addressing the lib/ maintainer directly this time.

Egorenkov's proposed fix moved the termination check rather than widening
it, which fixes the mid-entry case but regresses edge-aligned partial
coverage (a split ending exactly on an input-entry boundary with further
entries trailing) -- a case the current code handles correctly. Test
sg_split_t10_edge_first_two_trailing in patch 2 fails his fix and passes
the one here, which is the concrete reason for choosing `nb_splits <= 0`
over the reorder. He is kept as Reported-by; the diagnosis is his.

Impact. No in-tree caller triggers this today. Four of the five callers
(SEC, pxa_camera, spi-omap2-mcspi, and sa2ul via its DMA-mapped re-split)
request full coverage; the one caller that requests partial coverage
(DTHEv2 AEAD) bounds its consumer by cryptlen, masking the wrong
geometry. The fix matters for callers that legitimately request partial
coverage, which sg_split() has documented as supported since it was added
("the union of spans of all resulting scatter lists is a subrange of the
span of the original scatter list").

Testing. Verified against the 16-case KUnit suite: 9 pass / 7 fail on
mainline, 16/16 with patch 1 applied. The seven failures are exactly the
mid-entry partial-coverage cases. The same 16 cases were first developed
and run as a standalone userspace harness against an extracted copy of
lib/sg_split.c, with identical results.

[1] https://lore.kernel.org/all/20210418143425.22944-1-egorenar-dev@posteo.net/

Charles Pellegrini (2):
  lib: scatterlist: fix sg_calculate_split() nb_splits overshoot on
    partial coverage
  lib: scatterlist: add KUnit tests for sg_split()

 lib/Kconfig               |  13 ++
 lib/Makefile              |   1 +
 lib/sg_split.c            |   2 +-
 lib/test_sg_split_kunit.c | 371 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 386 insertions(+), 1 deletion(-)
 create mode 100644 lib/test_sg_split_kunit.c

--
2.47.3

[PATCH 1/2] lib: scatterlist: fix sg_calculate_split() nb_splits overshoot on partial coverage

[Charles Pellegrini]

sg_calculate_split() decrements nb_splits in two places per outer-loop
iteration: once in the inner while() that advances to the next output
split when a split boundary falls mid-entry, and once in the trailing

	if (!size && --nb_splits > 0)

The loop-termination check only tests for nb_splits == 0:

	if (!nb_splits)
		break;

When the final requested split ends mid-entry of a non-last input entry,
both decrements run with their bodies skipped: the inner while() takes
nb_splits 1 -> 0, then the trailing if() takes it 0 -> -1. The
termination check then sees -1, not 0, so the loop does not break and the
next input entry is folded into the last split, inflating its nents and
length_last_sg. The resulting split covers more bytes than requested.
For in=[2, 1], skip=0, nb_splits=1, sizes=[1] the single output split
reports a total length of 2 instead of 1.

Widen the termination check to also catch the overshoot to -1:

	if (nb_splits <= 0)
		break;

The new condition only fires when nb_splits is already negative, which is
reachable solely through the double decrement above, so it cannot change
behaviour for any input that splits correctly today.

This was first reported by Alexander Egorenkov in 2021, with an
identical root-cause analysis; that patch was never reviewed or merged.
Its proposed fix moved the existing termination check above the second
decrement rather than widening it. That handles the mid-entry case but
regresses edge-aligned partial coverage -- a split ending exactly on an
input-entry boundary with further entries trailing -- which the current
code handles correctly. The KUnit test added in the following patch
(sg_split_t10_edge_first_two_trailing) covers that regression.

No in-tree caller triggers the bug today: four request full coverage or
re-split over a DMA-mapped subset, and the one caller that requests
partial coverage (the DTHEv2 AEAD driver) bounds its consumer by
cryptlen, masking the wrong geometry. The fix matters for callers that
legitimately request partial coverage, which the API has documented as
supported since the original commit ("the union of spans of all resulting
scatter lists is a subrange of the span of the original scatter list").

Fixes: f8bcbe62acd0 ("lib: scatterlist: add sg splitting function")
Reported-by: Alexander Egorenkov <egorenar-dev@posteo.net>
Closes: https://lore.kernel.org/all/20210418143425.22944-1-egorenar-dev@posteo.net/
Assisted-by: Claude:claude-opus-4-8 hegel-c
Signed-off-by: Charles Pellegrini <c4ffein.work@gmail.com>
---
 lib/sg_split.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/sg_split.c b/lib/sg_split.c
index 24e8f5e48e63..28fe7fa6c9ac 100644
--- a/lib/sg_split.c
+++ b/lib/sg_split.c
@@ -67,7 +67,7 @@ static int sg_calculate_split(struct scatterlist *in, int nents, int nb_splits,
 			size = *(++sizes);
 		}
 
-		if (!nb_splits)
+		if (nb_splits <= 0)
 			break;
 	}
 
-- 
2.47.3

[PATCH 2/2] lib: scatterlist: add KUnit tests for sg_split()

[Charles Pellegrini]

Add a KUnit suite for lib/sg_split.c covering the (input shape, skip,
split sizes) matrix: full coverage, edge-aligned partial coverage,
mid-entry partial coverage at three boundary positions across one to
three input entries, skip-position variants (skip=0, at an exact entry
edge, and a mid-entry fast-forward), and multi-split runs with mid-entry
transitions. A degenerate sizes-contains-zero case is also exercised.

Each case checks the two documented post-conditions of sg_split(): that
the sum of each output split's entry lengths equals the requested split
size, and that the bytes exposed by each split match the corresponding
region of the input. Backing memory uses a position-unique byte pattern
so a content mismatch pinpoints which input region was mis-exposed.

Against the nb_splits overshoot fixed in the previous patch, the
mid-entry partial cases (e.g. sg_split_t5_minimal_repro) fail without the
fix and pass with it. sg_split_t10_edge_first_two_trailing additionally
guards against the regression introduced by the 2021 fix proposal, which
mishandled edge-aligned partial coverage.

Run with:

	tools/testing/kunit/kunit.py run sg_split

Assisted-by: Claude:claude-opus-4-8 hegel-c
Signed-off-by: Charles Pellegrini <c4ffein.work@gmail.com>
---
 lib/Kconfig               |  13 ++
 lib/Makefile              |   1 +
 lib/test_sg_split_kunit.c | 371 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 385 insertions(+)
 create mode 100644 lib/test_sg_split_kunit.c

diff --git a/lib/Kconfig b/lib/Kconfig
index 00a9509636c1..93276051336b 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -52,6 +52,19 @@ config PACKING_KUNIT_TEST
 
 	  When in doubt, say N.
 
+config SG_SPLIT_KUNIT_TEST
+	tristate "KUnit tests for sg_split" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	select SG_SPLIT
+	default KUNIT_ALL_TESTS
+	help
+	  This builds KUnit tests for lib/sg_split.c.
+
+	  For more information on KUnit and unit tests in general,
+	  please refer to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  When in doubt, say N.
+
 config BITREVERSE
 	tristate
 
diff --git a/lib/Makefile b/lib/Makefile
index f33a24bf1c19..1bedfa027932 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -149,6 +149,7 @@ obj-$(CONFIG_BITREVERSE) += bitrev.o
 obj-$(CONFIG_LINEAR_RANGES) += linear_ranges.o
 obj-$(CONFIG_PACKING)	+= packing.o
 obj-$(CONFIG_PACKING_KUNIT_TEST) += packing_test.o
+obj-$(CONFIG_SG_SPLIT_KUNIT_TEST) += test_sg_split_kunit.o
 obj-$(CONFIG_XXHASH)	+= xxhash.o
 obj-$(CONFIG_GENERIC_ALLOCATOR) += genalloc.o
 
diff --git a/lib/test_sg_split_kunit.c b/lib/test_sg_split_kunit.c
new file mode 100644
index 000000000000..81af01c37233
--- /dev/null
+++ b/lib/test_sg_split_kunit.c
@@ -0,0 +1,371 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for lib/sg_split.c.
+ *
+ * Each case sets up a deterministic input scatterlist (page + offset +
+ * length per entry), calls sg_split, and verifies the two documented
+ * post-conditions:
+ *
+ *   1. sum of out[k] entry lengths equals split_sizes[k]   (per-split size)
+ *   2. the bytes walked from out[k] match input bytes
+ *      [skip + sum(prev sizes) .. +sizes[k])               (per-split content)
+ *
+ * Backing memory is a position-unique byte pattern, so a content
+ * mismatch identifies exactly which input region got mis-exposed.
+ */
+
+#include <kunit/test.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#define SG_SPLIT_MAX_IN_ENTRIES		8
+#define SG_SPLIT_MAX_SPLITS		4
+#define SG_SPLIT_PAGE_BYTES		4096
+
+struct sg_split_in_spec {
+	int		page;
+	unsigned int	off;
+	unsigned int	len;
+};
+
+static u8 sg_split_pattern(int page, unsigned int off)
+{
+	return (u8)(((page * 31u) + off * 17u + 0x5a) & 0xffu);
+}
+
+/*
+ * Build the input SG, run sg_split, and verify both properties.
+ * KUnit handles cleanup of kunit_kzalloc'd memory automatically;
+ * we kfree the per-split output arrays ourselves.
+ */
+static void sg_split_run(struct kunit *test,
+			 const struct sg_split_in_spec *in_spec, int n_in,
+			 off_t skip,
+			 const size_t *sizes, int nb_splits)
+{
+	u8 *backing, *oracle_buf;
+	struct scatterlist *in_sgl;
+	struct scatterlist *out[SG_SPLIT_MAX_SPLITS] = { NULL };
+	int out_nents[SG_SPLIT_MAX_SPLITS] = { 0 };
+	unsigned long oracle_cursor = (unsigned long)skip;
+	size_t oracle_total = 0;
+	int rc, k, i;
+
+	KUNIT_ASSERT_LE(test, n_in, SG_SPLIT_MAX_IN_ENTRIES);
+	KUNIT_ASSERT_LE(test, nb_splits, SG_SPLIT_MAX_SPLITS);
+
+	backing = kunit_kzalloc(test,
+				SG_SPLIT_MAX_IN_ENTRIES * SG_SPLIT_PAGE_BYTES,
+				GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, backing);
+
+	for (int p = 0; p < SG_SPLIT_MAX_IN_ENTRIES; p++)
+		for (unsigned int o = 0; o < SG_SPLIT_PAGE_BYTES; o++)
+			backing[p * SG_SPLIT_PAGE_BYTES + o] =
+				sg_split_pattern(p, o);
+
+	in_sgl = kunit_kzalloc(test, sizeof(*in_sgl) * n_in, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, in_sgl);
+	sg_init_table(in_sgl, n_in);
+	for (i = 0; i < n_in; i++) {
+		u8 *buf = backing + in_spec[i].page * SG_SPLIT_PAGE_BYTES
+			+ in_spec[i].off;
+
+		sg_set_buf(&in_sgl[i], buf, in_spec[i].len);
+		oracle_total += in_spec[i].len;
+	}
+
+	/* Linear concatenation oracle for property 2. */
+	oracle_buf = kunit_kzalloc(test, oracle_total, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, oracle_buf);
+	{
+		size_t op = 0;
+
+		for (i = 0; i < n_in; i++) {
+			memcpy(oracle_buf + op,
+			       backing
+				+ in_spec[i].page * SG_SPLIT_PAGE_BYTES
+				+ in_spec[i].off,
+			       in_spec[i].len);
+			op += in_spec[i].len;
+		}
+	}
+
+	rc = sg_split(in_sgl, n_in, skip, nb_splits, sizes,
+		      out, out_nents, GFP_KERNEL);
+	KUNIT_ASSERT_EQ_MSG(test, rc, 0, "sg_split returned %d", rc);
+
+	for (k = 0; k < nb_splits; k++) {
+		unsigned long sum = 0;
+		struct scatterlist *sg;
+		int j;
+		u8 *got;
+		size_t copied = 0;
+
+		KUNIT_ASSERT_NOT_NULL_MSG(test, out[k],
+					  "split[%d] is NULL", k);
+
+		/* Property 1: per-split size. */
+		for_each_sg(out[k], sg, out_nents[k], j)
+			sum += sg->length;
+		KUNIT_EXPECT_EQ_MSG(test, sum, sizes[k],
+				    "P1 split[%d]: sum=%lu requested=%zu",
+				    k, sum, sizes[k]);
+
+		/* Property 2: per-split content. */
+		got = kunit_kzalloc(test, sizes[k] + 1, GFP_KERNEL);
+		KUNIT_ASSERT_NOT_NULL(test, got);
+		for_each_sg(out[k], sg, out_nents[k], j) {
+			size_t take = min_t(size_t, sg->length,
+					    sizes[k] - copied);
+
+			memcpy(got + copied, sg_virt(sg), take);
+			copied += take;
+			if (copied >= sizes[k])
+				break;
+		}
+		KUNIT_EXPECT_EQ_MSG(test,
+				    memcmp(got, oracle_buf + oracle_cursor,
+					   min_t(size_t, copied, sizes[k])),
+				    0,
+				    "P2 split[%d]: bytes mismatch", k);
+
+		oracle_cursor += sizes[k];
+	}
+
+	for (k = 0; k < nb_splits; k++)
+		kfree(out[k]);
+}
+
+/* ========================================================================
+ * Group A — full or edge-aligned coverage.  These must pass even on the
+ * pre-patch kernel; they exercise the path the existing five upstream
+ * callers all hit.
+ */
+
+static void sg_split_t1_single_entry_full(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 100 } };
+	const size_t sizes[] = { 100 };
+
+	sg_split_run(test, in, 1, 0, sizes, 1);
+}
+
+static void sg_split_t2_two_entries_full_coverage(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 60 }, { 1, 0, 40 } };
+	const size_t sizes[] = { 100 };
+
+	sg_split_run(test, in, 2, 0, sizes, 1);
+}
+
+static void sg_split_t3_two_entries_two_splits_full(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 60 }, { 1, 0, 40 } };
+	const size_t sizes[] = { 50, 50 };
+
+	sg_split_run(test, in, 2, 0, sizes, 2);
+}
+
+static void sg_split_t4_edge_aligned_partial(struct kunit *test)
+{
+	/* Boundary at end of in[0]; partial coverage but edge-aligned. */
+	const struct sg_split_in_spec in[] = { { 0, 0, 60 }, { 1, 0, 40 } };
+	const size_t sizes[] = { 60 };
+
+	sg_split_run(test, in, 2, 0, sizes, 1);
+}
+
+/* ========================================================================
+ * Group B — partial coverage with mid-entry boundary.  These exercise
+ * the contract clause "union of spans is a subrange of the original"
+ * from f8bcbe62acd0.  They fail on the pre-patch kernel.
+ */
+
+static void sg_split_t5_minimal_repro(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 2 }, { 1, 0, 1 } };
+	const size_t sizes[] = { 1 };
+
+	sg_split_run(test, in, 2, 0, sizes, 1);
+}
+
+static void sg_split_t6_dthev2_aead_shape(struct kunit *test)
+{
+	/* AEAD shape: [AAD || ciphertext || TAG]; skip=AAD, take cryptlen,
+	 * leave TAG as trailing tail.  Edge-aligned in this layout, so
+	 * even pre-patch the bug doesn't fire — included to pin that.
+	 */
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 64 }, { 1, 0, 100 }, { 2, 0, 16 }
+	};
+	const size_t sizes[] = { 100 };
+
+	sg_split_run(test, in, 3, 64, sizes, 1);
+}
+
+static void sg_split_t7_skip_plus_partial_midentry(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 100 }, { 1, 0, 50 } };
+	const size_t sizes[] = { 30 };
+
+	sg_split_run(test, in, 2, 20, sizes, 1);
+}
+
+/* ========================================================================
+ * Group C — sizes-containing-zero edge.  Pre-patch behaviour is to
+ * produce a zero-length split and return success.  The break-condition
+ * fix preserves this; a (rejected) earlier attempt at fixing the bug
+ * via `if (!size) continue;` regressed this case to a crash.
+ */
+
+static void sg_split_t8_sizes_trailing_zero(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 10 }, { 1, 0, 5 } };
+	const size_t sizes[] = { 10, 0 };
+
+	sg_split_run(test, in, 2, 0, sizes, 2);
+}
+
+/* ========================================================================
+ * Group D — boundary-position matrix.  Using 3 input entries of 10 bytes
+ * each, walk the (skip + sum(sizes)) boundary through each position to
+ * confirm the trigger condition is "mid-entry AND has trailing input".
+ */
+
+static void sg_split_t9_mid_first_two_trailing(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 }
+	};
+	const size_t sizes[] = { 5 };
+
+	sg_split_run(test, in, 3, 0, sizes, 1);
+}
+
+static void sg_split_t10_edge_first_two_trailing(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 }
+	};
+	const size_t sizes[] = { 10 };
+
+	sg_split_run(test, in, 3, 0, sizes, 1);
+}
+
+static void sg_split_t11_mid_middle_one_trailing(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 }
+	};
+	const size_t sizes[] = { 15 };
+
+	sg_split_run(test, in, 3, 0, sizes, 1);
+}
+
+static void sg_split_t12_mid_last_no_trailing(struct kunit *test)
+{
+	/* Boundary mid-in[2] with NO trailing entries.  Even with the
+	 * pre-patch dual-decrement overshoot, the for_each_sg loop ends
+	 * before iter N+1 can corrupt state.  Confirms the trigger
+	 * narrowness.
+	 */
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 }
+	};
+	const size_t sizes[] = { 25 };
+
+	sg_split_run(test, in, 3, 0, sizes, 1);
+}
+
+/* ========================================================================
+ * Group E — skip-position variations.
+ */
+
+static void sg_split_t13_skip_at_entry_boundary(struct kunit *test)
+{
+	/* skip = exactly len(in[0]).  `skip > sglen` is strict, so the
+	 * main body runs with len=0 in iter 1.  Iter 2 takes 5 from in[1];
+	 * boundary mid-in[1], in[2] trailing.
+	 */
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 }
+	};
+	const size_t sizes[] = { 5 };
+
+	sg_split_run(test, in, 3, 10, sizes, 1);
+}
+
+static void sg_split_t14_skip_fastforward_partial(struct kunit *test)
+{
+	/* skip=12 → iter 1 fast-forwards (skip -= 10, continue); iter 2
+	 * has effective skip=2, takes 5 from in[1] starting at byte 2;
+	 * boundary at byte 17 = mid-in[1] with in[2] trailing.
+	 */
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 }
+	};
+	const size_t sizes[] = { 5 };
+
+	sg_split_run(test, in, 3, 12, sizes, 1);
+}
+
+/* ========================================================================
+ * Group F — multi-split variations.
+ */
+
+static void sg_split_t15_two_splits_last_mid_trailing(struct kunit *test)
+{
+	/* sizes=[5,10] over [10,10,10]: first split mid-in[0], second
+	 * split ends mid-in[1] with in[2] trailing.  Trigger on the LAST
+	 * split.
+	 */
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 }
+	};
+	const size_t sizes[] = { 5, 10 };
+
+	sg_split_run(test, in, 3, 0, sizes, 2);
+}
+
+static void sg_split_t16_two_splits_full_coverage_mid_first(struct kunit *test)
+{
+	/* sizes=[3,17] over [10,10]: first-split boundary mid-in[0],
+	 * second-split boundary at end of in[1].  Full coverage.
+	 */
+	const struct sg_split_in_spec in[] = { { 0, 0, 10 }, { 1, 0, 10 } };
+	const size_t sizes[] = { 3, 17 };
+
+	sg_split_run(test, in, 2, 0, sizes, 2);
+}
+
+static struct kunit_case sg_split_test_cases[] = {
+	KUNIT_CASE(sg_split_t1_single_entry_full),
+	KUNIT_CASE(sg_split_t2_two_entries_full_coverage),
+	KUNIT_CASE(sg_split_t3_two_entries_two_splits_full),
+	KUNIT_CASE(sg_split_t4_edge_aligned_partial),
+	KUNIT_CASE(sg_split_t5_minimal_repro),
+	KUNIT_CASE(sg_split_t6_dthev2_aead_shape),
+	KUNIT_CASE(sg_split_t7_skip_plus_partial_midentry),
+	KUNIT_CASE(sg_split_t8_sizes_trailing_zero),
+	KUNIT_CASE(sg_split_t9_mid_first_two_trailing),
+	KUNIT_CASE(sg_split_t10_edge_first_two_trailing),
+	KUNIT_CASE(sg_split_t11_mid_middle_one_trailing),
+	KUNIT_CASE(sg_split_t12_mid_last_no_trailing),
+	KUNIT_CASE(sg_split_t13_skip_at_entry_boundary),
+	KUNIT_CASE(sg_split_t14_skip_fastforward_partial),
+	KUNIT_CASE(sg_split_t15_two_splits_last_mid_trailing),
+	KUNIT_CASE(sg_split_t16_two_splits_full_coverage_mid_first),
+	{}
+};
+
+static struct kunit_suite sg_split_test_suite = {
+	.name = "sg_split",
+	.test_cases = sg_split_test_cases,
+};
+
+kunit_test_suite(sg_split_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for lib/sg_split.c");
+MODULE_LICENSE("GPL");
-- 
2.47.3

Re: [PATCH 0/2] lib: scatterlist: fix sg_split() partial-coverage geometry + add KUnit tests

[Andrew Morton]

On Sun, 31 May 2026 16:42:20 -0700 (PDT) Charles Pellegrini <c4ffein.work@gmail.com> wrote:

> sg_calculate_split() can decrement its nb_splits counter once too often
> and miss the loop-termination check, corrupting the last output split's
> geometry. This series fixes that and adds a KUnit suite.

Thanks.  Has this been observed in real life, or was the patch motivated by
code review?

AI review
(https://sashiko.dev/#/patchset/178027099087.72481.1976843064458686851@gmail.com)
might have found a pre-existing issue, which you may choose to reject,
fix, add to todo list or ignore.  Also a possible issue in the kunit
test changes.

Please take a look, let us know?

Re: [PATCH 0/2] lib: scatterlist: fix sg_split() partial-coverage geometry + add KUnit tests

[c4ffein.work]

On Mon, 1 Jun 2026 17:55:49 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> Thanks.  Has this been observed in real life, or was the patch motivated by
> code review?

Not observed in real life.
I was crafting a pure C client for the Hegel PBT library (and related
bug-hunting Claude skills). I used known kernel bugs to benchmark various
interfaces and skills. Crafting Hegel tests for sg_split.c led Claude to
report this one to me. I only realized it was actually already reported
later.

> AI review
> (https://sashiko.dev/#/patchset/178027099087.72481.1976843064458686851@gmail.com)
> might have found a pre-existing issue, which you may choose to reject,
> fix, add to todo list or ignore.  Also a possible issue in the kunit
> test changes.
>
> Please take a look, let us know?

I took a look; it raised more questions and led to more changes:

---

1: The first comment noted that the existing code has no guard against a
split_sizes array ending with a 0 size
This is arguably the caller's responsibility, but since it's an
out-of-bounds write in an exported function, I lean toward guarding it
here.
A trailing zero-size split that receives no input entry leaves
out_sg == ZERO_SIZE_PTR, and sg_split_phys()/sg_split_mapped() then write
out_sg[-1].length: an out-of-bounds write.
Confirmed with KASAN, and with a userspace ASAN harness.
Fix: skip empty splits in both copiers.
(It needs a caller to request a trailing zero-size split with no input
left; no in-tree caller does, so it's latent.)

2: The second comment flagged an issue in the kunit suite
The test passes in_mapped_nents = n_in for an unmapped list, where it must
be 0.
On NEED_SG_DMA_LENGTH arches, the mapped pass then reads a zeroed
dma_length and returns -EINVAL.
It passed for me only because UML aliases dma_len onto ->length.
Fix: pass 0.

3: The test finding also turned up a separate pre-existing bug,
also latent.
On !NEED_SG_DMA_LENGTH arches sg_dma_len() aliases ->length, so
sg_split_phys() zeroes, via sg_dma_len(out_sg) = 0, the length it has just
computed.
The trailing out_sg[-1].length assignment restores only the last entry,
so for the non-last entries of a multi-entry split the zero sticks ->
silent short data.
Fix: write ->length after the sg_dma_len() = 0. Verified in-kernel (UML
and x86_64+KASAN) and under ASAN.
(No in-tree caller hits it: the sole unmapped caller, DTHEv2, runs on
arm64 = NEED, which is immune.)

---

v2 grows to 5 patches.
1. the overshoot fix: the exact same initial patch
2. the additional size computation fix
3. the base test suite, improved
  - fixed the second comment from sashiko
  - each case now run both unmapped and identity-mapped
  - added DMA-address + end-marker assertions and NEED-gated
    divergent-geometry cases
4. the zero-size OOB guard
5. its associated regression test

That way, patches 4 and 5 can be skipped if you think we should actually
keep the responsibility on the caller.
The automated review prompted patches 4 and 5 and the test-arg fix in 3;
chasing that fix is what surfaced 2. I'll note that in the commit messages.

I went with tolerate-and-skip for the empty trailing split rather than
rejecting it with -EINVAL, reasoning it's the droppable tail of the
request.
Happy to respin that one patch to -EINVAL if you'd prefer the stricter
contract -- trivial either way.

I'll send v2 threaded under this series shortly.

Thanks,
Charles

[PATCH v2 0/5] lib: scatterlist: fix sg_split() partial-coverage geometry, two latent corruption bugs, and add KUnit tests

[Charles Pellegrini]

v1 was a 2-patch series (the overshoot fix + a KUnit suite). Andrew's
review asked whether this was seen in real life -- it was not; it came
from property-based testing, not a field report -- and pointed at an
automated review (Sashiko) that flagged a pre-existing OOB and a bug in
the test. Chasing those turned up a third, separate latent bug. v2
addresses all of it.

sg_split() is EXPORT_SYMBOL'd, so its contract is "any caller, any
split_sizes[]", not just the six in-tree call sites. None of the three
bugs below is triggered by an in-tree caller today -- all are latent. So
even without a real-world trigger, I think they're worth fixing.

The three bugs (all Fixes: f8bcbe62acd0):

1. sg_calculate_split() nb_splits overshoot (the v1 fix, unchanged).
   Partial coverage ending mid-entry of a non-last input entry overshoots
   the counter to -1, misses the !nb_splits termination check, and folds a
   trailing entry into the last split.

2. sg_split_phys() ->length clobber. On !NEED_SG_DMA_LENGTH arches
   sg_dma_len() aliases ->length, so the DMA-scrub line zeroes the CPU
   length it has just computed; only the last entry is restored, so the
   non-last entries of a multi-entry split keep length 0 -> silent short
   data. Found while correcting the test argument below.

3. zero-nents ZERO_SIZE_PTR OOB. A trailing zero-size split that receives
   no input entry leaves out_sg == ZERO_SIZE_PTR; the out_sg[-1].length
   write is then out-of-bounds (KASAN splat). Flagged by the automated
   review; confirmed with KASAN and a userspace ASAN harness.

Plus a test-argument fix folded into the suite: v1 passed in_mapped_nents
= n_in for an unmapped list, where it must be 0. On NEED arches that drove
the mapped pass to read a zeroed dma_length and return -EINVAL, so the
suite would fail there (it passed on UML only because UML aliases dma_len
onto ->length). Correcting it is what exposed bug 2.

Patch layout (bisect-safe):

  1/5  overshoot fix              -- unchanged from v1
  2/5  ->length clobber fix       -- bug 2; lands before the test that
                                     exposes it
  3/5  KUnit suite                -- corrected in_mapped_nents; each case
                                     run unmapped AND identity-mapped;
                                     DMA-address + end-marker assertions;
                                     NEED-gated divergent-geometry cases
  4/5  zero-nents OOB guard       -- bug 3
  5/5  zero-nents regression test -- guards 4/5

Patches 4 and 5 are an isolable tail: if you'd rather treat a malformed
zero-size split as the caller's problem, they drop cleanly without
touching 1-3. On that bug I went tolerate-and-skip rather than rejecting
with -EINVAL, reasoning it's the droppable tail of the request -- trivial
to respin to -EINVAL if you prefer the stricter contract.

Testing. The suite runs each case both unmapped and identity-mapped
(dma_len == length, contiguous IOVA) on all arches, plus NEED-gated
divergent CPU/DMA geometry (coalescing) cases exercised on x86_64 with
KASAN. Results: UML (!NEED) 17 passed / 3 skipped; x86_64 + KASAN 20/20.
With bug 2 reverted, six unmapped multi-entry cases fail while their
mapped variants pass, so the suite catches it. A real dma_map_sg / IOMMU
rig is still deferred; the mapped coverage here is synthetic (identity
mapping + injected coalescing).

v1: https://lore.kernel.org/all/178027099087.72481.1976843064458686851@gmail.com/

Changes since v1:
 - corrected the KUnit in_mapped_nents argument (0 for unmapped lists)
 - new: sg_split_phys() ->length clobber fix (bug 2)
 - new: zero-nents OOB guard + its regression test (bug 3)
 - tests now run unmapped AND identity-mapped, with DMA-address and
   end-marker assertions and NEED-gated divergent-geometry cases
 - test file moved to lib/tests/sg_split_kunit.c (modern location);
   config moved to lib/Kconfig.debug, Makefile line to lib/tests/Makefile

Charles Pellegrini (5):
  lib: scatterlist: fix sg_calculate_split() nb_splits overshoot on
    partial coverage
  lib: scatterlist: fix sg_split_phys() ->length clobber on
    !NEED_SG_DMA_LENGTH
  lib: scatterlist: add KUnit tests for sg_split()
  lib: scatterlist: guard sg_split_phys()/sg_split_mapped() against
    zero-nents splits
  lib: scatterlist: add zero-nents regression test for sg_split()

 lib/Kconfig.debug          |  14 +
 lib/sg_split.c             |  15 +-
 lib/tests/Makefile         |   1 +
 lib/tests/sg_split_kunit.c | 526 +++++++++++++++++++++++++++++++++++++
 4 files changed, 552 insertions(+), 4 deletions(-)
 create mode 100644 lib/tests/sg_split_kunit.c

-- 
2.47.3

[PATCH v2 1/5] lib: scatterlist: fix sg_calculate_split() nb_splits overshoot on partial coverage

[Charles Pellegrini]

sg_calculate_split() decrements nb_splits in two places per outer-loop
iteration: once in the inner while() that advances to the next output
split when a split boundary falls mid-entry, and once in the trailing

    if (!size && --nb_splits > 0)

The loop-termination check only tests for nb_splits == 0:

    if (!nb_splits)
        break;

When the final requested split ends mid-entry of a non-last input entry,
both decrements run with their bodies skipped: the inner while() takes
nb_splits 1 -> 0, then the trailing if() takes it 0 -> -1. The
termination check then sees -1, not 0, so the loop does not break and the
next input entry is folded into the last split, inflating its nents and
length_last_sg. The resulting split covers more bytes than requested.
For in=[2, 1], skip=0, nb_splits=1, sizes=[1] the single output split
reports a total length of 2 instead of 1.

Widen the termination check to also catch the overshoot to -1:

    if (nb_splits <= 0)
        break;

The new condition only fires when nb_splits is already negative, which is
reachable solely through the double decrement above, so it cannot change
behaviour for any input that splits correctly today.

This was first reported by Alexander Egorenkov in 2021, with an identical
root-cause analysis; that patch was never reviewed or merged. Its proposed
fix moved the existing termination check above the second decrement rather
than widening it. That handles the mid-entry case but regresses
edge-aligned partial coverage -- a split ending exactly on an input-entry
boundary with further entries trailing -- which the current code handles
correctly. A KUnit test added later in this series
(sg_split_t10_edge_first_two_trailing) covers that regression.

No in-tree caller triggers the bug today: four request full coverage or
re-split over a DMA-mapped subset, and the one caller that requests
partial coverage (the DTHEv2 AEAD driver) bounds its consumer by cryptlen,
masking the wrong geometry. The fix matters for callers that legitimately
request partial coverage, which the API has documented as supported since
the original commit ("the union of spans of all resulting scatter lists is
a subrange of the span of the original scatter list").

Fixes: f8bcbe62acd0 ("lib: scatterlist: add sg splitting function")
Reported-by: Alexander Egorenkov <egorenar-dev@posteo.net>
Closes: https://lore.kernel.org/all/20210418143425.22944-1-egorenar-dev@posteo.net/
Assisted-by: Claude:claude-opus-4-8 hegel-c
Signed-off-by: Charles Pellegrini <c4ffein.work@gmail.com>
---
 lib/sg_split.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/sg_split.c b/lib/sg_split.c
index 24e8f5e48e63..28fe7fa6c9ac 100644
--- a/lib/sg_split.c
+++ b/lib/sg_split.c
@@ -67,7 +67,7 @@ static int sg_calculate_split(struct scatterlist *in, int nents, int nb_splits,
 			size = *(++sizes);
 		}
 
-		if (!nb_splits)
+		if (nb_splits <= 0)
 			break;
 	}
 
-- 
2.47.3

[PATCH v2 2/5] lib: scatterlist: fix sg_split_phys() ->length clobber on !NEED_SG_DMA_LENGTH

[Charles Pellegrini]

sg_split_phys() builds each output entry by copying the input entry and
then scrubbing the output's DMA view:

    *out_sg = *in_sg;
    sg_dma_address(out_sg) = 0;
    sg_dma_len(out_sg) = 0;

On configurations where CONFIG_NEED_SG_DMA_LENGTH is not selected,
sg_dma_len() is defined as ((sg)->length): there is no separate
dma_length member. The sg_dma_len(out_sg) = 0 line therefore zeroes the
CPU ->length that the function computes for the split. The trailing

    out_sg[-1].length = split->length_last_sg;

restores only the last entry, so for a multi-entry split every non-last
entry is left with length 0 and the split silently exposes fewer bytes
than requested. Where CONFIG_NEED_SG_DMA_LENGTH is selected (any config
with an IOMMU, and arches such as x86, arm64, powerpc, s390 or sparc)
sg_dma_len() touches the distinct dma_length field and ->length is
untouched, so the bug does not manifest.

Compute the length into a local and assign ->length after the DMA scrub,
so the value survives on both configurations:

    unsigned int len = in_sg->length;

    *out_sg = *in_sg;
    sg_dma_address(out_sg) = 0;
    sg_dma_len(out_sg) = 0;
    if (!j) {
        out_sg->offset += split->skip_sg0;
        len -= split->skip_sg0;
    }
    out_sg->length = len;

The sg_dma_len() = 0 scrub is kept deliberately: on NEED configurations it
clears the dma_length copied by *out_sg = *in_sg, which a consumer would
otherwise read as stale. Deleting it would fix the !NEED case but leak a
stale DMA length on NEED; reordering is correct on both.

No in-tree caller triggers this: the only caller that hands sg_split() an
unmapped list and requests a multi-entry partial split (the DTHEv2 AEAD
driver) builds for an arm64 platform, which selects
CONFIG_NEED_SG_DMA_LENGTH and is therefore immune. It was found while
correcting an in_mapped_nents argument in the KUnit suite (next patch),
which is what first exercised the unmapped multi-entry path on a !NEED
build.

Fixes: f8bcbe62acd0 ("lib: scatterlist: add sg splitting function")
Assisted-by: Claude:claude-opus-4-8 hegel-c
Signed-off-by: Charles Pellegrini <c4ffein.work@gmail.com>
---
 lib/sg_split.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/lib/sg_split.c b/lib/sg_split.c
index 28fe7fa6c9ac..ab574dd26b03 100644
--- a/lib/sg_split.c
+++ b/lib/sg_split.c
@@ -84,13 +84,16 @@ static void sg_split_phys(struct sg_splitter *splitters, const int nb_splits)
 		in_sg = split->in_sg0;
 		out_sg = split->out_sg;
 		for (j = 0; j < split->nents; j++, out_sg++) {
+			unsigned int len = in_sg->length;
+
 			*out_sg = *in_sg;
+			sg_dma_address(out_sg) = 0;
+			sg_dma_len(out_sg) = 0;
 			if (!j) {
 				out_sg->offset += split->skip_sg0;
-				out_sg->length -= split->skip_sg0;
+				len -= split->skip_sg0;
 			}
-			sg_dma_address(out_sg) = 0;
-			sg_dma_len(out_sg) = 0;
+			out_sg->length = len;
 			in_sg = sg_next(in_sg);
 		}
 		out_sg[-1].length = split->length_last_sg;
-- 
2.47.3

[PATCH v2 3/5] lib: scatterlist: add KUnit tests for sg_split()

[Charles Pellegrini]

Add a KUnit suite for lib/sg_split.c covering the (input shape, skip,
split sizes) matrix: full coverage, edge-aligned partial coverage,
mid-entry partial coverage at several boundary positions across one to
three input entries, skip-position variants, and multi-split runs.

Each case checks the two documented post-conditions of sg_split(): the sum
of each output split's entry lengths equals the requested split size, and
the bytes exposed by each split match the corresponding region of the
input. Backing memory uses a position-unique byte pattern so a content
mismatch pinpoints which input region was mis-exposed. Each case also
asserts that exactly one entry per split carries the end-of-list marker.

Every case is run twice: unmapped (in_mapped_nents = 0, exercising
sg_split_phys()) and with an identity DMA mapping (dma_len == length,
contiguous synthetic IOVA, exercising sg_split_mapped() and asserting the
DMA addresses and lengths). A further group exercises divergent CPU/DMA
geometry (coalescing, where the DMA segmentation is coarser than the CPU
one); those need a distinct dma_length field and are skipped where
CONFIG_NEED_SG_DMA_LENGTH is not selected.

The mid-entry partial cases fail without the sg_calculate_split() overshoot
fix in patch 1; sg_split_t10_edge_first_two_trailing additionally guards
the edge-aligned case that the 2021 fix proposal regressed. The unmapped
multi-entry cases fail without the sg_split_phys() ->length fix in patch 2.

Run with:

    tools/testing/kunit/kunit.py run sg_split

Assisted-by: Claude:claude-opus-4-8 hegel-c
Signed-off-by: Charles Pellegrini <c4ffein.work@gmail.com>
---
 lib/Kconfig.debug          |  14 +
 lib/tests/Makefile         |   1 +
 lib/tests/sg_split_kunit.c | 511 +++++++++++++++++++++++++++++++++++++
 3 files changed, 526 insertions(+)
 create mode 100644 lib/tests/sg_split_kunit.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 8ff5adcfe1e0..378414619c88 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2516,6 +2516,20 @@ config SEQ_BUF_KUNIT_TEST
 
 	  If unsure, say N.
 
+config SG_SPLIT_KUNIT_TEST
+	tristate "KUnit tests for sg_split()" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	select SG_SPLIT
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the sg_split() KUnit test suite.
+	  It tests the scatterlist splitting helper defined in
+	  lib/sg_split.c. For more information on KUnit and unit tests
+	  in general please refer to the KUnit documentation in
+	  Documentation/dev-tools/kunit/.
+
+	  If unsure, say N.
+
 config STRING_KUNIT_TEST
 	tristate "KUnit test string functions at runtime" if !KUNIT_ALL_TESTS
 	depends on KUNIT
diff --git a/lib/tests/Makefile b/lib/tests/Makefile
index 7e9c2fa52e35..80c727f6498b 100644
--- a/lib/tests/Makefile
+++ b/lib/tests/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_PRINTF_KUNIT_TEST) += printf_kunit.o
 obj-$(CONFIG_RANDSTRUCT_KUNIT_TEST) += randstruct_kunit.o
 obj-$(CONFIG_SCANF_KUNIT_TEST) += scanf_kunit.o
 obj-$(CONFIG_SEQ_BUF_KUNIT_TEST) += seq_buf_kunit.o
+obj-$(CONFIG_SG_SPLIT_KUNIT_TEST) += sg_split_kunit.o
 obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
 obj-$(CONFIG_SLUB_KUNIT_TEST) += slub_kunit.o
 obj-$(CONFIG_TEST_SORT) += test_sort.o
diff --git a/lib/tests/sg_split_kunit.c b/lib/tests/sg_split_kunit.c
new file mode 100644
index 000000000000..82eef313944d
--- /dev/null
+++ b/lib/tests/sg_split_kunit.c
@@ -0,0 +1,511 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for lib/sg_split.c.
+ *
+ * Each case sets up a deterministic input scatterlist (page + offset +
+ * length per entry), calls sg_split(), and verifies the two documented
+ * post-conditions:
+ *
+ *   1. sum of out[k] entry lengths equals split_sizes[k]   (per-split size)
+ *   2. the bytes walked from out[k] match input bytes
+ *      [skip + sum(prev sizes) .. +sizes[k])               (per-split content)
+ *
+ * Backing memory is a position-unique byte pattern, so a content mismatch
+ * identifies exactly which input region got mis-exposed.
+ *
+ * COVERAGE
+ * --------
+ * Every base case T1..T16 (+ T8b) runs in two modes via sg_split_run():
+ *   - unmapped : in_mapped_nents = 0  -> the CPU-view path (sg_split_phys).
+ *   - mapped   : an identity DMA mapping (dma_len == length, IOVA laid out
+ *                contiguously from SG_SPLIT_DMA_BASE) -> additionally the
+ *                pass-2 / sg_split_mapped() path, asserting the DMA addresses
+ *                and lengths as well.  This is the path the upstream callers
+ *                that pass a non-zero in_mapped_nents actually hit.
+ *
+ * The D* cases exercise DIVERGENT geometry, where the DMA segmentation is
+ * coarser than the CPU one (coalescing): dma_len != length, so pass 2
+ * computes a different per-split nents than pass 1.  They require a real,
+ * separate dma_length field and so are skipped on !NEED_SG_DMA_LENGTH
+ * arches (where sg_dma_len() aliases ->length and divergence cannot be
+ * represented).
+ */
+
+#include <kunit/test.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#define SG_SPLIT_MAX_IN_ENTRIES		8
+#define SG_SPLIT_MAX_SPLITS		4
+#define SG_SPLIT_PAGE_BYTES		4096
+/* Synthetic IOVA base, fits a 32-bit dma_addr_t, clearly not a kernel VA. */
+#define SG_SPLIT_DMA_BASE		((dma_addr_t)0x40000000UL)
+
+struct sg_split_in_spec {
+	int		page;
+	unsigned int	off;
+	unsigned int	len;
+};
+
+static u8 sg_split_pattern(int page, unsigned int off)
+{
+	return (u8)(((page * 31u) + off * 17u + 0x5a) & 0xffu);
+}
+
+/* Verify the DMA view of one split: addresses tile the contiguous IOVA
+ * window [SG_SPLIT_DMA_BASE + cursor ..] and the DMA lengths sum to want.
+ */
+static void sg_split_check_dma(struct kunit *test, const char *mode, int k,
+			       struct scatterlist *out, int dma_nents,
+			       size_t want, unsigned long cursor)
+{
+	struct scatterlist *sg;
+	unsigned long sum = 0, pos = cursor;
+	int j;
+
+	for_each_sg(out, sg, dma_nents, j) {
+		KUNIT_EXPECT_EQ_MSG(test, (u64)sg_dma_address(sg),
+				    (u64)(SG_SPLIT_DMA_BASE + pos),
+				    "[%s] split[%d] dma addr[%d]=0x%llx want 0x%llx",
+				    mode, k, j, (u64)sg_dma_address(sg),
+				    (u64)(SG_SPLIT_DMA_BASE + pos));
+		sum += sg_dma_len(sg);
+		pos += sg_dma_len(sg);
+	}
+	KUNIT_EXPECT_EQ_MSG(test, sum, want,
+			    "[%s] split[%d] DMA sum=%lu requested=%zu",
+			    mode, k, sum, want);
+}
+
+/* Verify the CPU view of one split: ->length sums to want and the page
+ * bytes match the linear oracle window.  nents is the number of CPU
+ * entries to walk (out_nents when geometry matches, sg_nents() under
+ * coalescing).
+ */
+static void sg_split_check_cpu(struct kunit *test, const char *mode, int k,
+			       struct scatterlist *out, int nents, size_t want,
+			       const u8 *oracle, unsigned long cursor)
+{
+	struct scatterlist *sg;
+	unsigned long sum = 0;
+	size_t copied = 0;
+	u8 *got;
+	int j;
+
+	for_each_sg(out, sg, nents, j)
+		sum += sg->length;
+	KUNIT_EXPECT_EQ_MSG(test, sum, want,
+			    "[%s] split[%d] CPU sum=%lu requested=%zu",
+			    mode, k, sum, want);
+
+	got = kunit_kzalloc(test, want + 1, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, got);
+	for_each_sg(out, sg, nents, j) {
+		size_t take = min_t(size_t, sg->length, want - copied);
+
+		memcpy(got + copied, sg_virt(sg), take);
+		copied += take;
+		if (copied >= want)
+			break;
+	}
+	KUNIT_EXPECT_EQ_MSG(test,
+			    memcmp(got, oracle + cursor,
+				   min_t(size_t, copied, want)),
+			    0, "[%s] split[%d] CPU bytes mismatch", mode, k);
+}
+
+/* Allocate + pattern-fill backing, build the input SG, and build the
+ * linear concatenation oracle.  Returns the input SG; *oracle_out and
+ * *backing_out get the two kunit-managed buffers.
+ */
+static struct scatterlist *
+sg_split_build_input(struct kunit *test,
+		     const struct sg_split_in_spec *in_spec, int n_in,
+		     u8 **oracle_out)
+{
+	u8 *backing, *oracle;
+	struct scatterlist *in_sgl;
+	size_t total = 0, op = 0;
+	int i;
+
+	backing = kunit_kzalloc(test,
+				SG_SPLIT_MAX_IN_ENTRIES * SG_SPLIT_PAGE_BYTES,
+				GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, backing);
+	for (int p = 0; p < SG_SPLIT_MAX_IN_ENTRIES; p++)
+		for (unsigned int o = 0; o < SG_SPLIT_PAGE_BYTES; o++)
+			backing[p * SG_SPLIT_PAGE_BYTES + o] =
+				sg_split_pattern(p, o);
+
+	in_sgl = kunit_kzalloc(test, sizeof(*in_sgl) * n_in, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, in_sgl);
+	sg_init_table(in_sgl, n_in);
+	for (i = 0; i < n_in; i++) {
+		u8 *buf = backing + in_spec[i].page * SG_SPLIT_PAGE_BYTES
+			+ in_spec[i].off;
+
+		sg_set_buf(&in_sgl[i], buf, in_spec[i].len);
+		total += in_spec[i].len;
+	}
+
+	oracle = kunit_kzalloc(test, total ? total : 1, GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, oracle);
+	for (i = 0; i < n_in; i++) {
+		memcpy(oracle + op,
+		       backing + in_spec[i].page * SG_SPLIT_PAGE_BYTES
+			+ in_spec[i].off,
+		       in_spec[i].len);
+		op += in_spec[i].len;
+	}
+
+	*oracle_out = oracle;
+	return in_sgl;
+}
+
+/* Run one case in one mode (unmapped or identity-mapped). */
+static void sg_split_run_mode(struct kunit *test, const char *mode,
+			      const struct sg_split_in_spec *in_spec, int n_in,
+			      off_t skip, const size_t *sizes, int nb_splits,
+			      bool dma_mapped)
+{
+	struct scatterlist *in_sgl;
+	struct scatterlist *out[SG_SPLIT_MAX_SPLITS] = { NULL };
+	int out_nents[SG_SPLIT_MAX_SPLITS] = { 0 };
+	unsigned long cursor = (unsigned long)skip;
+	u8 *oracle;
+	int rc, k;
+
+	KUNIT_ASSERT_LE(test, n_in, SG_SPLIT_MAX_IN_ENTRIES);
+	KUNIT_ASSERT_LE(test, nb_splits, SG_SPLIT_MAX_SPLITS);
+
+	in_sgl = sg_split_build_input(test, in_spec, n_in, &oracle);
+
+	if (dma_mapped) {
+		/* Identity mapping: dma_len == length, contiguous IOVA. */
+		dma_addr_t a = SG_SPLIT_DMA_BASE;
+		int i;
+
+		for (i = 0; i < n_in; i++) {
+			sg_dma_address(&in_sgl[i]) = a;
+			sg_dma_len(&in_sgl[i]) = in_spec[i].len;
+			a += in_spec[i].len;
+		}
+	}
+
+	rc = sg_split(in_sgl, dma_mapped ? n_in : 0, skip, nb_splits, sizes,
+		      out, out_nents, GFP_KERNEL);
+	KUNIT_ASSERT_EQ_MSG(test, rc, 0, "[%s] sg_split returned %d", mode, rc);
+
+	for (k = 0; k < nb_splits; k++) {
+		KUNIT_ASSERT_NOT_NULL_MSG(test, out[k], "[%s] split[%d] NULL",
+					  mode, k);
+		/* Exactly one end marker, on the final entry: each entry is
+		 * end-marked iff it is the last.  out_nents == 0 (the
+		 * degenerate zero-size split, out[k] == ZERO_SIZE_PTR) walks
+		 * zero times, so we never index out[k][-1].
+		 */
+		for (int j = 0; j < out_nents[k]; j++)
+			KUNIT_EXPECT_EQ_MSG(test, !!sg_is_last(&out[k][j]),
+					    j == out_nents[k] - 1,
+					    "[%s] split[%d] entry[%d] end-mark wrong",
+					    mode, k, j);
+		/* Identity mapping keeps CPU and DMA geometry equal, so
+		 * out_nents[k] is both counts; it is also 0 for a degenerate
+		 * zero-size split, keeping the walk safe.
+		 */
+		sg_split_check_cpu(test, mode, k, out[k], out_nents[k],
+				   sizes[k], oracle, cursor);
+		if (dma_mapped)
+			sg_split_check_dma(test, mode, k, out[k], out_nents[k],
+					   sizes[k], cursor);
+		cursor += sizes[k];
+	}
+
+	for (k = 0; k < nb_splits; k++)
+		kfree(out[k]);
+}
+
+/* Run a case both unmapped and identity-mapped. */
+static void sg_split_run(struct kunit *test,
+			 const struct sg_split_in_spec *in_spec, int n_in,
+			 off_t skip, const size_t *sizes, int nb_splits)
+{
+	sg_split_run_mode(test, "unmapped", in_spec, n_in, skip, sizes,
+			  nb_splits, false);
+	sg_split_run_mode(test, "mapped", in_spec, n_in, skip, sizes,
+			  nb_splits, true);
+}
+
+/* ========================================================================
+ * Group A — full or edge-aligned coverage.
+ */
+static void sg_split_t1_single_entry_full(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 100 } };
+	const size_t sizes[] = { 100 };
+
+	sg_split_run(test, in, 1, 0, sizes, 1);
+}
+
+static void sg_split_t2_two_entries_full_coverage(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 60 }, { 1, 0, 40 } };
+	const size_t sizes[] = { 100 };
+
+	sg_split_run(test, in, 2, 0, sizes, 1);
+}
+
+static void sg_split_t3_two_entries_two_splits_full(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 60 }, { 1, 0, 40 } };
+	const size_t sizes[] = { 50, 50 };
+
+	sg_split_run(test, in, 2, 0, sizes, 2);
+}
+
+static void sg_split_t4_edge_aligned_partial(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 60 }, { 1, 0, 40 } };
+	const size_t sizes[] = { 60 };
+
+	sg_split_run(test, in, 2, 0, sizes, 1);
+}
+
+/* ========================================================================
+ * Group B — partial coverage with mid-entry boundary.
+ */
+static void sg_split_t5_minimal_repro(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 2 }, { 1, 0, 1 } };
+	const size_t sizes[] = { 1 };
+
+	sg_split_run(test, in, 2, 0, sizes, 1);
+}
+
+static void sg_split_t6_dthev2_aead_shape(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 64 }, { 1, 0, 100 }, { 2, 0, 16 } };
+	const size_t sizes[] = { 100 };
+
+	sg_split_run(test, in, 3, 64, sizes, 1);
+}
+
+static void sg_split_t7_skip_plus_partial_midentry(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 100 }, { 1, 0, 50 } };
+	const size_t sizes[] = { 30 };
+
+	sg_split_run(test, in, 2, 20, sizes, 1);
+}
+
+/* ========================================================================
+ * Group C — trailing zero-size split.
+ */
+static void sg_split_t8_sizes_trailing_zero(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 10 }, { 1, 0, 5 } };
+	const size_t sizes[] = { 10, 0 };
+
+	sg_split_run(test, in, 2, 0, sizes, 2);
+}
+
+/* ========================================================================
+ * Group D — boundary-position matrix (3 x 10-byte entries).
+ */
+static void sg_split_t9_mid_first_two_trailing(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 } };
+	const size_t sizes[] = { 5 };
+
+	sg_split_run(test, in, 3, 0, sizes, 1);
+}
+
+static void sg_split_t10_edge_first_two_trailing(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 } };
+	const size_t sizes[] = { 10 };
+
+	sg_split_run(test, in, 3, 0, sizes, 1);
+}
+
+static void sg_split_t11_mid_middle_one_trailing(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 } };
+	const size_t sizes[] = { 15 };
+
+	sg_split_run(test, in, 3, 0, sizes, 1);
+}
+
+static void sg_split_t12_mid_last_no_trailing(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 } };
+	const size_t sizes[] = { 25 };
+
+	sg_split_run(test, in, 3, 0, sizes, 1);
+}
+
+/* ========================================================================
+ * Group E — skip-position variations.
+ */
+static void sg_split_t13_skip_at_entry_boundary(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 } };
+	const size_t sizes[] = { 5 };
+
+	sg_split_run(test, in, 3, 10, sizes, 1);
+}
+
+static void sg_split_t14_skip_fastforward_partial(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 } };
+	const size_t sizes[] = { 5 };
+
+	sg_split_run(test, in, 3, 12, sizes, 1);
+}
+
+/* ========================================================================
+ * Group F — multi-split, mid-entry internal transition.
+ */
+static void sg_split_t15_two_splits_last_mid_trailing(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 10 }, { 1, 0, 10 }, { 2, 0, 10 } };
+	const size_t sizes[] = { 5, 10 };
+
+	sg_split_run(test, in, 3, 0, sizes, 2);
+}
+
+static void sg_split_t16_two_splits_full_coverage_mid_first(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = { { 0, 0, 10 }, { 1, 0, 10 } };
+	const size_t sizes[] = { 3, 17 };
+
+	sg_split_run(test, in, 2, 0, sizes, 2);
+}
+
+/* ========================================================================
+ * Group G — divergent CPU/DMA geometry (coalescing).  NEED arches only:
+ * needs a separate dma_length field so dma_len != length is representable.
+ */
+static void sg_split_run_divergent(struct kunit *test,
+				   const struct sg_split_in_spec *in_spec,
+				   int n_in, off_t skip,
+				   const size_t *sizes, int nb_splits,
+				   const unsigned int *dma_seg, int n_dma)
+{
+#ifndef CONFIG_NEED_SG_DMA_LENGTH
+	kunit_skip(test,
+		   "divergent CPU/DMA geometry needs a separate dma_length field (NEED_SG_DMA_LENGTH)");
+#else
+	struct scatterlist *in_sgl;
+	struct scatterlist *out[SG_SPLIT_MAX_SPLITS] = { NULL };
+	int out_nents[SG_SPLIT_MAX_SPLITS] = { 0 };
+	unsigned long cursor = (unsigned long)skip;
+	dma_addr_t a = SG_SPLIT_DMA_BASE;
+	u8 *oracle;
+	int rc, k, i;
+
+	KUNIT_ASSERT_LE(test, n_in, SG_SPLIT_MAX_IN_ENTRIES);
+	KUNIT_ASSERT_LE(test, nb_splits, SG_SPLIT_MAX_SPLITS);
+
+	in_sgl = sg_split_build_input(test, in_spec, n_in, &oracle);
+
+	/* Coalesced DMA view on the first n_dma entries, contiguous IOVA. */
+	for (i = 0; i < n_dma; i++) {
+		sg_dma_address(&in_sgl[i]) = a;
+		sg_dma_len(&in_sgl[i]) = dma_seg[i];
+		a += dma_seg[i];
+	}
+
+	rc = sg_split(in_sgl, n_dma, skip, nb_splits, sizes,
+		      out, out_nents, GFP_KERNEL);
+	KUNIT_ASSERT_EQ_MSG(test, rc, 0, "divergent sg_split returned %d", rc);
+
+	for (k = 0; k < nb_splits; k++) {
+		KUNIT_ASSERT_NOT_NULL_MSG(test, out[k], "split[%d] NULL", k);
+		/* CPU view describes the scattered pages (pass 1): its entry
+		 * count is the end-marker count sg_nents(), not the (smaller)
+		 * DMA count out_nents[k].
+		 */
+		sg_split_check_cpu(test, "divergent", k, out[k],
+				   sg_nents(out[k]), sizes[k], oracle, cursor);
+		/* DMA view describes the coalesced contiguous IOVA (pass 2). */
+		sg_split_check_dma(test, "divergent", k, out[k], out_nents[k],
+				   sizes[k], cursor);
+		cursor += sizes[k];
+	}
+
+	for (k = 0; k < nb_splits; k++)
+		kfree(out[k]);
+#endif
+}
+
+static void sg_split_d1_coalesce_3to1(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 4096 }, { 1, 0, 4096 }, { 2, 0, 4096 } };
+	const size_t sizes[] = { 5000, 7288 };
+	const unsigned int dma[] = { 12288 };
+
+	sg_split_run_divergent(test, in, 3, 0, sizes, 2, dma, 1);
+}
+
+static void sg_split_d2_coalesce_3to2(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 4096 }, { 1, 0, 4096 }, { 2, 0, 4096 } };
+	const size_t sizes[] = { 6000, 6288 };
+	const unsigned int dma[] = { 8192, 4096 };
+
+	sg_split_run_divergent(test, in, 3, 0, sizes, 2, dma, 2);
+}
+
+static void sg_split_d3_coalesce_3to1_skip(struct kunit *test)
+{
+	const struct sg_split_in_spec in[] = {
+		{ 0, 0, 4096 }, { 1, 0, 4096 }, { 2, 0, 4096 } };
+	const size_t sizes[] = { 5000 };
+	const unsigned int dma[] = { 12288 };
+
+	sg_split_run_divergent(test, in, 3, 1000, sizes, 1, dma, 1);
+}
+
+static struct kunit_case sg_split_test_cases[] = {
+	KUNIT_CASE(sg_split_t1_single_entry_full),
+	KUNIT_CASE(sg_split_t2_two_entries_full_coverage),
+	KUNIT_CASE(sg_split_t3_two_entries_two_splits_full),
+	KUNIT_CASE(sg_split_t4_edge_aligned_partial),
+	KUNIT_CASE(sg_split_t5_minimal_repro),
+	KUNIT_CASE(sg_split_t6_dthev2_aead_shape),
+	KUNIT_CASE(sg_split_t7_skip_plus_partial_midentry),
+	KUNIT_CASE(sg_split_t8_sizes_trailing_zero),
+	KUNIT_CASE(sg_split_t9_mid_first_two_trailing),
+	KUNIT_CASE(sg_split_t10_edge_first_two_trailing),
+	KUNIT_CASE(sg_split_t11_mid_middle_one_trailing),
+	KUNIT_CASE(sg_split_t12_mid_last_no_trailing),
+	KUNIT_CASE(sg_split_t13_skip_at_entry_boundary),
+	KUNIT_CASE(sg_split_t14_skip_fastforward_partial),
+	KUNIT_CASE(sg_split_t15_two_splits_last_mid_trailing),
+	KUNIT_CASE(sg_split_t16_two_splits_full_coverage_mid_first),
+	KUNIT_CASE(sg_split_d1_coalesce_3to1),
+	KUNIT_CASE(sg_split_d2_coalesce_3to2),
+	KUNIT_CASE(sg_split_d3_coalesce_3to1_skip),
+	{}
+};
+
+static struct kunit_suite sg_split_test_suite = {
+	.name = "sg_split",
+	.test_cases = sg_split_test_cases,
+};
+
+kunit_test_suite(sg_split_test_suite);
+MODULE_DESCRIPTION("KUnit tests for lib/sg_split.c");
+MODULE_LICENSE("GPL");
-- 
2.47.3

[PATCH v2 4/5] lib: scatterlist: guard sg_split_phys()/sg_split_mapped() against zero-nents splits

[Charles Pellegrini]

If a caller passes a split_sizes[] array whose trailing entry is 0 and
the preceding splits exactly consume the input scatterlist,
sg_calculate_split() returns success with the trailing split's nents
left at 0. sg_split() then allocates that split's output with

    kmalloc_objs(struct scatterlist, 0, gfp_mask)

which returns ZERO_SIZE_PTR. Both copiers finish each split with a fixup of
the last written output entry -- sg_split_phys() does

    out_sg[-1].length = split->length_last_sg;
    sg_mark_end(out_sg - 1);

and sg_split_mapped() does

    sg_dma_len(--out_sg) = split->length_last_sg;

For a zero-nents split out_sg has not advanced, so that fixup dereferences
ZERO_SIZE_PTR: an out-of-bounds write. KASAN reports it as a slab
out-of-bounds write; a userspace ASAN harness reproduces it as a heap
overflow.

Skip empty splits at the top of the per-split loop in both copiers:

    if (!split->nents)
        continue;

No in-tree caller passes such a split_sizes[] array, so this is latent; it
was surfaced by an automated review of the v1 posting. The fix tolerates
and skips the degenerate split rather than rejecting it with -EINVAL in
sg_calculate_split(); if a stricter contract is preferred, the rejecting
variant is a trivial respin.

Fixes: f8bcbe62acd0 ("lib: scatterlist: add sg splitting function")
Link: https://lore.kernel.org/all/20260601175549.b4a10c07dd9e3b867f66da0c@linux-foundation.org/
Assisted-by: Claude:claude-opus-4-8 hegel-c
Signed-off-by: Charles Pellegrini <c4ffein.work@gmail.com>
---
 lib/sg_split.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/sg_split.c b/lib/sg_split.c
index ab574dd26b03..e90a1bfbdb56 100644
--- a/lib/sg_split.c
+++ b/lib/sg_split.c
@@ -81,6 +81,8 @@ static void sg_split_phys(struct sg_splitter *splitters, const int nb_splits)
 	struct sg_splitter *split;
 
 	for (i = 0, split = splitters; i < nb_splits; i++, split++) {
+		if (!split->nents)
+			continue;
 		in_sg = split->in_sg0;
 		out_sg = split->out_sg;
 		for (j = 0; j < split->nents; j++, out_sg++) {
@@ -108,6 +110,8 @@ static void sg_split_mapped(struct sg_splitter *splitters, const int nb_splits)
 	struct sg_splitter *split;
 
 	for (i = 0, split = splitters; i < nb_splits; i++, split++) {
+		if (!split->nents)
+			continue;
 		in_sg = split->in_sg0;
 		out_sg = split->out_sg;
 		for (j = 0; j < split->nents; j++, out_sg++) {
-- 
2.47.3

[PATCH v2 5/5] lib: scatterlist: add zero-nents regression test for sg_split()

[Charles Pellegrini]

Add sg_split_t8b_trailing_zero_no_input_left: a split_sizes[] array whose
first split consumes the entire input, so the trailing zero-size split
receives no input entry (nents == 0). Without the guard in the previous
patch, the last-entry fixup in sg_split_phys()/sg_split_mapped() writes
through a ZERO_SIZE_PTR; with it, the empty split is skipped.

Run under KASAN to catch a regression as a clean out-of-bounds report,
e.g.:

    tools/testing/kunit/kunit.py run sg_split \
        --arch=x86_64 --kconfig_add=CONFIG_KASAN=y

Without KASAN the write lands at a wild address and faults, so the case
still fails -- KASAN just turns it into a precise report rather than an
oops.

Assisted-by: Claude:claude-opus-4-8 hegel-c
Signed-off-by: Charles Pellegrini <c4ffein.work@gmail.com>
---
 lib/tests/sg_split_kunit.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lib/tests/sg_split_kunit.c b/lib/tests/sg_split_kunit.c
index 82eef313944d..6d4ba0f2d5bd 100644
--- a/lib/tests/sg_split_kunit.c
+++ b/lib/tests/sg_split_kunit.c
@@ -311,6 +311,20 @@ static void sg_split_t8_sizes_trailing_zero(struct kunit *test)
 	sg_split_run(test, in, 2, 0, sizes, 2);
 }
 
+static void sg_split_t8b_trailing_zero_no_input_left(struct kunit *test)
+{
+	/* First split consumes the entire input; the trailing zero-size split
+	 * gets no input entry (nents == 0).  sg_split_phys()/sg_split_mapped()
+	 * must not write out_sg[-1] for that empty split (out_sg would be
+	 * ZERO_SIZE_PTR).  Distinct from T8, where a trailing input entry
+	 * leaves the zero split with nents > 0.
+	 */
+	const struct sg_split_in_spec in[] = { { 0, 0, 10 } };
+	const size_t sizes[] = { 10, 0 };
+
+	sg_split_run(test, in, 1, 0, sizes, 2);
+}
+
 /* ========================================================================
  * Group D — boundary-position matrix (3 x 10-byte entries).
  */
@@ -487,6 +501,7 @@ static struct kunit_case sg_split_test_cases[] = {
 	KUNIT_CASE(sg_split_t6_dthev2_aead_shape),
 	KUNIT_CASE(sg_split_t7_skip_plus_partial_midentry),
 	KUNIT_CASE(sg_split_t8_sizes_trailing_zero),
+	KUNIT_CASE(sg_split_t8b_trailing_zero_no_input_left),
 	KUNIT_CASE(sg_split_t9_mid_first_two_trailing),
 	KUNIT_CASE(sg_split_t10_edge_first_two_trailing),
 	KUNIT_CASE(sg_split_t11_mid_middle_one_trailing),
-- 
2.47.3