Re: [PATCH] perf: arm_pmuv3: Zero initialize hw_id branch stack field
James Clark <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-perf-users |
|---|---|
| Message-ID | <[email protected]> |
On 07/08/2026 10:27, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > > Pre-existing issues: > - [High] Missing initialization of `new_type` and `priv` fields in `perf_clear_branch_entry_bitfields()` allows cross-sample metadata leakage and KASLR bypass. > -- > > --- Patch [1]: [PATCH] perf: arm_pmuv3: Zero initialize hw_id branch stack field --- > commit 9362aa7ec453c50e757449959540fd185a720850 > Author: James Clark <[email protected]> > > perf: arm_pmuv3: Zero initialize hw_id branch stack field > > This patch modifies the allocation of the BRBE branch_stack to use kzalloc > instead of kmalloc. This ensures that the hw_id field, and any future fields, > are safely zero-initialized upon allocation to comply with documentation. > >> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c >> --- a/drivers/perf/arm_pmuv3.c >> +++ b/drivers/perf/arm_pmuv3.c >> @@ -1361,7 +1361,7 @@ static int branch_records_alloc(struct arm_pmu *armpmu) >> struct pmu_hw_events *events_cpu; >> >> events_cpu = per_cpu_ptr(armpmu->hw_events, cpu); >> - events_cpu->branch_stack = kmalloc(size, GFP_KERNEL); >> + events_cpu->branch_stack = kzalloc(size, GFP_KERNEL); > > [Severity: High] > This is a pre-existing issue, but since we are fixing uninitialized branch > stack fields on allocation, does the per-sample reuse of these entries later > leak previous metadata? > > The perf_clear_branch_entry_bitfields() helper zeroes most bitfields, but > misses the new_type and priv fields: > > include/linux/perf_event.h:perf_clear_branch_entry_bitfields() { > br->mispred = 0; > br->predicted = 0; > ... > br->reserved = 0; > // new_type and priv are not cleared > } > > In the ARM BRBE driver, the per-CPU branch_stack buffer entries are reused > and cleared using this helper. If a hardware branch record is source-only, > entry->priv is not explicitly set: > > drivers/perf/arm_brbe.c:perf_entry_from_brbe_regset() { > perf_clear_branch_entry_bitfields(entry); > ... > if (!brbe_record_is_source_only(brbinf)) > entry->priv = brbinf_get_perf_priv(brbinf); > } > > If the previous sample in that slot was a user-space branch, entry->priv > retains PERF_BR_PRIV_USER. When filter_branch_privilege() later checks > entry->priv, could this allow kernel source addresses to be exposed to > unprivileged users and bypass KASLR? > >> if (!events_cpu->branch_stack) >> return -ENOMEM; >> } > Fixed by "[PATCH v6 3/3] perf/core: Clear the whole branch entry in perf_clear_branch_entry()" on the list.