Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALCETrUh0s8zV0S=Si4V_RB1dEUx6n2RtALC-+VJhGMEBqPTzA@mail.gmail.com>
Date: Fri, 24 Jun 2016 10:47:04 -0700
From: Andy Lutomirski <luto@...capital.net>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Brian Gerst <brgerst@...il.com>, Peter Zijlstra <peterz@...radead.org>, 
	Oleg Nesterov <oleg@...hat.com>, Andy Lutomirski <luto@...nel.org>, 
	"the arch/x86 maintainers" <x86@...nel.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, 
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>, Borislav Petkov <bp@...en8.de>, 
	Nadav Amit <nadav.amit@...il.com>, Kees Cook <keescook@...omium.org>, 
	"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, Josh Poimboeuf <jpoimboe@...hat.com>, 
	Jann Horn <jann@...jh.net>, Heiko Carstens <heiko.carstens@...ibm.com>
Subject: Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core)

On Fri, Jun 24, 2016 at 10:40 AM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Fri, Jun 24, 2016 at 10:21 AM, Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
>>
>> But as mentioned, I must have missed something. There were a number of
>> places where the code used the task_stack_page() and
>> task_thread_info() interchangably, which used to work and is no longer
>> true. There might simply be cases I missed.
>
> .. and immediately as I wrote that, I went "Duh".
>
> One place I missed was free_thread_info(), which should now free the
> stack, not the ti pointer. But it does
>
>         struct page *page = virt_to_page(ti);
>
> and frees that, which is bogus. It turns out that we do do
>
>         free_thread_info(tsk->stack);
>
> which is bogus too, and undoes it, but I think I have a few new places
> to look at..

Try patching in this thing, which cleans up a bunch of that core crap:

https://git.kernel.org/cgit/linux/kernel/git/luto/linux.git/commit/?h=x86/vmap_stack&id=7ca9fb6bbf2838cc94b2af41e94854d02649c58c

It might not apply without the rest of my series, though.

FWIW, your patch is much more lenient than my approach: I was planning
prohibiting architectures from supplying their own struct thread_info
if they put it in task_struct.  To make that work, I have patches to
remove everything but cpu, flags, and task from x86's thread_info
first.  I'm planning on tidying them up and sending them out after the
vmap stack stuff lands in -tip -- I don't want to have big series that
depend on each other flying around by email at the same time, because
everyone will go nuts trying to figure out what applies where.

--Andy

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.