linux-kernel - Re: [PATCH] Reduce __print_symbol/sprint

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, 21 Sep 2007 15:21:05 +0100
From:	Paulo Marques <pmarques@...popie.com>
To:	gilboad@...il.com
CC:	Satyam Sharma <satyam@...radead.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Reduce __print_symbol/sprint_symbol stack usage.

Gilboa Davara wrote:
> Hello Paulo,

Hi, Gilboa

> [snip]
>> [...]
>> if we change the interface from "print_symbol(fmt, addr)" to 
>> "print_symbol(prefix, addr, int newline)" we can simply do:
>>
>> printk(prefix);
>> printk_symbol(addr);
>> if (newline)
>> 	printk("\n");
>>
>> where "printk_symbol" is a new function that does the same as 
>> sprint_symbol, but does "printk" instead of "sprintf".
>>
>> This should reduce immensely the stack usage of print_symbol without the 
>> need for locking.
> 
> I fully agree.
> ... Further more, multiple printk_symbols should be combined into a
> single, multi-line printk transaction. (To prevent debug printk's from
> trashing a BUG() dump_stack). 

Usually the developer can separate the output by hand in the unlikely 
case of simultaneous concurrent users of printk, so I don't think this 
is really a big problem.

>> Of course this requires changing _all_ callers of print_symbol to use 
>> the new interface, but these are less than 100 ;)
> 
> This is my first contribution to the Linux kernel. As such I rather
> start small, and work my way up slowly. (Read: solve the immediate stack
> over-run now, think about changing the symbol_display interface later)

Yes, but this is a sensitive area, so you can not just implement 
something now that can cause regressions, and just fix it later.

Kernel panics are quite rare and the information provided by the stack 
dump is _extremely_ precious.

Even more, risking deadlocks caused by something that should only be 
used to produce debug information is even worse.

>> Comments?
> 
> I do agree that the current interface needs work.
> 
> ... But as I said, I rather start slowly and on small scale. (Though I
> did find a rather problematic place to start at... ;))

Well, if we agree that this is the way to go, then the way to start 
slowly would be to submit a patch that makes both interfaces available 
for a while and changes the most stack-critical callers to the new 
interface.

Then slowly keep submitting patches to change other callers 
progressively until there are no more callers of the old interface. At 
that point we can just drop it entirely.

-- 
Paulo Marques - www.grupopie.com

"God is love. Love is blind. Ray Charles is blind. Ray Charles is God."
-
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/