linux-kernel - Re: [RFC PATCH 0/5] x86: check stack overflows more reliably

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 Nov 2011 17:55:35 +0900
From:	HAYASAKA Mitsuo <mitsuo.hayasaka.hu@...achi.com>
To:	Jason Baron <jbaron@...hat.com>
Cc:	Pekka Enberg <penberg@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Randy Dunlap <rdunlap@...otime.net>, x86@...nel.org,
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
	yrl.pp-manager.tt@...achi.com
Subject: Re: [RFC PATCH 0/5] x86: check stack overflows more reliably

Hi Jason,

> Another thought might be to make stack_overflow_check() depend on a jump
> label. Its not something that going to be switch on/off often, and then perhaps
> we wouldn't even need DEBUG_STACKOVERFLOW...It seems like a good
> use-case to me.

It is interesting to use a jump label for stack overflow checking...
However, I'd like to implement this detail-check simply using
DEBUG_STACKOVERFLOW option because I guess stack_overflow_check() will
be seldom switched on/off after the system operation starts, as you said.

In addition, I will change the default overflow checking to the
detail-check instead of the original one if the option is enabled in
Kconfig. This is because it turned out that the additional checking
overhead is negligible (about 17 cycles) from the evaluation below.


[Evaluation]

The performance of the detail-check was compared to original one
which checks kernel stack only, on the following conditions.

- Measure the worst performance using tsc.
  In the detail-check, all stack type were checked for every IRQ
  even if the stack pointer pointed to all available stacks.
  That is, the patch was changed a little for this evaluation.
- Calculate the average from the 30,000 IRQ evaluations.

The results show the performance regression of the detail-check
for a IRQ is 17 cycles compared to the original one.


	| Original | Detail Check |
-----------------------------------
Average	|    49    |      66      |
(cycles)

I think this overhead can be ignored.

Thanks

(2011/11/18 1:59), Jason Baron wrote:
> On Tue, Nov 08, 2011 at 04:34:28PM +0900, HAYASAKA Mitsuo wrote:
>> Hi Pekka,
>>
>> Thank you for your comments.
>>
>> (2011/11/07 16:00), Pekka Enberg wrote:
>>> On Mon, Nov 7, 2011 at 7:51 AM, Mitsuo Hayasaka
>>> <mitsuo.hayasaka.hu@...achi.com> wrote:
>>>> (2) check stack overflow in detail
>>>>    Currently, only kernel stack is checked for the overflow,
>>>>    which is not sufficient for enterprise systems. To enhance
>>>>    reliability, expand stack overflow checking to IRQ and
>>>>    exception stacks optionally. This is disabled by default
>>>>    in Kconfig.
>>>
>>> This sounds useful. What's the reason for not enabling this by
>>> default? Performance regressions?
>>
>> I'm worried about performance regressions because this patch checks 
>> a stack overflow in detail.
>>
>> However, I guess there is no problem for enabling it by default 
>> since this option is for debug and appears only if a DEBUG_STACKOVERFLOW
>> option is enabled.
>>
>> So, I'd like to send the revised patch if it does not have any further problem.
>>
>>
> 
> Another thought might be to make stack_overflow_check() depend on a jump
> label. Its not something that going to be switch on/off often, and then perhaps
> we wouldn't even need DEBUG_STACKOVERFLOW...It seems like a good
> use-case to me.
> 
> Thanks,
> 
> -Jason
> 

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