linux-kernel - Re: [PATCH] perf branch: Fix heap out-of-bounds write in branch_type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1f73657c-74b3-44a7-bee4-b7cda1a75a02@pascal-lab.net>
Date: Tue, 12 Aug 2025 15:31:54 +0800
From: Yujun Dong <yujundong@...cal-lab.net>
To: Anshuman Khandual <anshuman.khandual@....com>,
 linux-perf-users@...r.kernel.org
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
 Adrian Hunter <adrian.hunter@...el.com>,
 "Liang, Kan" <kan.liang@...ux.intel.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf branch: Fix heap out-of-bounds write in
 branch_type_count()

On 2025/8/11 15:03, Anshuman Khandual wrote:
 > On 09/08/25 3:08 PM, Yujun Dong wrote:
 > > The branch_type_count() function writes to 
st->new_counts[flags->new_type]
 > > when flags->type is PERF_BR_EXTEND_ABI. However, while the array
 > > st->new_counts is sized for PERF_BR_NEW_MAX (8) entries, the field
 > > flags->new_type is a 4-bit unsigned value and may hold values up to 15.
 > >
 > > This mismatch allows crafted perf data to trigger a heap out-of-bounds
 > > write when flags->new_type >= 8, leading to memory corruption.
 >
 > Crafted ? How could flags->new_type >= 8 when PERF_BR_NEW_MAX is 
capped at 8.
 > Is this a real scenario that happened on a system ?
 >

Thanks for your review and for raising that question. The new_type field
in struct branch_flags is declared as a 4-bit bitfield (u64 new_type:4),
meaning it can hold values from 0 to 15, even though PERF_BR_NEW_MAX is
defined as 8. So, it's entirely possible for flags->new_type to be >= 8.

In fact, I've observed such cases when running real-world perf record/top,
where perf.data produced contains invalid new_type values, likely due to
other bugs or unexpected data corruption. Additionally, a maliciously
crafted perf.data file can also force this out-of-bounds write.

 > >
 > > Add a bounds check to ensure flags->new_type is less than
 > > PERF_BR_NEW_MAX before accessing the new_counts array.
 >
 > But it might make sense to add this check just to be on the safer side.
 >

Notably, new_type is only used in two places:
1. In branch_new_type_name(), where the bounds are already validated.
2. In branch_type_count(), where the current patch now adds the
necessary check.

Admittedly, the mismatch between the bit-field width (0-15) and
PERF_BR_NEW_MAX (8) is the root cause. While adjusting the bit-field
to match PERF_BR_NEW_MAX would also resolve the mismatch, that risks
breaking existing compatibility. Therefore, adding a bounds check at
the use site is the least disruptive correction.

 > >
 > > Fixes: 0ddea8e2a0c2 ("perf branch: Extend branch type classification")
 > > Signed-off-by: Yujun Dong <yujundong@...cal-lab.net>
 > > ---
 > >  tools/perf/util/branch.c | 8 +++++---
 > >  1 file changed, 5 insertions(+), 3 deletions(-)
 > >
 > > diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c
 > > index 3712be067464..8ea6628c7735 100644
 > > --- a/tools/perf/util/branch.c
 > > +++ b/tools/perf/util/branch.c
 > > @@ -21,10 +21,12 @@ void branch_type_count(struct branch_type_stat 
*st, struct branch_flags *flags,
 > >      if (flags->type == PERF_BR_UNKNOWN || from == 0)
 > >          return;
 > >
 > > -    if (flags->type == PERF_BR_EXTEND_ABI)
 > > -        st->new_counts[flags->new_type]++;
 > > -    else
 > > +    if (flags->type == PERF_BR_EXTEND_ABI) {
 > > +        if (flags->new_type < PERF_BR_NEW_MAX)
 > > +            st->new_counts[flags->new_type]++;
 > > +    } else {
 > >          st->counts[flags->type]++;
 > > +    }
 > >
 > >      if (flags->type == PERF_BR_COND) {
 > >          if (to > from)
 >