linux-kernel - Re: [PATCH 1/6] POWERPC: use KSYM_NAME

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: <47978E7D.9060606@grupopie.com>
Date:	Wed, 23 Jan 2008 18:59:09 +0000
From:	Paulo Marques <pmarques@...popie.com>
To:	Cyrill Gorcunov <gorcunov@...il.com>
CC:	LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
	David Miller <davem@...emloft.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 1/6] POWERPC: use KSYM_NAME_LEN

Cyrill Gorcunov wrote:
> [Paulo Marques - Wed, Jan 23, 2008 at 06:26:28PM +0000]
>> Cyrill Gorcunov wrote:
>>> [...]
>>>  	case 's':
>>> -		getstring(tmp, 64);
>>> +		getstring(tmp, sizeof(tmp));
>>>  		if (setjmp(bus_error_jmp) == 0) {
>>>  			catch_memory_errors = 1;
>>>  			sync();
> 
> just after that poin in the original code a call to kallsyms_lookup_name
> is done - so i think it could be an overflow (of course it depends
> on what *exactly* the name is being searched, and Paulo - I didn't
> managed to get *the whole picture* of what is going on in this
> code - so the thoughs were like: kallsyms_lookup_name could find
> a quite long name restricted by KSYM_NAME_LEN (dunno how it could
> happens - due to buggy code or due to memory corruption outside,
> it does not matter - the only matter - it *could* find that long
> name).

Ah, now I understand your confusion: kallsyms_lookup_name doesn't fill 
the name. It searches the name and returns the address. It is the 
_caller_ that fills the name, not kallsyms_lookup_name.

It is used for stuff like: "give me the address of function foo":
addr = kallsyms_lookup_name("foo");

> Anyway - it's just an attempt ;) we always could drop it far-far away ;)

I think that using KSYM_NAME_LEN would be a nice cleanup for xmon, but 
it is for the powerpc guys to decide if they want to do it. I just 
wanted to point the change in behavior so that it wouldn't go unnoticed.

For all we know, the stack may at that point be close to full and an 
extra 64 bytes may tip it over the edge.

>> This also introduces a change in behavior. It is still a nice cleanup, 
>> though. So, if the powerpc people feel they can spare an extra 64 bytes of 
>> stack here, I guess it's ok.
> 
> Thanks a lot for review Paulo!

No problem. I always keep an eye out for kallsyms related stuff.

-- 
Paulo Marques - www.grupopie.com

"There cannot be a crisis today; my schedule is already full."
--
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/