linux-kernel - Re: [PATCH] perf inject: correct event attribute sizes

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] [day] [month] [year] [list]

Date:   Fri, 15 Jan 2021 16:48:25 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Denis Nikitin <denik@...omium.org>,
        linux-perf-users@...r.kernel.org, peterz@...radead.org,
        mingo@...hat.com, mark.rutland@....com,
        alexander.shishkin@...ux.intel.com, namhyung@...nel.org,
        al.grant@....com, al.grant@...s.arm.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf inject: correct event attribute sizes

Em Wed, Dec 23, 2020 at 11:11:05PM +0100, Jiri Olsa escreveu:
> On Tue, Dec 15, 2020 at 10:58:16PM -0800, Denis Nikitin wrote:
> > When perf inject reads a perf.data file from an older version of perf,
> > it writes event attributes into the output with the original size field,
> > but lays them out as if they had the size currently used. Readers see
> > a corrupt file. Update the size field to match the layout.
> > 
> > Signed-off-by: Al Grant <al.grant@...s.arm.com>
> > Signed-off-by: Denis Nikitin <denik@...omium.org>
> > ---
> >  tools/perf/util/header.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> > index be850e9f8852..0d95981df8dd 100644
> > --- a/tools/perf/util/header.c
> > +++ b/tools/perf/util/header.c
> > @@ -3331,6 +3331,14 @@ int perf_session__write_header(struct perf_session *session,
> >  	attr_offset = lseek(ff.fd, 0, SEEK_CUR);
> >  
> >  	evlist__for_each_entry(evlist, evsel) {
> > +		if (evsel->core.attr.size < sizeof(evsel->core.attr)) {
> > +			/*
> > +			 * We are likely in "perf inject" and have read
> > +			 * from an older file. Update attr size so that
> > +			 * reader gets the right offset to the ids.
> > +			 */
> > +			evsel->core.attr.size = sizeof(evsel->core.attr);
> > +		}
> 
> seems ok, just wondering if it would be better
> to fix it in perf_event__process_attr instead
> 
> where we know where the data is coming from,
> here we could cover for some unexpected case
> of having different attr.size?
> 
> anyway both would be probably equal, I'm ok
> with either way
> 
> Acked-by: Jiri Olsa <jolsa@...hat.com>

Thanks, applied.

- Arnaldo