Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 24 Jun 2016 08:25:20 -0400
From: Brian Gerst <>
To: Linus Torvalds <>
Cc: Peter Zijlstra <>, Oleg Nesterov <>, 
	Andy Lutomirski <>, Andy Lutomirski <>, 
	"the arch/x86 maintainers" <>, Linux Kernel Mailing List <>, 
	"" <>, Borislav Petkov <>, 
	Nadav Amit <>, Kees Cook <>, 
	"" <>, Josh Poimboeuf <>, 
	Jann Horn <>, Heiko Carstens <>
Subject: Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core)

On Fri, Jun 24, 2016 at 2:17 AM, Linus Torvalds
<> wrote:
> On Thu, Jun 23, 2016 at 12:17 PM, Linus Torvalds
> <> wrote:
>> With the goal being that I'm hoping that we can then actually get rid
>> of this (at least on x86-64, even if we leave it in some other
>> architectures) in 4.8.
> The context here was that we could almost get rid of thread-info
> entirely, at least for x86-64, by moving it into struct task_struct.
> It turns out that we're not *that* far off after the obvious cleanups
> I already committed, but I couldn't get things quite to work.
> I'm attaching a patch that I wrote today that doesn't boot, but "looks
> right". The reason I'm attaching it is because I'm hoping somebody
> wants to take a look and maybe see what else I missed, but mostly
> because I think the patch is interesting in a couple of cases where we
> just do incredibly ugly things.
> First off, some code that Andy wrote when he re-organized the entry path.
> Oh Gods, Andy. That pt_regs_to_thread_info() thing made me want to do
> unspeakable acts on a poor innocent wax figure that looked _exactly_
> like you.
> I just got rid of pt_regs_to_thread_info() entirely, and just replaced
> it with current_thread_info().  I'm not at all convinced that trying
> to be that clever was really a good idea.
> Secondly, the x86-64 ret_from_fork calling convention was documented
> wrongly. It says %rdi contains the previous task pointer. Yes it does,
> but it doesn't mention that %r8 is supposed to contain the new
> thread_info. That was fun to find.
> And thirdly, the stack size games that asm/kprobes.h plays are just
> disgusting. I stared at that code for much too long. I may in fact be
> going blind as a result.
> The rest was fairly straightforward, although since the end result
> doesn't actually work, that "straightforward" may be broken too. But
> the basic approach _looks_ sane.
> Comments? Anybody want to play with this and see where I went wrong?
> (Note - this patch was written on top of the two thread-info removal
> patches I committed in
>    da01e18a37a5 x86: avoid avoid passing around 'thread_info' in stack
> dumping code
>    6720a305df74 locking: avoid passing around 'thread_info' in mutex
> debugging code
> and depends on them, since "ti->task" no longer exists with
> CONFIG_THREAD_INFO_IN_TASK. "ti" and "task" will have the same value).
>                  Linus

  * A newly forked process directly context switches into this address.
  * rdi: prev task we switched from
+ * rsi: task we're switching to
-    LOCK ; btr $TIF_FORK, TI_flags(%r8)
+    LOCK ; btr $TIF_FORK, TI_flags(%rsi)    /* rsi: this newly forked task */

     call    schedule_tail            /* rdi: 'prev' task parameter */

I think you forgot GET_THREAD_INFO() here.  RSI is the task, not the
thread_info.  FYI, this goes away with my switch_to() rewrite, which
removes TIF_FORK.

Brian Gerst

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.