Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY_BREAKPOINT.

[Andi Kleen]

On Tue, Nov 07, 2017 at 09:15:41AM +0100, Peter Zijlstra wrote:
> On Mon, Nov 06, 2017 at 03:16:58PM -0800, Andi Kleen wrote:
> > > +static int _perf_event_modify_breakpoint(struct perf_event *bp,
> > > +					 struct perf_event_attr *attr)
> > > +{
> > > +	u64 old_addr = bp->attr.bp_addr;
> > > +	u64 old_len = bp->attr.bp_len;
> > > +	int old_type = bp->attr.bp_type;
> > > +	int err = 0;
> > > +
> > > +	_perf_event_disable(bp);
> > > +
> > > +	bp->attr.bp_addr = attr->bp_addr;
> > > +	bp->attr.bp_type = attr->bp_type;
> > > +	bp->attr.bp_len = attr->bp_len;
> > 
> > You don't check any of the other fields, so user space is free
> > to fill in junk. That means they can never be used for anything.
> > It would be better to check at least some of them for being
> > zero, and also that the type matches the break point.
> 
> Yes, the values should at the very least get the exact same validation
> they would get on creating an event with those values.

In this case the ioctl could be also generalized. Not call it _BREAKPOINT,
just _MODIFY. Just for now it would be only implemented for break points,
but that could be potentially extended later.

-Andi