Re: [PATCH v3 1/2] powerpc/perf: Infrastructure to support checking of attr.config*

[Michael Ellerman <mpe@ellerman.id.au>]

Madhavan Srinivasan <maddy@linux.ibm.com> writes:
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 6817331e22ff..c6eeb4fdc5fd 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1958,6 +1958,20 @@ static int power_pmu_event_init(struct perf_event *event)
>  
>  		if (ppmu->blacklist_ev && is_event_blacklisted(ev))
>  			return -EINVAL;
> +		/*
> +		 * PMU config registers have fields that are
> +		 * reserved and specific value to bit field as reserved.
> +		 * For ex., MMCRA[61:62] is Randome Sampling Mode (SM)
> +		 * and value of 0b11 to this field is reserved.
> +		 *
> +		 * This check is needed only for raw event type,
> +		 * since tools like fuzzer use raw event type to
> +		 * provide randomized event code values for test.
> +		 *
> +		 */
> +		if (ppmu->check_attr_config &&
> +		    ppmu->check_attr_config(event))
> +			return -EINVAL;
>  		break;

It's not obvious from the diff, but you're only doing the check for RAW
events.

But I think that's wrong, we should always check, even if the event code
comes from the kernel we should still apply the same checks. Otherwise
we might inadvertently use an invalid event code for a HARDWARE or CACHE
event.

cheers