linux-kernel - Re: [PATCH] perf/core: fast breakpoint modification via _IOC_MODIFY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171107081541.GF3326@worktop>
Date:   Tue, 7 Nov 2017 09:15:41 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Andi Kleen <ak@...ux.intel.com>
Cc:     Milind Chabbi <chabbi.milind@...il.com>, jolsa@...hat.com,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        Hari Bathini <hbathini@...ux.vnet.ibm.com>,
        Jin Yao <yao.jin@...ux.intel.com>,
        Kan Liang <kan.liang@...el.com>,
        Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf/core: fast breakpoint modification via
 _IOC_MODIFY_BREAKPOINT.

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.