linux-kernel - Re: [PATCH perf/core v7 02/21] perf buildid: Fix to set correct dso name for kallsyms

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: <20160512110212.c8bb188b823e1a637e53ae26@kernel.org>
Date:	Thu, 12 May 2016 11:02:12 +0900
From:	Masami Hiramatsu <mhiramat@...nel.org>
To:	Arnaldo Carvalho de Melo <acme@...nel.org>
Cc:	linux-kernel@...r.kernel.org, Namhyung Kim <namhyung@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Hemant Kumar <hemant@...ux.vnet.ibm.com>,
	Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
	Brendan Gregg <brendan.d.gregg@...il.com>
Subject: Re: [PATCH perf/core v7 02/21] perf buildid: Fix to set correct dso
 name for kallsyms

On Wed, 11 May 2016 12:45:00 -0300
Arnaldo Carvalho de Melo <acme@...nel.org> wrote:

> Em Wed, May 11, 2016 at 10:51:37PM +0900, Masami Hiramatsu escreveu:
> > Fix to set correct dso name ("[kernel.kallsyms]") for
> > kallsyms, as for vdso does.
> 
> Can you rewrite the above comment and also break this down in two
> patches,

No, could you explain what parts this consists of?
I think this patch is done just one thing. Actually I can rewrite
this as one-line patch like below

  	if (asprintf(&filename, "%s%s%s", buildid_dir, slash ? "/" : "",
-		     is_vdso ? DSO__NAME_VDSO : realname) < 0)
+		     is_vdso ? DSO__NAME_VDSO : (is_kallsyms ? "[kernel.kallsyms]": realname)) < 0)

But indeed this is not readable so clean it up like below.

> probably decribing what is the problem that it fixes?

Ah, indeed. This is actually not a fix for existing bug, instead
it will prevent buggy behavior.　Current function can get any filepath
with is_vdso = true or is_kallsyms = true, but it seems easily giving
contradictory combination.

Hmm, OK, I think I have to solve this issue in another way.

Thanks!

> 
> - Arnaldo
>  
> > Signed-off-by: Masami Hiramatsu <mhiramat@...nel.org>
> > ---
> >  tools/perf/util/build-id.c |   12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> > index b6ecf87..234d8a1 100644
> > --- a/tools/perf/util/build-id.c
> > +++ b/tools/perf/util/build-id.c
> > @@ -343,21 +343,25 @@ void disable_buildid_cache(void)
> >  static char *build_id_cache__dirname_from_path(const char *name,
> >  					       bool is_kallsyms, bool is_vdso)
> >  {
> > -	char *realname = (char *)name, *filename;
> > +	const char *realname = name;
> > +	char *filename;
> >  	bool slash = is_kallsyms || is_vdso;
> >  
> >  	if (!slash) {
> >  		realname = realpath(name, NULL);
> >  		if (!realname)
> >  			return NULL;
> > -	}
> > +	} else if (is_vdso)
> > +		realname = DSO__NAME_VDSO;
> > +	else
> > +		realname = "[kernel.kallsyms]";
> >  
> >  	if (asprintf(&filename, "%s%s%s", buildid_dir, slash ? "/" : "",
> > -		     is_vdso ? DSO__NAME_VDSO : realname) < 0)
> > +		     realname) < 0)
> >  		filename = NULL;
> >  
> >  	if (!slash)
> > -		free(realname);
> > +		free((char *)realname);
> >  
> >  	return filename;
> >  }


-- 
Masami Hiramatsu <mhiramat@...nel.org>