Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 14 Nov 2017 08:13:43 -0800
From: Andy Lutomirski <luto@...nel.org>
To: Alexander Popov <alex.popov@...ux.com>
Cc: Andy Lutomirski <luto@...nel.org>, 
	"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, Kees Cook <keescook@...omium.org>, 
	PaX Team <pageexec@...email.hu>, Brad Spengler <spender@...ecurity.net>, 
	Ingo Molnar <mingo@...nel.org>, Peter Zijlstra <peterz@...radead.org>, 
	Tycho Andersen <tycho@...ker.com>, Laura Abbott <labbott@...hat.com>, 
	Mark Rutland <mark.rutland@....com>, Ard Biesheuvel <ard.biesheuvel@...aro.org>, 
	Borislav Petkov <bp@...en8.de>, Thomas Gleixner <tglx@...utronix.de>, "H . Peter Anvin" <hpa@...or.com>, 
	X86 ML <x86@...nel.org>
Subject: Re: [PATCH RFC v5 2/5] gcc-plugins: Add STACKLEAK plugin for tracking
 the kernel stack

On Tue, Nov 14, 2017 at 7:36 AM, Alexander Popov <alex.popov@...ux.com> wrote:
> On 30.10.2017 21:06, Alexander Popov wrote:
>> On 30.10.2017 20:32, Peter Zijlstra wrote:
>>> On Mon, Oct 30, 2017 at 07:51:33PM +0300, Alexander Popov wrote:
>>>> When the thread stack is exhausted, this BUG() is hit. But do_error_trap(),
>>>> which handles the exception, calls track_stack() itself again (since it is
>>>> instrumented by the gcc plugin). So this recursion proceeds with exhausting the
>>>> thread stack.
>>>
>>> Add a __attribute__((nostacktrack)) on it?
>>
>> Yes, I already tried some blacklisting in the plugin, but it didn't really help,
>> because:
>>
>> 1. there are other (more than 5) instrumented functions, that are called during
>> BUG() handling too;
>>
>> 2. decreasing CONFIG_STACKLEAK_TRACK_MIN_SIZE would add more instrumented
>> functions, which should be manually blacklisted (not good).
>>
>> I guess handling BUG() in another stack would be a solution. For example, Andy
>> Lutomirski calls handle_stack_overflow in the DOUBLEFAULT_STACK
>> (arch/x86/mm/fault.c). Should I do something similar?
>
> Hello Andy! May I ask your advice?
>
> When CONFIG_VMAP_STACK is disabled and STACKLEAK is enabled (for example, on
> x86_32), we need another way to detect stack depth overflow. That is the reason
> of having this BUG() in track_stack(). But it turns out to be recursive since
> track_stack() will be called again during BUG() handling.

What does the STEAKLACK plugin actually do?  I haven't followed this enough.

>
> We can avoid that recursion by handling oops in another stack. It looks similar
> to the way you call handle_stack_overflow() in arch/x86/mm/fault.c. But it seems
> that I can't reuse that code, am I right?

You'd probably have to make 32-bit compatible, which means making a
32-bit variant of this thingy:

                asm volatile ("movq %[stack], %%rsp\n\t"
                              "call handle_stack_overflow\n\t"
                              "1: jmp 1b"
                              : ASM_CALL_CONSTRAINT
                              : "D" ("kernel stack overflow (page fault)"),
                                "S" (regs), "d" (address),
                                [stack] "rm" (stack));

Or you could force a double-fault.

>
> How should I do it properly?
>
> By the way, you wrote that you have some entry code changes which conflict with
> STACKLEAK. May I ask for more details?

It's this thing:

https://git.kernel.org/pub/scm/linux/kernel/git/luto/linux.git/log/?h=x86/entry_stack.wip

and I'll probably drop the ".wip" from the name shortly.

>
> Best regards,
> Alexander

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.