Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Integer underflow in the `pdu_len` bounds check allows out-of-bounds 
read and kernel memory disclosure when processing 48-byte v1 blktrace events.
--

commit 1abcf72f1e975080a981a6233d954f5dd76e431f
Author: Donggeun Yoo <[email protected]>

blktrace: fix the field offsets of the synthesized v1 record

This commit corrects the field offsets when synthesizing a v1 blk_io_trace
from a v2 ring buffer entry. It assigns each v1 field from its v2 counterpart
and attempts to bound the appended PDU by the entry size.

Link: https://lore.kernel.org/all/[email protected]/

> diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
> index 8cd2520b4c99e..da14e213dbcfe 100644
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -1768,17 +1768,24 @@ 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 *t = te_blk_io_trace(iter->ent);
>       struct trace_seq *s = &iter->seq;
[ ... ]
>       struct blk_io_trace old = {
>               .magic    = BLK_IO_TRACE_MAGIC | BLK_IO_TRACE_VERSION,
>               .time     = iter->ts,
> +             .sector   = t->sector,
> +             .bytes    = t->bytes,
> +             .action   = lower_32_bits(t->action),
> +             .pid      = t->pid,
> +             .device   = t->device,
> +             .cpu      = t->cpu,
> +             .error    = t->error,
> +             .pdu_len  = min_t(size_t, t->pdu_len,
> +                               iter->ent_size - sizeof(*t)),

[Severity: High]
Does this subtraction underflow when processing a 48-byte v1 blktrace event?

If iter->ent_size is 48, and sizeof(*t) is 64 (the size of struct
blk_io_trace2), the subtraction results in -16. Because min_t is casting
this negative value to size_t, it evaluates to a massive unsigned value.

While an unbounded out-of-bounds read existed in the baseline, this patch
attempts to bound the pdu_len but introduces an integer underflow instead.
This causes the bounds check to be completely bypassed, leaving the
vulnerability unresolved.

>       };
>  
> -     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, t + 1, old.pdu_len);

[Severity: High]
Because the bounds check above can underflow, does this trace_seq_putmem()
call perform an out-of-bounds read?

If the length is unvalidated due to the size_t cast of a negative number,
this will copy arbitrary adjacent kernel memory into the user-visible
trace sequence. This can potentially lead to information disclosure or a
kernel panic if the read crosses an unmapped page boundary.

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to