Re: [PATCH v1 4/5] perf: Introduce address range filtering

[Alexander Shishkin]

Peter Zijlstra <peterz@xxxxxxxxxxxxx> writes:

> On Thu, Apr 21, 2016 at 06:17:02PM +0300, Alexander Shishkin wrote:
>> +			/* otherwise, fall through to the cross-call */
>> +		}
>> +
>> +		/* matches smp_wmb() in event_sched_in() */
>> +		smp_rmb();
>> +
>> +		ret = cpu_function_call(READ_ONCE(event->oncpu),
>> +					__perf_event_addr_filters_setup, &id);
>> +	} while (ret == -EAGAIN);
>> +
>> +	return ret;
>> +}
>
> This whole thing seems rather full of tricky :/

Yes, so I got rid of it.

>> +	mm = get_task_mm(event->ctx->task);
>> +	if (!mm)
>> +		return;
>
> kernel threads may not have a filter?

Good catch. They won't have executable vmas, but they can still have
kernel regions, so here we should skip scanning the address space and
proceed to propagating the filters to the pmu.

>> +/*
>> + * Address range filtering: limiting the data to certain
>> + * instruction address ranges. Filters are ioctl()ed to us from
>> + * userspace as ascii strings.
>> + *
>> + * Filter string format:
>> + *
>> + * ACTION SOURCE:RANGE_SPEC
>> + * where ACTION is one of the
>> + *  * "filter": limit the trace to this region
>> + *  * "start": start tracing from this address
>> + *  * "stop": stop tracing at this address/region;
>> + * SOURCE is either "file" or "kernel"
>> + * RANGE_SPEC is
>> + *  * for "kernel": <start address>[/<size>]
>> + *  * for "file":   <start address>[/<size>]@</path/to/object/file>
>> + *
>> + * if <size> is not specified, the range is treated as a single address.
>> + */
>
> Scary bit of string manipulation there.. have you tried fuzzing it? ;-)

Not yet. But you have a point. :)

So below is what I came up with instead of this patch. The 5/5 also
changed as a result, and I pushed the whole thing here [1].

[1] https://git.kernel.org/cgit/linux/kernel/git/ash/linux.git/log/?h=perf-addr-filters