linux-kernel - Re: [PATCH] kallsyms: optimize kallsyms_lookup

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: <20170426200810.2126-1-naveen.n.rao@linux.vnet.ibm.com>
Date:   Thu, 27 Apr 2017 01:38:10 +0530
From:   "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
To:     Michael Ellerman <mpe@...erman.id.au>
Cc:     David Laight <David.Laight@...LAB.COM>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Paul Clarke <pc@...ibm.com>, linuxppc-dev@...ts.ozlabs.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kallsyms: optimize kallsyms_lookup_name() for a few cases

Michael Ellerman wrote:
> "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com> writes:
>> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
>> index 6a3b249a2ae1..d134b060564f 100644
>> --- a/kernel/kallsyms.c
>> +++ b/kernel/kallsyms.c
>> @@ -205,6 +205,12 @@ unsigned long kallsyms_lookup_name(const char *name)
>>  	unsigned long i;
>>  	unsigned int off;
>>  
>> +	if (!name || *name == '\0')
>> +		return false;
>> +
>> +	if (strnchr(name, MODULE_NAME_LEN, ':'))
>> +		return module_kallsyms_lookup_name(name);
>> +
>>  	for (i = 0, off = 0; i < kallsyms_num_syms; i++) {
>>  		off = kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
> 	...				
> 	}
>        	return module_kallsyms_lookup_name(name);
> 
> Is the rest of the context.
> 
> Which looks a bit odd, we already did module lookup previously?
> 
> But it's correct, because you can lookup a symbol in a module without a
> module prefix, it just looks in every module.

Yes.

> 
> You could invert the logic, ie. check that there isn't a ":" in the name
> and only in that case do the for loop, always falling back to module
> lookup.
> 
> Or just add a comment explaining why we call module lookup in two places.

Good point. Here's a v2 - I'm using a goto so as to not indent the code too much.

Thanks for the review!
- Naveen

--
[PATCH v2] kallsyms: optimize kallsyms_lookup_name() for a few cases

1. Fail early for invalid/zero length symbols.
2. Detect names of the form <mod:name> and skip checking for kernel
symbols in that case.

Signed-off-by: Naveen N. Rao <naveen.n.rao@...ux.vnet.ibm.com>
---
 kernel/kallsyms.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 6a3b249a2ae1..f7558dc5c6ac 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -205,12 +205,20 @@ unsigned long kallsyms_lookup_name(const char *name)
 	unsigned long i;
 	unsigned int off;
 
+	if (!name || *name == '\0')
+		return 0;
+
+	/* For symbols of the form <mod>:<sym>, only check the modules */
+	if (strnchr(name, MODULE_NAME_LEN, ':'))
+		goto mod;
+
 	for (i = 0, off = 0; i < kallsyms_num_syms; i++) {
 		off = kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
 
 		if (strcmp(namebuf, name) == 0)
 			return kallsyms_sym_address(i);
 	}
+mod:
 	return module_kallsyms_lookup_name(name);
 }
 EXPORT_SYMBOL_GPL(kallsyms_lookup_name);
-- 
2.12.2