|
Message-ID: <CAMzpN2i7ve0Rq9BCxjp2FUw5naPLrGDd7Mf3evrrRjUZN75oJQ@mail.gmail.com> Date: Fri, 24 Jun 2016 08:25:20 -0400 From: Brian Gerst <brgerst@...il.com> To: Linus Torvalds <torvalds@...ux-foundation.org> Cc: Peter Zijlstra <peterz@...radead.org>, Oleg Nesterov <oleg@...hat.com>, Andy Lutomirski <luto@...capital.net>, 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 2:17 AM, Linus Torvalds <torvalds@...ux-foundation.org> wrote: > On Thu, Jun 23, 2016 at 12:17 PM, Linus Torvalds > <torvalds@...ux-foundation.org> 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 */ ENTRY(ret_from_fork) - 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.