| 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
| ||
|
Date: Fri, 12 Feb 2016 13:42:10 -0200
From: Arnaldo Carvalho de Melo <acme@...hat.com>
To: Jiri Olsa <jolsa@...hat.com>
Cc: Wang Nan <wangnan0@...wei.com>,
Alexei Starovoitov <ast@...nel.org>,
Brendan Gregg <brendan.d.gregg@...il.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Cody P Schafer <dev@...yps.com>,
"David S. Miller" <davem@...emloft.net>,
He Kuang <hekuang@...wei.com>,
Jérémie Galarneau
<jeremie.galarneau@...icios.com>, Jiri Olsa <jolsa@...nel.org>,
Kirill Smelkov <kirr@...edi.com>,
Li Zefan <lizefan@...wei.com>,
Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
Namhyung Kim <namhyung@...nel.org>,
Peter Zijlstra <peterz@...radead.org>, pi3orama@....com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/54] perf data: Fix releasing event_class
Em Fri, Feb 12, 2016 at 01:19:42PM +0100, Jiri Olsa escreveu:
> On Thu, Feb 11, 2016 at 07:04:13PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Feb 05, 2016 at 02:01:30PM +0000, Wang Nan escreveu:
> > > A new patch of libbabeltrace [1] reveals a object leak problem in
> > > perf data CTF support: perf code never release event_class which is
> > > allocated in add_event() and stored in evsel's private field.
> > >
> > > If libbabeltrace has the above patch applied, leaking event_class
> > > prevent writer being destroied and flushing metadata. For example:
> >
> > Ok, so if the user has an older version of this libbabeltrace, what
> > happens?
>
> it's standard cleanup that should be there even for old version,
>
> IIUC the problem is that new version of libbabeltrace started
> to check on refcounts on some exit function and gets crazy
> if there's a mess/leak
>
> IIRC I've already acked this one
Ok, Wang, if you notice such Acked-by, collect them, i.e. add them to
new versions of your patchkits, that helps me in processing them, i.e.
knowing that there was already discussion/acknowledgement from people
directly involved in the affected code.
thanks,
- Arnaldo
> jirka
>
> >
> > Would he/she gets some warning about the requirement of a later version?
> >
> > - Arnaldo
> >
> > > $ ./perf record ls
> > > Lowering default frequency rate to 500.
> > > Please consider tweaking /proc/sys/kernel/perf_event_max_sample_rate.
> > > perf.data
> > > [ perf record: Woken up 1 times to write data ]
> > > [ perf record: Captured and wrote 0.012 MB perf.data (12 samples) ]
> > > $ ./perf data convert --to-ctf ./out.ctf
> > > [ perf data convert: Converted 'perf.data' into CTF data './out.ctf' ]
> > > [ perf data convert: Converted and wrote 0.000 MB (12 samples) ]
> > > $ cat ./out.ctf/metadata
> > > $ ls -l ./out.ctf/metadata
> > > -rw-r----- 1 w00229757 mm 0 Jan 27 10:49 ./out.ctf/metadata
> > >
> > > The correct result should be:
> > > ...
> > > $ cat ./out.ctf/metadata
> > > /* CTF 1.8 */
> > >
> > > trace {
> > > [SNIP]
> > >
> > > $ ls -l ./out.ctf/metadata
> > > -rw-r----- 1 w00229757 mm 2446 Jan 27 10:52 ./out.ctf/metadata
> > >
> > > The full story is:
> > >
> > > Patch [1] of babeltrace redesign reference counting scheme. In that
> > > patch:
> > >
> > > * writer <- trace (bt_ctf_writer_create)
> > > * trace <- stream_class (bt_ctf_trace_add_stream_class)
> > > * stream_class <- event_class (bt_ctf_stream_class_add_event_class)
> > > ('<-' means 'is a parent of')
> > >
> > > Holding of event_class causes reference count of corresponding
> > > 'writer' increases through parent chain. Perf expect 'writer' is
> > > released (so metadata is flushed) through bt_ctf_writer_put() in
> > > ctf_writer__cleanup(). However, since it never release event_class,
> > > the reference of 'writer' won't be reduced, so bt_ctf_writer_put()
> > > won't lead releasing of writer.
> > >
> > > Before this CTF patch, !(writer <- trace). Even event_class leak,
> > > writer is able to be released.
> > >
> > > [1] https://github.com/efficios/babeltrace/commit/e6a8e8e4744633807083a077ff9f101eb97d9801
> > >
> > > Signed-off-by: Wang Nan <wangnan0@...wei.com>
> > > Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> > > Cc: Jiri Olsa <jolsa@...nel.org>
> > > Cc: Jérémie Galarneau <jeremie.galarneau@...icios.com>
> > > Cc: Zefan Li <lizefan@...wei.com>
> > > Cc: pi3orama@....com
> > > ---
> > > tools/perf/util/data-convert-bt.c | 18 ++++++++++++++++++
> > > 1 file changed, 18 insertions(+)
> > >
> > > diff --git a/tools/perf/util/data-convert-bt.c b/tools/perf/util/data-convert-bt.c
> > > index 34cd1e4..b722e57 100644
> > > --- a/tools/perf/util/data-convert-bt.c
> > > +++ b/tools/perf/util/data-convert-bt.c
> > > @@ -858,6 +858,23 @@ static int setup_events(struct ctf_writer *cw, struct perf_session *session)
> > > return 0;
> > > }
> > >
> > > +static void cleanup_events(struct perf_session *session)
> > > +{
> > > + struct perf_evlist *evlist = session->evlist;
> > > + struct perf_evsel *evsel;
> > > +
> > > + evlist__for_each(evlist, evsel) {
> > > + struct evsel_priv *priv;
> > > +
> > > + priv = evsel->priv;
> > > + bt_ctf_event_class_put(priv->event_class);
> > > + zfree(&evsel->priv);
> > > + }
> > > +
> > > + perf_evlist__delete(evlist);
> > > + session->evlist = NULL;
> > > +}
> > > +
> > > static int setup_streams(struct ctf_writer *cw, struct perf_session *session)
> > > {
> > > struct ctf_stream **stream;
> > > @@ -1171,6 +1188,7 @@ int bt_convert__perf2ctf(const char *input, const char *path, bool force)
> > > (double) c.events_size / 1024.0 / 1024.0,
> > > c.events_count);
> > >
> > > + cleanup_events(session);
> > > perf_session__delete(session);
> > > ctf_writer__cleanup(cw);
> > >
> > > --
> > > 1.8.3.4
Powered by blists - more mailing lists