From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751424AbcFXMZY (ORCPT ); Fri, 24 Jun 2016 08:25:24 -0400 Received: from mail-oi0-f42.google.com ([209.85.218.42]:34436 "EHLO mail-oi0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751089AbcFXMZW (ORCPT ); Fri, 24 Jun 2016 08:25:22 -0400 MIME-Version: 1.0 In-Reply-To: References: <20160623143126.GA16664@redhat.com> <20160623170352.GA17372@redhat.com> <20160623185340.GO30154@twins.programming.kicks-ass.net> From: Brian Gerst Date: Fri, 24 Jun 2016 08:25:20 -0400 Message-ID: Subject: Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) To: Linus Torvalds Cc: Peter Zijlstra , Oleg Nesterov , Andy Lutomirski , Andy Lutomirski , "the arch/x86 maintainers" , Linux Kernel Mailing List , "linux-arch@vger.kernel.org" , Borislav Petkov , Nadav Amit , Kees Cook , "kernel-hardening@lists.openwall.com" , Josh Poimboeuf , Jann Horn , Heiko Carstens Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 */ 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