linux-kernel - Re: [RFC] kallsyms_lookup always requires buffers

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] [day] [month] [year] [list]

Date:	Wed, 23 Aug 2006 09:49:27 +0200
From:	Franck Bui-Huu <vagabon.xyz@...il.com>
To:	Paulo Marques <pmarques@...popie.com>
CC:	Franck <vagabon.xyz@...il.com>, rusty@...tcorp.com.au,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] kallsyms_lookup always requires buffers

Hi

Paulo Marques wrote:
> Franck Bui-Huu wrote:
>>
>> This simple patch adds a new entry "kallsyms_lookup_gently()". The
>> name actually sucks but I can't figure out a coherent name with the
>> rest of the file. If someone could give me a better idea...
> 
> kallsyms_lookup_size_offset() ?
> 
> Granted it is a little bigger, but it tells exactly what it does and
> there are not that many users that we have to write this name all that
> often.
> 

ok.

>> This new entry does exactly the same as kallsyms_lookup() but does
>> not require any buffers to store any names. It returns 0 if it fails
>> otherwise 1.
>>
>> Do you think this can be usefull ?
> 
> You tell me, since you're proposing the change ;)
> 

:)
actually the question was rather "does this change look sensible to you ?"

>> +static int is_kernel_addr(unsigned long addr)
> 
> Maybe change the name to kallsyms_is_kernel_addr, because this function
> does more things than what the generic name implies.
> 

it sounds like a public function name. Maybe is_ksym_addr() is better ?

>> + * Lookup an address but don't bother to find any names.
>> + */
>> +int kallsyms_lookup_gently(unsigned long addr, unsigned long
>> *symbolsize,
>> +               unsigned long *offset)
>> +{
>> +    int rv;
>> +
>> +    if (is_kernel_addr(addr))
>> +        rv = !!get_symbol_pos(addr, symbolsize, offset);
>> +    else
>> +        rv = !!module_address_lookup(addr, symbolsize, offset, NULL);
>> +
>> +    return rv;
>> +}
> 
> <minor nitpick>
> 
> Why not just:
> 
>> if (is_kernel_addr(addr))
>>    return !!get_symbol_pos(addr, symbolsize, offset);
>>
>> return !!module_address_lookup(addr, symbolsize, offset, NULL);
> 
> and just get rid of "rv" completely?
> 
> </minor nitpick>
> 

No problem.

>> +EXPORT_SYMBOL_GPL(kallsyms_lookup_gently);
> 
> I agree with Arjan here. If kallsyms_lookup wasn't exported, I don't see
> a reason to export this either.
> 

Already removed.

thanks
		Franck
-
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/