linux-kernel - Re: [PATCH 2/2] perf trace: Fix segfault on perf trace -i perf.data

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]

Date:	Tue, 12 Nov 2013 10:23:43 -0300
From:	Arnaldo Carvalho de Melo <acme@...stprotocols.net>
To:	Namhyung Kim <namhyung@...nel.org>
Cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Paul Mackerras <paulus@...ba.org>,
	Ingo Molnar <mingo@...nel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	David Ahern <dsahern@...il.com>,
	Pekka Enberg <penberg@...nel.org>
Subject: Re: [PATCH 2/2] perf trace: Fix segfault on perf trace -i perf.data

Em Tue, Nov 12, 2013 at 09:27:23PM +0000, Namhyung Kim escreveu:
> Hi Arnaldo,
> 
> On Tue, Nov 12, 2013 at 08:57:00AM -0300, Arnaldo Carvalho de Melo wrote:
> > So this becomes the first part of this patch, split from yours and
> > massaged a bit so that by looking at the patch it becomes quickly clear
> > what it is doing, please let me now if I can keep this as-is (with your
> > authorship, etc).
> 
> Looks good to me.

Thanks for checking!
 
> But I just have a nitpick, please see below.
 
> > +{
> > +	evsel->priv = malloc(sizeof(struct syscall_tp));
> > +	if (evsel->priv != NULL) {
> > +		if (perf_evsel__init_sc_tp_uint_field(evsel, id))
> > +			goto out_delete;
> > +
> > +		evsel->handler = handler;
> > +		return 0;
> > +	}
> > +
> > +	return -ENOMEM;
> > +
> > +out_delete:
> > +	free(evsel->priv);
> > +	evsel->priv = NULL;
 
> Is this part needed?  I can see that perf_evsel__delete_priv() can do
> it for you anyway.  Yes I know it's needed for my later change, but I
> think we do it a bit differently.
 
> And again, is perf_evsel__delete_priv() needed?  Isn't the ->priv is
> not used for anything else?  Why not just letting perf_evsel__delete()
> handle this transparently?

Because it may point to something not allocated via malloc, so the one
who allocates it, frees it, furthermore, the one who allocates it, if
fails to complete the greater init sequence of which the allocation is
part of, frees it and leaves it as it was before the transaction
started.
 
- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/