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

Ricardo AlgRicSpain <[email protected]>
Newsgroups org.kernel.vger.linux-block
Message-ID <CAOeC6wqyPMoUohXg9v=BxHYWmPncR_243CC2P=bptAaPvQXp+Q@mail.gmail.com>
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,
poc.c (application/octet-stream, 13.6 KB)
/*
 * 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;
}
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.