Re: [PATCH] perf/x86/amd: check event before enable to avoid GPF

[George Kennedy <george.kennedy@oracle.com>]

Hi Ravi,

On 5/29/2024 11:37 PM, Ravi Bangoria wrote:
> Hi George,
>
>> Events can be deleted and the entry can be NULL.
> Can you please also explain "how".
It looks like x86_pmu_stop() is clearing the bit in active_mask and 
setting the events entry to NULL (and doing it in the correct order) for 
the same events index that amd_pmu_enable_all() is trying to enable.
>
>> Check event for NULL in amd_pmu_enable_all() before enable to avoid a GPF.
>> This appears to be an AMD only issue.
>>
>> Syzkaller reported a GPF in amd_pmu_enable_all.
> Can you please provide a bug report link? Also, any reproducer?
The Syzkaller reproducer can be found in this link:
https://lore.kernel.org/netdev/CAMt6jhyec7-TSFpr3F+_ikjpu39WV3jnCBBGwpzpBrPx55w20g@mail.gmail.com/T/#u
>
>> @@ -760,7 +760,8 @@ static void amd_pmu_enable_all(int added)
>>   		if (!test_bit(idx, cpuc->active_mask))
>>   			continue;
>>   
>> -		amd_pmu_enable_event(cpuc->events[idx]);
>> +		if (cpuc->events[idx])
>> +			amd_pmu_enable_event(cpuc->events[idx]);
> What if cpuc->events[idx] becomes NULL after if (cpuc->events[idx]) but
> before amd_pmu_enable_event(cpuc->events[idx])?
Good question, but the crash has not reproduced with the proposed fix in 
hours of testing. It usually reproduces within minutes without the fix.

Thank you,
George
>
> Thanks,
> Ravi