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]

Date:	Fri, 25 Sep 2015 01:57:04 +0000
From:	平松雅巳 / HIRAMATU，MASAMI 
	<masami.hiramatsu.pt@...achi.com>
To:	"'Arnaldo Carvalho de Melo'" <acme@...nel.org>
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

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.

Ah, OK. I didn't know that...

>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 :)

OK, thanks,

>
>> 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!
>
>> 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;