[PATCH v6 0/3] perf/core: sched_task() dispatch and branch entry fixes

[PATCH v6 0/3] perf/core: sched_task() dispatch and branch entry fixes

[Puranjay Mohan]

These three fixes were found while adding BRBE support for
bpf_get_branch_snapshot() on arm64 and have been carried in that series
since v1 [1]. They do not depend on it, so they go on their own from
here; the version number continues from that series to avoid two
numbering schemes for the same patches.

Patch 1 stops __perf_pmu_sched_task() passing a NULL pmu_ctx to
pmu->sched_task(). armv8pmu_sched_task() is the only implementation that
dereferences the argument, so the oops needs BRBE.

Patch 2 makes perf_pmu_sched_task() visit PMUs whose events are all
CPU-wide. They are skipped today whenever the scheduled task has a perf
event of its own, so branch records leak across task boundaries with
perf record -b -a. intel_pmu_lbr_add() calls perf_sched_cb_inc()
unconditionally, so x86 LBR is affected the same way.

Dropping the early return alone leaves both dispatch paths running for
one case, so perf_ctx_sched_task_cb() gains a matching gate. The two
could instead be collapsed into perf_pmu_sched_task() alone, since
__perf_pmu_sched_task() already passes the same epc, but that would move
the callback out of the perf_ctx_disable() window for every PMU rather
than just that one case, which seemed like too much for a fix tagged for
stable.

Patch 3 clears struct perf_branch_entry with a single struct assignment.
perf_clear_branch_entry_bitfields() had drifted from the struct: new_type
and priv were never cleared, and arm_pmuv3.c allocates the per-CPU branch
stack with kmalloc().

Tested on a 128 CPU arm64 machine with BRBE. A WARN_ON_ONCE() at the gate
patch 2 adds to perf_ctx_sched_task_cb() fires within seconds of running
perf record -b -a alongside a task-bound event pinned to a different CPU.

Changes in v6:
- Split the sched_task() fix into patches 1 and 2; the NULL dereference
  and the missed dispatch are separate bugs with different reachability.
- Gate perf_ctx_sched_task_cb() on cpc->task_epc. v5 removed the early
  return in perf_pmu_sched_task() without it, so both paths ran for a
  task whose event for that PMU is pinned to another CPU. Caught by the
  WARN_ON_ONCE() described above.
- Tag patches 1 and 2 for stable.
- Send separately from the BRBE series, rebased onto tip perf/core.

[1] https://lore.kernel.org/all/20260616155716.2631508-1-puranjay@kernel.org/

Based on tip perf/core (f4dfab174244).

Puranjay Mohan (3):
  perf/core: Fix NULL pmu_ctx passed to pmu->sched_task()
  perf/core: Run sched_task() for PMUs with only CPU-wide events
  perf/core: Clear the whole branch entry in perf_clear_branch_entry()

 arch/x86/events/amd/brs.c   |  2 +-
 arch/x86/events/amd/lbr.c   |  2 +-
 arch/x86/events/intel/lbr.c |  6 +++---
 drivers/perf/arm_brbe.c     |  2 +-
 include/linux/perf_event.h  | 16 ++--------------
 kernel/events/core.c        | 15 +++++++++++----
 6 files changed, 19 insertions(+), 24 deletions(-)

-- 
2.53.0-Meta

[PATCH v6 1/3] perf/core: Fix NULL pmu_ctx passed to pmu->sched_task()

[Puranjay Mohan]

perf_pmu_sched_task() returns early when cpuctx->task_ctx is set, and
cpc->task_epc is only non-NULL while a task context is scheduled in on
this CPU. __perf_pmu_sched_task() therefore always passes NULL:

  Unable to handle kernel NULL pointer dereference at virtual address 00
  pc : armv8pmu_sched_task+0x14/0x50
  Call trace:
   armv8pmu_sched_task+0x14/0x50 (P)
   perf_pmu_sched_task+0xac/0x108
   __perf_event_task_sched_out+0x6c/0xe0

Pass &cpc->epc instead. __perf_init_event_pmu_context() sets its ->pmu
when the PMU is registered; ->ctx stays NULL until a CPU-wide event
attaches. That is enough here because armv8pmu_sched_task() is the only
in-tree implementation that dereferences the argument at all, and it
only reads ->pmu. The oops therefore needs BRBE, which arrived in v6.17.

Fixes: bd2756811766 ("perf: Rewrite core context handling")
Cc: stable@vger.kernel.org
Acked-by: Usama Arif <usama.arif@linux.dev>
Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 kernel/events/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index b282de3e7d7ca..9815894b67e77 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3907,7 +3907,8 @@ static void __perf_pmu_sched_task(struct perf_cpu_pmu_context *cpc,
 	perf_ctx_lock(cpuctx, cpuctx->task_ctx);
 	perf_pmu_disable(pmu);
 
-	pmu->sched_task(cpc->task_epc, task, sched_in);
+	pmu->sched_task(cpc->task_epc ? cpc->task_epc : &cpc->epc,
+			task, sched_in);
 
 	perf_pmu_enable(pmu);
 	perf_ctx_unlock(cpuctx, cpuctx->task_ctx);
-- 
2.53.0-Meta

[PATCH v6 2/3] perf/core: Run sched_task() for PMUs with only CPU-wide events

[Puranjay Mohan]

perf_pmu_sched_task() returns early when cpuctx->task_ctx is set and
leaves the work to perf_ctx_sched_task_cb(). That one only walks
ctx->pmu_ctx_list, so a PMU whose events are all CPU-wide is never
visited and its sched_task() callback does not run. With

  perf record -b -e cycles -a -- ls

armv8pmu_sched_task() is skipped on every switch to a task that has a
perf event of its own, and BRBE records leak across the task boundary.
intel_pmu_lbr_add() calls perf_sched_cb_inc() unconditionally as well,
so LBR records leak the same way on x86.

Drop the early return and instead skip the individual CPCs that
perf_ctx_sched_task_cb() already handles.

That requires the two to agree on which CPC belongs to which path, and
they do not. perf_ctx_sched_task_cb() gates on cpc->sched_cb_usage,
which perf_sched_cb_inc() sets per CPU for every branch stack user,
while the new gate uses cpc->task_epc, which __link_epc() sets only on
the CPU the task context is scheduled in on. A task with an event for
that PMU pinned to another CPU has an epc on ctx->pmu_ctx_list while
cpc->task_epc is NULL, so both paths would run and sched_task() would be
called twice per context switch. On x86 the second
__intel_pmu_lbr_restore() finds lbr_stack_state == LBR_NONE and calls
intel_pmu_lbr_reset(), throwing away the callstack the first one
restored. So gate perf_ctx_sched_task_cb() on cpc->task_epc too.

For the CPCs that perf_pmu_sched_task() now handles, the callback no
longer runs inside the perf_ctx_disable() and perf_ctx_enable() pair in
perf_event_context_sched_in(). __perf_pmu_sched_task() disables the PMU
around the call itself, so the callback still runs with it disabled.

Fixes: bd2756811766 ("perf: Rewrite core context handling")
Cc: stable@vger.kernel.org
Acked-by: Usama Arif <usama.arif@linux.dev>
Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 kernel/events/core.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 9815894b67e77..675dd05935f35 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -3757,6 +3757,9 @@ static void perf_ctx_sched_task_cb(struct perf_event_context *ctx,
 	list_for_each_entry(pmu_ctx, &ctx->pmu_ctx_list, pmu_ctx_entry) {
 		cpc = this_cpc(pmu_ctx->pmu);
 
+		if (cpc->task_epc != pmu_ctx)
+			continue;
+
 		if (cpc->sched_cb_usage && pmu_ctx->pmu->sched_task)
 			pmu_ctx->pmu->sched_task(pmu_ctx, task, sched_in);
 	}
@@ -3921,12 +3924,15 @@ static void perf_pmu_sched_task(struct task_struct *prev,
 	struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context);
 	struct perf_cpu_pmu_context *cpc;
 
-	/* cpuctx->task_ctx will be handled in perf_event_context_sched_in/out */
-	if (prev == next || cpuctx->task_ctx)
+	if (prev == next)
 		return;
 
-	list_for_each_entry(cpc, this_cpu_ptr(&sched_cb_list), sched_cb_entry)
+	list_for_each_entry(cpc, this_cpu_ptr(&sched_cb_list), sched_cb_entry) {
+		if (cpuctx->task_ctx && cpc->task_epc)
+			continue;
+
 		__perf_pmu_sched_task(cpc, sched_in ? next : prev, sched_in);
+	}
 }
 
 static void perf_event_switch(struct task_struct *task,
-- 
2.53.0-Meta

[PATCH v6 3/3] perf/core: Clear the whole branch entry in perf_clear_branch_entry()

[Puranjay Mohan]

perf_clear_branch_entry_bitfields() clears the bitfields of struct
perf_branch_entry one by one and leaves from/to alone, since callers
overwrite those straight away. The list has to be kept in sync with the
struct by hand and has already fallen behind: new_type and priv were
added to perf_branch_entry and never added here.

Only BRBE writes those two, and neither is written for every record.
brbe_set_perf_entry_type() leaves new_type alone for a branch type it
does not recognise, and priv is not set for source-only records.
arm_pmuv3.c allocates the per-CPU branch stack with kmalloc(), so such a
record carries whatever the slot held: uninitialised kmalloc() data on
the first pass over the buffer, the previous record's values after that.
Both reach userspace through the branch stack. Nothing under
arch/x86/events/ writes either field, so x86 is unaffected.

Clear the entry with a single struct assignment instead:

	*br = (struct perf_branch_entry){ };

The bitfields add up to exactly 64 bits, so there is no padding, and
every caller assigns from/to immediately afterwards, so zeroing those as
well changes nothing. PERF_BR_SPEC_NA is 0, so dropping the explicit
spec assignment leaves the behaviour unchanged. Nothing needs keeping in
sync when a field is added.

The helper no longer touches only bitfields, so rename it to
perf_clear_branch_entry().

Fixes: b190bc4ac9e6 ("perf: Extend branch type classification")
Fixes: 5402d25aa571 ("perf: Capture branch privilege information")
Suggested-by: James Clark <james.clark@linaro.org>
Reviewed-by: James Clark <james.clark@linaro.org>
Acked-by: Usama Arif <usama.arif@linux.dev>
Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 arch/x86/events/amd/brs.c   |  2 +-
 arch/x86/events/amd/lbr.c   |  2 +-
 arch/x86/events/intel/lbr.c |  6 +++---
 drivers/perf/arm_brbe.c     |  2 +-
 include/linux/perf_event.h  | 16 ++--------------
 5 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/arch/x86/events/amd/brs.c b/arch/x86/events/amd/brs.c
index 06f35a6b58a5b..68c5f42965e91 100644
--- a/arch/x86/events/amd/brs.c
+++ b/arch/x86/events/amd/brs.c
@@ -343,7 +343,7 @@ void amd_brs_drain(void)
 
 		rdmsrq(brs_from(brs_idx), from);
 
-		perf_clear_branch_entry_bitfields(br+nr);
+		perf_clear_branch_entry(br + nr);
 
 		br[nr].from = from;
 		br[nr].to   = to;
diff --git a/arch/x86/events/amd/lbr.c b/arch/x86/events/amd/lbr.c
index 5b437dc8e4ce2..3639817456119 100644
--- a/arch/x86/events/amd/lbr.c
+++ b/arch/x86/events/amd/lbr.c
@@ -183,7 +183,7 @@ void amd_pmu_lbr_read(void)
 		    entry.to.split.reserved)
 			continue;
 
-		perf_clear_branch_entry_bitfields(br + out);
+		perf_clear_branch_entry(br + out);
 
 		br[out].from	= sign_ext_branch_ip(entry.from.split.ip);
 		br[out].to	= sign_ext_branch_ip(entry.to.split.ip);
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index f8fadb0b16a45..6541c7046c2b3 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -756,7 +756,7 @@ void intel_pmu_lbr_read_32(struct cpu_hw_events *cpuc)
 
 		rdmsrq(x86_pmu.lbr_from + lbr_idx, msr_lastbranch.lbr);
 
-		perf_clear_branch_entry_bitfields(br);
+		perf_clear_branch_entry(br);
 
 		br->from	= msr_lastbranch.from;
 		br->to		= msr_lastbranch.to;
@@ -847,7 +847,7 @@ void intel_pmu_lbr_read_64(struct cpu_hw_events *cpuc)
 		if (abort && x86_pmu.lbr_double_abort && out > 0)
 			out--;
 
-		perf_clear_branch_entry_bitfields(br+out);
+		perf_clear_branch_entry(br + out);
 		br[out].from	 = from;
 		br[out].to	 = to;
 		br[out].mispred	 = mis;
@@ -921,7 +921,7 @@ static void intel_pmu_store_lbr(struct cpu_hw_events *cpuc,
 		to = rdlbr_to(i, lbr);
 		info = rdlbr_info(i, lbr);
 
-		perf_clear_branch_entry_bitfields(e);
+		perf_clear_branch_entry(e);
 
 		e->from		= from;
 		e->to		= to;
diff --git a/drivers/perf/arm_brbe.c b/drivers/perf/arm_brbe.c
index ba554e0c846c4..effbdeacfcbb3 100644
--- a/drivers/perf/arm_brbe.c
+++ b/drivers/perf/arm_brbe.c
@@ -604,7 +604,7 @@ static bool perf_entry_from_brbe_regset(int index, struct perf_branch_entry *ent
 		return false;
 
 	brbinf = bregs.brbinf;
-	perf_clear_branch_entry_bitfields(entry);
+	perf_clear_branch_entry(entry);
 	if (brbe_record_is_complete(brbinf)) {
 		entry->from = bregs.brbsrc;
 		entry->to = bregs.brbtgt;
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 48d851fbd8ea5..e034be4a473a8 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1467,21 +1467,9 @@ static inline u32 perf_sample_data_size(struct perf_sample_data *data,
 	return size;
 }
 
-/*
- * Clear all bitfields in the perf_branch_entry.
- * The to and from fields are not cleared because they are
- * systematically modified by caller.
- */
-static inline void perf_clear_branch_entry_bitfields(struct perf_branch_entry *br)
+static inline void perf_clear_branch_entry(struct perf_branch_entry *br)
 {
-	br->mispred	= 0;
-	br->predicted	= 0;
-	br->in_tx	= 0;
-	br->abort	= 0;
-	br->cycles	= 0;
-	br->type	= 0;
-	br->spec	= PERF_BR_SPEC_NA;
-	br->reserved	= 0;
+	*br = (struct perf_branch_entry){ };
 }
 
 extern void perf_output_sample(struct perf_output_handle *handle,
-- 
2.53.0-Meta