linux-kernel - Re: [RFC] perf probe: Fix offline module name missmatch issue

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, 26 Apr 2016 19:46:02 +0900
From:	Masami Hiramatsu <mhiramat@...nel.org>
To:	Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com>
Cc:	linux-kernel@...r.kernel.org, acme@...nel.org,
	peterz@...radead.org, mingo@...hat.com,
	alexander.shishkin@...ux.intel.com,
	masami.hiramatsu.pt@...achi.com, wangnan0@...wei.com,
	namhyung@...nel.org, srikar@...ux.vnet.ibm.com,
	naveen.n.rao@...ux.vnet.ibm.com
Subject: Re: [RFC] perf probe: Fix offline module name missmatch issue

On Tue, 26 Apr 2016 14:26:48 +0530
Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com> wrote:

> Thanks Masami for reviewing.
> 
> Please find my replies to your comment.
> 
> On Tuesday 26 April 2016 02:54 AM, Masami Hiramatsu wrote:
> > Hi Ravi,
> >
> > On Mon, 25 Apr 2016 16:08:27 +0530
> > Ravi Bangoria <ravi.bangoria@...ux.vnet.ibm.com> wrote:
> >
> >> Perf can add a probe on kernel module which has not been loaded yet.
> >> Current implementation finds module name from path. But if filename
> >> is different from actual module name then perf fails to register
> >> probe while loading module because of mismatch in names. For example,
> >> samples/kobject/kobject-example.ko is loaded as kobject_example.
> > Ah! right, good catch!
> > Have some comment below;
> >
> >> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> >> index 8319fbb..05d0905 100644
> >> --- a/tools/perf/util/probe-event.c
> >> +++ b/tools/perf/util/probe-event.c
> >> @@ -265,6 +265,65 @@ static bool kprobe_warn_out_range(const char *symbol, unsigned long address)
> >>   	return true;
> >>   }
> >>   
> >> +/*
> >> + * NOTE:
> >> + * '.gnu.linkonce.this_module' section of kernel module elf directly
> >> + * maps to 'struct module' from linux/module.h. This section contains
> >> + * actual module name which will be used by kernel after loading it.
> >> + * But, we cannot use 'struct module' here since linux/module.h is not
> >> + * exposed to user-space. Offset of 'name' has remained same from long
> >> + * time, so hardcoding it here.
> >> + */
> >> +#ifdef __LP64__
> >> +#define MOD_NAME_OFFSET 24
> >> +#else
> >> +#define MOD_NAME_OFFSET 12
> >> +#endif
> >> +
> >> +/*
> >> + * @module can be module name of module file path. In case of path,
> >> + * inspect elf and find out what is actual module name.
> >> + * Caller has to free mod_name after using it.
> >> + */
> >> +char *find_module_name(const char *module)
> > Could you make this function static, since there is no caller outside
> > this file?
> 
> Yes. no caller outside of this file. But,
> 
> In this patch, function find_module_name is defined outside of
> #ifdef HAVE_DWARF_SUPPORT while it's being called from inside of
> #ifdef HAVE_DWARF_SUPPORT.
> 
> If I make it static and if there is no dwarf support, there will be 
> compilation
> error about function defined but not used.
> 
> And in second patch("perf probe: Fix module probe issue if no dwarf 
> support"),
> I'm calling it from outside of #ifdef HAVE_DWARF_SUPPORT.
> 
> So I have two options:
> 1. merge both the patches and make definition as static
> 2. make function static in second patch
> 
> I've chose second approach and sent v2. But please let me know if there is
> better way to do it.

Ah, I see.
In that case, you can swap the patch in the series and move find_module_name
in the other patch ;)

Thanks!

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