From: Charles Pellegrini <c4ffein.work@gmail.com>
To: akpm@linux-foundation.org, egorenar-dev@posteo.net,
robert.jarzmik@free.fr, t-pratham@ti.com, david@davidgow.net,
linux-kernel@vger.kernel.org
Subject: [PATCH v2 2/5] lib: scatterlist: fix sg_split_phys() ->length clobber on !NEED_SG_DMA_LENGTH
Date: Wed, 10 Jun 2026 15:39:33 -0700 (PDT) [thread overview]
Message-ID: <178113124323.90620.15074575220138727748@gmail.com> (raw)
In-Reply-To: <178113124323.90620.6403136846887207198@gmail.com>
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
next prev parent reply other threads:[~2026-06-10 22:39 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-31 23:42 [PATCH 0/2] lib: scatterlist: fix sg_split() partial-coverage geometry + add KUnit tests Charles Pellegrini
2026-05-31 23:42 ` [PATCH 1/2] lib: scatterlist: fix sg_calculate_split() nb_splits overshoot on partial coverage Charles Pellegrini
2026-05-31 23:42 ` [PATCH 2/2] lib: scatterlist: add KUnit tests for sg_split() Charles Pellegrini
2026-06-02 0:55 ` [PATCH 0/2] lib: scatterlist: fix sg_split() partial-coverage geometry + add KUnit tests Andrew Morton
2026-06-10 22:35 ` c4ffein.work
2026-06-10 22:39 ` [PATCH v2 0/5] lib: scatterlist: fix sg_split() partial-coverage geometry, two latent corruption bugs, and " Charles Pellegrini
2026-06-10 22:39 ` [PATCH v2 1/5] lib: scatterlist: fix sg_calculate_split() nb_splits overshoot on partial coverage Charles Pellegrini
2026-06-10 22:39 ` Charles Pellegrini [this message]
2026-06-10 22:39 ` [PATCH v2 3/5] lib: scatterlist: add KUnit tests for sg_split() Charles Pellegrini
2026-06-10 22:39 ` [PATCH v2 5/5] lib: scatterlist: add zero-nents regression test " Charles Pellegrini
2026-06-10 22:39 ` [PATCH v2 4/5] lib: scatterlist: guard sg_split_phys()/sg_split_mapped() against zero-nents splits Charles Pellegrini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=178113124323.90620.15074575220138727748@gmail.com \
--to=c4ffein.work@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=david@davidgow.net \
--cc=egorenar-dev@posteo.net \
--cc=linux-kernel@vger.kernel.org \
--cc=robert.jarzmik@free.fr \
--cc=t-pratham@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome