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 <[email protected]>

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/[email protected]?part=1

Reply via email to