block/t10-pi.c: blk_integrity_interval() prot_iter misalignment with extended metadata

block/t10-pi.c: blk_integrity_interval() prot_iter misalignment with extended metadata

[Ricardo AlgRicSpain]

[-- 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;
}

Re: block/t10-pi.c: blk_integrity_interval() prot_iter misalignment with extended metadata

[Christoph Hellwig]

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.