[PATCH] blktrace: build the synthesized v1 record from the entry's own layout

[PATCH] blktrace: build the synthesized v1 record from the entry's own layout

[Donggeun Yoo]

blk_trace_synthesize_old_trace() emits a classic blk_io_trace for the
binary trace_pipe output by copying 32 bytes from the ring buffer entry's
sector onward into a struct blk_io_trace. It reads them at blk_io_trace2
offsets, and the two layouts diverge after bytes: v2 has a 32-bit pid at
28 and a 64-bit action at 32, where v1 has a 32-bit action at 28 and pid
at 32. On a v2 entry every field from action on lands one slot off, so a
consumer reads the pid as the action, the device as the cpu, and the cpu
as error and pdu_len.

The PDU comes from the wrong offset as well. The copy ends at v2 offset
48 + pdu_len while the PDU starts at 64, and the emitted pdu_len is taken
from the v2 cpu, so it reads 0 while extra bytes were appended and the
consumer resynchronizes on the wrong boundary.

The entry is not always a v2 record. __blk_add_trace() reserves
sizeof(struct blk_io_trace) when the trace was set up by BLKTRACESETUP,
and on such an entry the 32-byte copy is correct. pdu_len is still read
at v2 offset 50 though, past the end of a 48-byte entry, and drives an
unbounded copy that desynchronizes the stream. That is what syzbot hit.

Take the layout from iter->ent_size, which is the only discriminator the
entry carries -- magic and sequence are the ftrace header, not a version
stamp. Assign the v1 fields from their counterparts in that layout and
append the PDU from the end of it, bounded by the entry size.

  BUG: KASAN: slab-out-of-bounds in seq_buf_putmem+0x124/0x180
  Read of size 1352 at addr ffff8880295bdb98 by task syz.2.2551/19243
   seq_buf_putmem+0x124/0x180 lib/seq_buf.c:241
   blk_trace_synthesize_old_trace kernel/trace/blktrace.c:1780 [inline]
   blk_trace_event_print_binary+0x130/0x1b0 kernel/trace/blktrace.c:1788
   tracing_read_pipe+0x568/0xb50 kernel/trace/trace.c:5444
   vfs_read+0x213/0xa80 fs/read_write.c:572

Fixes: 4d8bc7bd4f73 ("blktrace: move ftrace blk_io_tracer to blk_io_trace2")
Reported-by: syzbot+f179b16e13624138b0f1@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=f179b16e13624138b0f1
Cc: stable@vger.kernel.org
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Assisted-by: Claude:claude-fable-5
---
QEMU x86_64, KASAN, virtio disk, base 5225b8eec4c9. Binary stream read from
trace_pipe with options/bin set, decoded at v1 offsets.

v2 entry, via /sys/block/vda/trace/enable. Unpatched, offset 28 holds the pid
and 32 the action; device, cpu, error and pdu_len are each one slot off.
Patched, every field is in place: action 0x08110001 at 28, pid at 32, device
0x0fd00000 at 36, cpu at 40, error and pdu_len 0.

v1 entry, via a BLKTRACESETUP helper -- the only path that sets
bt->version = 1. Unpatched, the 48-byte record is right and eight bytes are
then appended behind it, so the next record's magic no longer lands on a
record boundary and the stream never recovers. Patched, the record is
byte-identical and the 64-byte cadence holds across the capture.

PDU-carrying entry, via BLK_TA_REMAP from reading a partition. Unpatched, the
record says pdu_len 0 and then appends 16 bytes taken from v2 offset 48, the
error/pdu_len/pad trailer, so the reader resynchronizes 16 bytes late.
Patched, pdu_len is 16 and the bytes are the blk_io_trace_remap:
device_from 0x0fd00001, device_to 0x0fd00000, sector_from 0.

Not exercised: the splat itself. iter->ent points into a ring buffer
sub-buffer page, so KASAN sees the over-read only when it crosses the page
end -- the entry has to sit about a kilobyte short of the tail and be
followed by an event with a large time delta. 120 rounds over varying buffer
sizes did not land it. The over-read is visible without KASAN in the v1 arm
above: those eight appended bytes are out-of-bounds data.

__blk_add_trace() reserving a v1-sized entry at all is a separate defect,
fixed by Adriano Cordova's "blktrace: always record ftrace events as
blk_io_trace2", <20260903202932.156278-1-adrianox@gmail.com>. It cites a
different syzbot report, but removing the short entry closes this one as
well. Once it lands every entry takes the first arm, so the branch can come
out; the field assignment is the fix and stays either way. Until then the
second arm is what keeps this function from reading past a 48-byte entry.
 kernel/trace/blktrace.c | 48 +++++++++++++++++++++++++++++++++--------
 1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 8cd2520b4c99..455d761ff84e 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -1768,17 +1768,47 @@ static enum print_line_t blk_trace_event_print(struct trace_iterator *iter,
 
 static void blk_trace_synthesize_old_trace(struct trace_iterator *iter)
 {
+	const struct blk_io_trace2 *t2 = te_blk_io_trace(iter->ent);
+	const struct blk_io_trace *t1 = (const struct blk_io_trace *)iter->ent;
 	struct trace_seq *s = &iter->seq;
-	struct blk_io_trace2 *t = (struct blk_io_trace2 *)iter->ent;
-	const int offset = offsetof(struct blk_io_trace2, sector);
-	struct blk_io_trace old = {
-		.magic	  = BLK_IO_TRACE_MAGIC | BLK_IO_TRACE_VERSION,
-		.time     = iter->ts,
-	};
+	struct blk_io_trace old;
+	const void *pdu;
+
+	if (iter->ent_size >= sizeof(*t2)) {
+		old = (struct blk_io_trace) {
+			.sector   = t2->sector,
+			.bytes    = t2->bytes,
+			.action   = lower_32_bits(t2->action),
+			.pid      = t2->pid,
+			.device   = t2->device,
+			.cpu      = t2->cpu,
+			.error    = t2->error,
+			.pdu_len  = min_t(size_t, t2->pdu_len,
+					  iter->ent_size - sizeof(*t2)),
+		};
+		pdu = t2 + 1;
+	} else if (iter->ent_size >= sizeof(*t1)) {
+		old = (struct blk_io_trace) {
+			.sector   = t1->sector,
+			.bytes    = t1->bytes,
+			.action   = t1->action,
+			.pid      = t1->pid,
+			.device   = t1->device,
+			.cpu      = t1->cpu,
+			.error    = t1->error,
+			.pdu_len  = min_t(size_t, t1->pdu_len,
+					  iter->ent_size - sizeof(*t1)),
+		};
+		pdu = t1 + 1;
+	} else {
+		return;
+	}
+
+	old.magic = BLK_IO_TRACE_MAGIC | BLK_IO_TRACE_VERSION;
+	old.time = iter->ts;
 
-	trace_seq_putmem(s, &old, offset);
-	trace_seq_putmem(s, &t->sector,
-			 sizeof(old) - offset + t->pdu_len);
+	trace_seq_putmem(s, &old, sizeof(old));
+	trace_seq_putmem(s, pdu, old.pdu_len);
 }
 
 static enum print_line_t
-- 
2.53.0

Re: [PATCH] blktrace: build the synthesized v1 record from the entry's own layout

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Size-based differentiation between v1 and v2 events causes v1 entries with a large PDU (>= 16 bytes) to be misidentified as v2 entries, scrambling output fields and truncating the PDU.
--

commit 7a0e33d5f62e85ae6d055e44b6cd6349ae56c6ba
Author: Donggeun Yoo <donggeunyoo.kernel@gmail.com>

blktrace: build the synthesized v1 record from the entry's own layout

This commit addresses a bug where synthesized v1 blktrace events read from
incorrect offsets when handling a v2 entry. It attempts to fix this by
properly differentiating between v1 and v2 records using the entry's size,
and correctly assigning fields and PDU lengths based on the detected layout.

Closes: https://syzkaller.appspot.com/bug?extid=f179b16e13624138b0f1

> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
> index 8cd2520b4c99e..455d761ff84e6 100644
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c

[ ... ]

> @@ -1768,17 +1768,47 @@ static enum print_line_t blk_trace_event_print(struct trace_iterator *iter,
>  
>  static void blk_trace_synthesize_old_trace(struct trace_iterator *iter)
>  {
> +	const struct blk_io_trace2 *t2 = te_blk_io_trace(iter->ent);
> +	const struct blk_io_trace *t1 = (const struct blk_io_trace *)iter->ent;
>  	struct trace_seq *s = &iter->seq;
> +	struct blk_io_trace old;
> +	const void *pdu;
> +
> +	if (iter->ent_size >= sizeof(*t2)) {

[Severity: Medium]
Will this size-based heuristic incorrectly classify v1 entries with a large
PDU as v2 entries?

If a user configures block tracing via BLKTRACESETUP, it uses v1 records
which have a 48-byte base size. If a BLK_TA_REMAP event is recorded, it
includes a 16-byte PDU (struct blk_io_trace_remap).

The total size of this event is exactly 64 bytes. Since
sizeof(struct blk_io_trace2) is also 64 bytes, iter->ent_size >= sizeof(*t2)
evaluates to true (64 >= 64), and the v2 branch is taken.

Does this result in the output being scrambled because fields like action,
pid, device, and cpu are read from incorrect offsets, and the PDU being
truncated because the calculated length clamps to 0?

> +		old = (struct blk_io_trace) {
> +			.sector   = t2->sector,
> +			.bytes    = t2->bytes,
> +			.action   = lower_32_bits(t2->action),
> +			.pid      = t2->pid,
> +			.device   = t2->device,
> +			.cpu      = t2->cpu,
> +			.error    = t2->error,
> +			.pdu_len  = min_t(size_t, t2->pdu_len,
> +					  iter->ent_size - sizeof(*t2)),
> +		};
> +		pdu = t2 + 1;
> +	} else if (iter->ent_size >= sizeof(*t1)) {

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913025827.457116-1-donggeunyoo.kernel@gmail.com?part=1

Re: [PATCH] blktrace: build the synthesized v1 record from the entry's own layout

[Donggeun Yoo]

On Sun, 13 Sep 2026 03:10:30 +0000, sashiko-bot@kernel.org wrote:
> - [Medium] Size-based differentiation between v1 and v2 events causes v1
> entries with a large PDU (>= 16 bytes) to be misidentified as v2 entries,
> scrambling output fields and truncating the PDU.

Correct. A v1 record carrying a 16-byte BLK_TA_REMAP PDU is 64 bytes, the
size of a v2 record, and nothing else in the entry tells them apart.

v2 drops the size test and depends on Adriano Cordova's "blktrace: always
record ftrace events as blk_io_trace2" instead:

  https://lore.kernel.org/all/20260913063155.708520-1-donggeunyoo.kernel@gmail.com/