* block/t10-pi.c: blk_integrity_interval() prot_iter misalignment with extended metadata
@ 2026-08-12 12:37 Ricardo AlgRicSpain
2026-08-14 6:38 ` Christoph Hellwig
0 siblings, 1 reply; 2+ messages in thread
From: Ricardo AlgRicSpain @ 2026-08-12 12:37 UTC (permalink / raw)
To: linux-block
[-- Attachment #1: Type: text/plain, Size: 971 bytes --]
Hi,
blk_integrity_interval() (block/t10-pi.c:270-305) advances prot_iter
inconsistently:
- fast path (t10-pi.c:282): advances metadata_size - pi_offset bytes
- split-tuple path (t10-pi.c:285-287, 296-299): advances only
pi_tuple_size bytes via blk_integrity_copy_to_tuple/copy_from_tuple
When metadata_size - pi_offset > pi_tuple_size (e.g. NVMe extended
metadata) and the tuple splits across bvec segments, prot_iter
under-advances by the trailing opaque bytes, and the offset accumulates
across subsequent intervals, breaking guard/data correspondence on
verify and generate.
Suggested fix: after copy_to_tuple/copy_from_tuple, advance the
remaining bytes:
bvec_iter_advance(iter->bip->bip_vec, &iter->prot_iter,
iter->bi->metadata_size - iter->bi->pi_offset -
iter->bi->pi_tuple_size);
...matching what blk_tuple_remap_end() already does (t10-pi.c:426-436).
Standalone C PoC reproducing the misalignment attached (poc.c).
Thanks,
[-- Attachment #2: poc.c --]
[-- Type: application/octet-stream, Size: 13888 bytes --]
/*
* PoC — blk_integrity_interval(): protection iterator misalignment
* ------------------------------------------------------------------------
* Reproduces in userspace the exact logic of t10-pi.c:
*
* blk_integrity_interval() (t10-pi.c:270)
* blk_integrity_copy_to_tuple() / blk_integrity_copy_from_tuple() (t10-pi.c:93)
* bvec_iter_advance_single() / bvec_iter_advance() (include/linux/bvec.h)
* mp_bvec_iter_bvec() (include/linux/bvec.h)
*
* and demonstrates the divergence in how `prot_iter` is advanced between the
* fast path (aligned tuple) and the split-tuple path, when the metadata
* format has trailing padding/opaque bytes (metadata_size > pi_offset +
* pi_tuple_size).
*
* Specifically, for a bio whose metadata block is split across two bvecs:
* - interval 0 is processed via the slow path -> prot_iter advances ONLY pi_tuple_size
* - interval 1 is processed via the fast path -> prot_iter advances metadata_size-pi_offset
* Interval 1 ends up misaligned: its tuple is read/written at the wrong
* offset (overlapping with the opaque bytes of interval 0), and the error
* accumulates over subsequent intervals.
*
* It also demonstrates the violation of the precondition of
* `bvec_iter_advance_single` (bytes <= bv[i->bi_idx].bv_len) on the fast
* path when metadata_size - pi_offset > pbv.bv_len, which leaves the
* iterator in an inconsistent state (OOB on the next access).
*
* Build and run:
* gcc -O0 -Wall -Wextra -o poc poc.c && ./poc
*
* Output: 0 if the bug reproduces (mismatch detected), 1 otherwise.
*/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>
/* ------------------------------------------------------------------ */
/* Minimal model of struct bio_vec / bvec_iter (include/linux/bvec.h) */
/* ------------------------------------------------------------------ */
typedef unsigned int u32;
struct bio_vec {
void *bv_page; /* in userspace: base of the buffer */
u32 bv_len;
u32 bv_offset;
};
struct bvec_iter {
u32 bi_size; /* bytes remaining */
u32 bi_idx; /* current bvec */
u32 bi_bvec_done; /* bytes consumed from the current bvec */
};
static inline u32 bv_min(u32 a, u32 b) { return a < b ? a : b; }
static inline struct bio_vec
mp_bvec_iter_bvec(const struct bio_vec *bvecs, struct bvec_iter iter)
{
const struct bio_vec *b = &bvecs[iter.bi_idx];
struct bio_vec out = {
.bv_page = b->bv_page,
.bv_len = bv_min(b->bv_len - iter.bi_bvec_done, iter.bi_size),
.bv_offset = b->bv_offset + iter.bi_bvec_done,
};
return out;
}
/* Single-page version (t10-pi.c uses bvec_iter_bvec); without paging the
* result is identical to mp_bvec_iter_bvec. */
static inline struct bio_vec
bvec_iter_bvec(const struct bio_vec *bvecs, struct bvec_iter iter)
{
return mp_bvec_iter_bvec(bvecs, iter);
}
/*
* bvec_iter_advance_single() — literal port of include/linux/bvec.h.
* Documented PRECONDITION: bytes <= bv[i->bi_idx].bv_len (must not cross
* bvec boundaries). The fast path of blk_integrity_interval violates it
* (see §4).
*/
static inline void
bvec_iter_advance_single(const struct bio_vec *bv, struct bvec_iter *iter,
unsigned int bytes)
{
unsigned int done = iter->bi_bvec_done + bytes;
if (done == bv[iter->bi_idx].bv_len) {
done = 0;
iter->bi_idx++;
}
iter->bi_bvec_done = done;
iter->bi_size -= bytes;
}
/* bvec_iter_advance() — literal port; DOES cross bvec boundaries. This is
* the one blk_tuple_remap_end() uses to consume the full metadata block. */
static inline void
bvec_iter_advance(const struct bio_vec *bv, struct bvec_iter *iter,
unsigned int bytes)
{
unsigned int idx = iter->bi_idx;
if (bytes > iter->bi_size) {
iter->bi_size = 0;
return;
}
iter->bi_size -= bytes;
bytes += iter->bi_bvec_done;
while (bytes && bytes >= bv[idx].bv_len) {
bytes -= bv[idx].bv_len;
idx++;
}
iter->bi_idx = idx;
iter->bi_bvec_done = bytes;
}
/* ------------------------------------------------------------------ */
/* Port of blk_integrity_copy_to_tuple / copy_from_tuple (t10-pi.c) */
/* ------------------------------------------------------------------ */
/* The copies advance exactly `tuple_size` bytes (sum of len across bvecs). */
static void
blk_integrity_copy_to_tuple(const struct bio_vec *bvecs, struct bvec_iter *iter,
unsigned int tuple_size)
{
while (tuple_size) {
struct bio_vec pbv = bvec_iter_bvec(bvecs, *iter);
unsigned int len = bv_min(tuple_size, pbv.bv_len);
/* memcpy(prot_buf, tuple, len) — content is not modeled */
bvec_iter_advance_single(bvecs, iter, len);
tuple_size -= len;
}
}
static void
blk_integrity_copy_from_tuple(const struct bio_vec *bvecs, struct bvec_iter *iter,
unsigned int tuple_size)
{
while (tuple_size) {
struct bio_vec pbv = bvec_iter_bvec(bvecs, *iter);
unsigned int len = bv_min(tuple_size, pbv.bv_len);
/* memcpy(tuple, prot_buf, len) */
bvec_iter_advance_single(bvecs, iter, len);
tuple_size -= len;
}
}
/* ------------------------------------------------------------------ */
/* Port of blk_integrity_interval (t10-pi.c:270) */
/* ------------------------------------------------------------------ */
/*
* Returns the offset (within the metadata buffer) at which the interval's
* tuple was accessed, or -1 if the iterator state is inconsistent.
* `verify` = true -> verification path; false -> generate path.
*/
static long
blk_integrity_interval_replica(const struct bio_vec *bvecs,
struct bvec_iter *prot_iter,
u32 pi_offset, u32 pi_tuple_size,
u32 metadata_size, bool verify,
u32 *csum_offset_at_entry)
{
/* offset of prot_iter on entry (how much has already been consumed) */
u32 consumed = bvecs[0].bv_offset; /* unused: we measure via bi_size */
(void)consumed;
/*
* blk_integrity_csum_offset(): consumes `pi_offset` bytes (front
* padding) while computing the checksum over them. We only model
* the advance here.
*/
unsigned int offset = pi_offset;
struct bvec_iter iter = *prot_iter;
while (offset > 0) {
struct bio_vec pbv = bvec_iter_bvec(bvecs, iter);
unsigned int len = bv_min(pbv.bv_len, offset);
bvec_iter_advance_single(bvecs, &iter, len);
offset -= len;
}
*csum_offset_at_entry = iter.bi_bvec_done;
/* absolute offset of the tuple = bytes consumed from the start.
* The buffer starts at the first bvec; the bvecs are contiguous in
* this PoC, so we compute the absolute position by summing them up. */
u32 consumed_abs = 0;
for (u32 i = 0; i < iter.bi_idx; i++)
consumed_abs += bvecs[i].bv_len;
consumed_abs += iter.bi_bvec_done;
consumed_abs -= pi_offset; /* subtract the padding already traversed */
struct bio_vec pbv = bvec_iter_bvec(bvecs, iter);
if (pbv.bv_len >= pi_tuple_size) {
/* FAST PATH — advances metadata_size - pi_offset */
bvec_iter_advance_single(bvecs, &iter,
metadata_size - pi_offset);
} else if (verify) {
/* SLOW PATH (verify) — advances ONLY pi_tuple_size */
blk_integrity_copy_to_tuple(bvecs, &iter, pi_tuple_size);
} else {
/* SLOW PATH (generate): blk_integrity_set writes into the
* on-stack tuple and copy_from_tuple advances pi_tuple_size. */
blk_integrity_copy_from_tuple(bvecs, &iter, pi_tuple_size);
}
*prot_iter = iter;
return (long)consumed_abs;
}
/*
* FIXED version: after the slow path, consumes the rest of the block
* (metadata_size - pi_offset - pi_tuple_size), same as blk_tuple_remap_end.
*/
static long
blk_integrity_interval_fixed(const struct bio_vec *bvecs,
struct bvec_iter *prot_iter,
u32 pi_offset, u32 pi_tuple_size,
u32 metadata_size, bool verify,
u32 *csum_offset_at_entry)
{
unsigned int offset = pi_offset;
struct bvec_iter iter = *prot_iter;
while (offset > 0) {
struct bio_vec pbv = bvec_iter_bvec(bvecs, iter);
unsigned int len = bv_min(pbv.bv_len, offset);
bvec_iter_advance_single(bvecs, &iter, len);
offset -= len;
}
*csum_offset_at_entry = iter.bi_bvec_done;
u32 consumed_abs = 0;
for (u32 i = 0; i < iter.bi_idx; i++)
consumed_abs += bvecs[i].bv_len;
consumed_abs += iter.bi_bvec_done;
consumed_abs -= pi_offset;
struct bio_vec pbv = bvec_iter_bvec(bvecs, iter);
if (pbv.bv_len >= pi_tuple_size) {
bvec_iter_advance_single(bvecs, &iter, metadata_size - pi_offset);
} else {
/* slow path: copy + consume the rest of the block */
if (verify)
blk_integrity_copy_to_tuple(bvecs, &iter, pi_tuple_size);
else
blk_integrity_copy_from_tuple(bvecs, &iter, pi_tuple_size);
/* FIX: advance the rest of the metadata block */
u32 rest = metadata_size - pi_offset - pi_tuple_size;
if (rest > 0)
bvec_iter_advance(bvecs, &iter, rest);
}
*prot_iter = iter;
return (long)consumed_abs;
}
/* ------------------------------------------------------------------ */
/* Test helpers */
/* ------------------------------------------------------------------ */
static int failures;
static bool bug_reproduced;
#define CHECK(cond, fmt, ...) \
do { \
if (cond) { \
printf(" [OK ] " fmt "\n", ##__VA_ARGS__); \
} else { \
printf(" [BUG] " fmt "\n", ##__VA_ARGS__); \
failures++; \
} \
} while (0)
/*
* Scenario: a metadata buffer of `n_intervals * metadata_size` bytes split
* into two contiguous bvecs. The split point is at `split` bytes, so that
* the first metadata block (0..metadata_size) ends up divided and its
* tuple (if split < pi_tuple_size) is processed via the slow path.
*/
static void
run_scenario(const char *name, u32 metadata_size, u32 pi_offset,
u32 pi_tuple_size, u32 n_intervals, u32 split,
bool expect_misalign)
{
char *buf = calloc(1, (size_t)n_intervals * metadata_size);
struct bio_vec bvecs[2] = {
{ .bv_page = buf, .bv_len = split, .bv_offset = 0 },
{ .bv_page = buf, .bv_len = (u32)n_intervals * metadata_size - split,
.bv_offset = split },
};
struct bvec_iter buggy = {
.bi_size = (u32)n_intervals * metadata_size,
.bi_idx = 0, .bi_bvec_done = 0,
};
struct bvec_iter fixed = buggy;
u32 off_buggy[16], off_fixed[16];
u32 csum_e;
printf("\n=== %s ===\n", name);
printf(" metadata_size=%u pi_offset=%u pi_tuple_size=%u "
"intervals=%u split=%u expect_misalign=%s\n",
metadata_size, pi_offset, pi_tuple_size, n_intervals, split,
expect_misalign ? "yes" : "no");
bool repro = false;
for (u32 iv = 0; iv < n_intervals; iv++) {
off_buggy[iv] = (u32)blk_integrity_interval_replica(
bvecs, &buggy, pi_offset, pi_tuple_size,
metadata_size, true, &csum_e);
off_fixed[iv] = (u32)blk_integrity_interval_fixed(
bvecs, &fixed, pi_offset, pi_tuple_size,
metadata_size, true, &csum_e);
}
for (u32 iv = 0; iv < n_intervals; iv++) {
u32 expected = iv * metadata_size;
printf(" interval %u: buggy tuple@%u fixed tuple@%u (correct=%u)%s\n",
iv, off_buggy[iv], off_fixed[iv], expected,
off_buggy[iv] != expected ? " <-- MISALIGN" : "");
if (off_buggy[iv] != expected)
repro = true;
}
if (expect_misalign) {
if (repro)
bug_reproduced = true;
CHECK(repro, "%s: bug reproduced (tuple at wrong offset)", name);
} else {
CHECK(!repro, "%s: no opaque, both paths agree (no bug)", name);
}
free(buf);
}
/* Fast path: precondition violation of bvec_iter_advance_single */
static void
run_fastpath_precond_violation(void)
{
char buf[32];
struct bio_vec bvecs[1] = { { .bv_page = buf, .bv_len = 12, .bv_offset = 0 } };
struct bvec_iter iter = { .bi_size = 12, .bi_idx = 0, .bi_bvec_done = 0 };
u32 pi_offset = 0, pi_tuple_size = 8, metadata_size = 16; /* opaque=8 */
u32 csum_e;
printf("\n=== Fast path: bvec_iter_advance_single precondition ===\n");
printf(" bvec[0].bv_len=12 pi_tuple_size=8 metadata_size=16\n");
printf(" pbv.bv_len(12) >= pi_tuple_size(8) -> fast path\n");
printf(" but advance=metadata_size-pi_offset=16 > pbv.bv_len=12\n");
long off = blk_integrity_interval_replica(
bvecs, &iter, pi_offset, pi_tuple_size, metadata_size, true,
&csum_e);
printf(" result: bi_idx=%u bi_bvec_done=%u (bvec_len=12)\n",
iter.bi_idx, iter.bi_bvec_done);
CHECK(iter.bi_bvec_done > bvecs[0].bv_len,
"bvec_iter_advance_single() left bi_bvec_done=%u > bv_len=%u "
"(precondition violation bytes <= bv[i].bv_len -> OOB on the "
"next access)",
iter.bi_bvec_done, bvecs[0].bv_len);
(void)off;
}
/* ------------------------------------------------------------------ */
/* main */
/* ------------------------------------------------------------------ */
int main(void)
{
printf("PoC: blk_integrity_interval() — prot_iter misalignment\n");
printf(" replica of t10-pi.c over bvec_iter (upstream master)\n");
/* NVMe-like case: 16B metadata, 8B tuple at the start, 8B opaque.
* WITH trailing opaque -> the slow path advances less -> misalignment. */
run_scenario("nvme-extended 16B/8B+8B", 16, 0, 8, 4, 4, true);
/* CRC64-like: 16B metadata, 16B tuple, no opaque -> should not fail. */
run_scenario("crc64 16B/16B (no opaque)", 16, 0, 16, 4, 8, false);
/* with pi_offset: 24B metadata, 8B tuple at offset 8 (8+8 opaque). */
run_scenario("pi_offset 24B/8B@8", 24, 8, 8, 3, 10, true);
run_fastpath_precond_violation();
/* Control case: plain T10 8B (no opaque) -> no bug. */
printf("\n=== Control: plain T10 8B (no opaque) ===\n");
run_scenario("flat-8B", 8, 0, 8, 4, 4, false);
printf("\n%s\n", failures ? "==> SOME CHECK FAILED (unexpected behavior)"
: "==> all expected checks passed");
printf("%s\n", bug_reproduced
? "==> BUG CONFIRMED: prot_iter misaligns with extended metadata (trailing opaque)"
: "==> bug not reproduced");
return bug_reproduced ? 0 : 1;
}
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: block/t10-pi.c: blk_integrity_interval() prot_iter misalignment with extended metadata
2026-08-12 12:37 block/t10-pi.c: blk_integrity_interval() prot_iter misalignment with extended metadata Ricardo AlgRicSpain
@ 2026-08-14 6:38 ` Christoph Hellwig
0 siblings, 0 replies; 2+ messages in thread
From: Christoph Hellwig @ 2026-08-14 6:38 UTC (permalink / raw)
To: Ricardo AlgRicSpain, Martin K. Petersen; +Cc: linux-block
On Wed, Aug 12, 2026 at 02:37:28PM +0200, Ricardo AlgRicSpain wrote:
> Hi,
>
> blk_integrity_interval() (block/t10-pi.c:270-305) advances prot_iter
> inconsistently:
>
> - fast path (t10-pi.c:282): advances metadata_size - pi_offset bytes
> - split-tuple path (t10-pi.c:285-287, 296-299): advances only
> pi_tuple_size bytes via blk_integrity_copy_to_tuple/copy_from_tuple
>
> When metadata_size - pi_offset > pi_tuple_size (e.g. NVMe extended
> metadata) and the tuple splits across bvec segments, prot_iter
> under-advances by the trailing opaque bytes, and the offset accumulates
> across subsequent intervals, breaking guard/data correspondence on
> verify and generate.
>
> Suggested fix: after copy_to_tuple/copy_from_tuple, advance the
> remaining bytes:
>
> bvec_iter_advance(iter->bip->bip_vec, &iter->prot_iter,
> iter->bi->metadata_size - iter->bi->pi_offset -
> iter->bi->pi_tuple_size);
>
> ...matching what blk_tuple_remap_end() already does (t10-pi.c:426-436).
>
> Standalone C PoC reproducing the misalignment attached (poc.c).
Please send a patch with your proposed fix. Wiring up your reproducer
in blktests would also be very helpful.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-14 6:38 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12 12:37 block/t10-pi.c: blk_integrity_interval() prot_iter misalignment with extended metadata Ricardo AlgRicSpain
2026-08-14 6:38 ` Christoph Hellwig
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.