linux-kernel - Re: [PATCH] perf probe: Fix module probing with shortname

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: <20150924132304.GH1897@kernel.org>
Date:	Thu, 24 Sep 2015 10:23:04 -0300
From:	'Arnaldo Carvalho de Melo' <acme@...nel.org>
To:	平松雅巳 / HIRAMATU，MASAMI 
	<masami.hiramatsu.pt@...achi.com>
Cc:	"Wangnan (F)" <wangnan0@...wei.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"lizefan@...wei.com" <lizefan@...wei.com>,
	"pi3orama@....com" <pi3orama@....com>,
	Namhyung Kim <namhyung@...nel.org>,
	Jiri Olsa <jolsa@...hat.com>
Subject: Re: [PATCH] perf probe: Fix module probing with shortname

Em Thu, Sep 24, 2015 at 10:10:31AM +0000, 平松雅巳 / HIRAMATU，MASAMI escreveu:
> 
> 
> -- 
> Masami HIRAMATSU
> Linux Technology Research Center, System Productivity Research Dept.
> Center for Technology Innovation - Systems Engineering
> Hitachi, Ltd., Research & Development Group
> E-mail: masami.hiramatsu.pt@...achi.com
> 
> 
> >-----Original Message-----
> >From: Arnaldo Carvalho de Melo [mailto:acme@...nel.org]
> >Sent: Thursday, September 24, 2015 10:50 AM
> >To: Wangnan (F)
> >Cc: linux-kernel@...r.kernel.org; lizefan@...wei.com; pi3orama@....com; Namhyung Kim; Jiri Olsa; 平松雅巳 / HIRAMATU，MASAMI
> >Subject: Re: [PATCH] perf probe: Fix module probing with shortname
> >
> >Em Wed, Sep 23, 2015 at 01:03:19PM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Em Tue, Sep 22, 2015 at 11:49:02PM -0300, Arnaldo Carvalho de Melo escreveu:
> >> > Em Wed, Sep 23, 2015 at 09:14:44AM +0800, Wangnan (F) escreveu:
> >> > > On 2015/9/22 21:35, Arnaldo Carvalho de Melo wrote:
> >> > > >Em Tue, Sep 22, 2015 at 03:34:32AM +0000, Wang Nan escreveu:
> >> > > >>This is caused by a misunderstood of dso->kernel in kernel_get_module_dso()
> >> > > >>that, for kernel module, dso->kernel is DSO_TYPE_USER. dso->kernel is DSO_TYPE_KERNEL
> >> > > >>iff dso is vmlinux.
> >> > > >Kernel modules having DSO_TYPE_USER seems to be the bug, no? I'll try to
> >> > > >check that...
> >
> >> > > I also noticed this problem when I working on commit
> >> > > 1f121b03d058dd07199d8924373d3c52a207f63b ("perf tools: Deal with
> >> > > kernel module names in '[]' correctly") ;)
> >
> >> > Thanks for working on this, it is an area that needs cleaning up, too
> >> > many ways to say what a dso is, will study your findings and try to come
> >> > up with a patch proposal tomorrow.
> >> > > So care must be taken.
> >>
> >> So, yes, there are multiple uses for this dso->kernel thing, we need to
> >> look at each one and go on clarifying it so that this gets corrected and
> >> sane, but I think we need some helpers to clarify all this, namely:
> >>
> >> 	Adding DSO_TYPE_KMODULE and DSO_TYPE_GUEST_KMODULE, setting
> >> dso->kernel with it when loading host and guest kernel modules and
> >> adding:
> >>
> >> 	bool dso__is_host_kernel(struct dso *dso)
> >> 	bool dso__is_host_kmodule(struct dso *dso)
> >>
> >> 	bool dso__is_guest_kernel(struct dso *dso)
> >
> >So I tried this, ended up basically using __map__is_kernel(), in several
> >places, which I think is right, and I've stashed in my tmp.perf/core
> >branch so that you can take a look.
> >
> >But then I realized that we already have a better way to achieve what is
> >needed in that function, see the patch below, fixed things for me:
> 
> Hmm, this is a bit hacky, but yes, it looks good to me.

You mean the [] dso->short_name part? But that was already being taken
into account by the previous, dso list search.

But yeah, probably we should remove those brackets, anything requiring
full disambiguation should use the dso->long_name anyway,  but I'll
leave this for later :)

> However, I hope to have above solution, for future use.

Right, my work now is to reduce the use of dso->kernel, using
__map__is_kernel(map), we'll see what is left.
 
> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>

Thanks!
 
> Thanks!
> 
> >
> >diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> >index 2b78e8f19b45..7fb0533ab18c 100644
> >--- a/tools/perf/util/probe-event.c
> >+++ b/tools/perf/util/probe-event.c
> >@@ -269,12 +269,13 @@ static int kernel_get_module_dso(const char *module, struct dso **pdso)
> > 	int ret = 0;
> >
> > 	if (module) {
> >-		list_for_each_entry(dso, &host_machine->dsos.head, node) {
> >-			if (!dso->kernel)
> >-				continue;
> >-			if (strncmp(dso->short_name + 1, module,
> >-				    dso->short_name_len - 2) == 0)
> >-				goto found;
> >+		char module_name[128];
> >+
> >+		snprintf(module_name, sizeof(module_name), "[%s]", module);
> >+		map = map_groups__find_by_name(&host_machine->kmaps, MAP__FUNCTION, module_name);
> >+		if (map) {
> >+			dso = map->dso;
> >+			goto found;
> > 		}
> > 		pr_debug("Failed to find module %s.\n", module);
> > 		return -ENOENT;
--
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/