From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Lutomirski Subject: Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Date: Thu, 23 Jun 2016 12:08:59 -0700 Message-ID: References: <20160623143126.GA16664@redhat.com> <20160623170352.GA17372@redhat.com> Reply-To: kernel-hardening@lists.openwall.com Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: List-Post: List-Help: List-Unsubscribe: List-Subscribe: In-Reply-To: To: Linus Torvalds Cc: Oleg Nesterov , Peter Zijlstra , Andy Lutomirski , the arch/x86 maintainers , Linux Kernel Mailing List , "linux-arch@vger.kernel.org" , Borislav Petkov , Nadav Amit , Kees Cook , Brian Gerst , "kernel-hardening@lists.openwall.com" , Josh Poimboeuf , Jann Horn , Heiko Carstens List-Id: linux-arch.vger.kernel.org On Thu, Jun 23, 2016 at 11:46 AM, Linus Torvalds wrote: > On Thu, Jun 23, 2016 at 10:52 AM, Linus Torvalds > wrote: >> >> Ugh. Looking around at this, it turns out that a great example of this >> kind of legacy issue is the debug_mutex stuff. > > Interestingly, the *only* other user of ti->task for a full > allmodconfig build of x86-64 seems to be > > arch/x86/kernel/dumpstack.c > > with the print_context_stack() -> print_ftrace_graph_addr() -> task = > tinfo->task chain. > > And that doesn't really seem to want thread_info either. The callers > all have 'task', and have to generate thread_info from that anyway. > > So this attached patch (which includes the previous one) seems to > build. I didn't actually boot it, but there should be no users left > unless there is some asm code that has hardcoded offsets.. I think you'll break some architectures when you remove the initialization of ti->task. That either needs to be pushed down into arch code in unicore32, openrisc, microblaze, powerpx, xtensa, sparc, parisc, arm, mips, s390, and whatever I missed, or you should leave the field initialized and existing and wait for my patch to conditionally remove/embed thread_info to get rid of the initialization part. On the C side, there's: arm's contextidr_notifier (easily fixable) sh's irqctx->tinfo.task = curctx->task; (probably useless) and print_ftrace_graph_addr cris's ugdb_trap_user (probably easily fixable) sparc's arch_trigger_all_cpu_backtrace (possibly quite hard to fix) sparc's flush_thread (trivial) sparc's __save_stack_trace (not sure) unicore's __die (probably easy) metag's do_softirq_own_stack (not sure if it's useful) I found these with this coccinelle script: @@ struct thread_info *ti; @@ * ti->task $ spatch --sp-file titask.cocci --dir . From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk0-f51.google.com ([209.85.213.51]:33950 "EHLO mail-vk0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751014AbcFWTJZ (ORCPT ); Thu, 23 Jun 2016 15:09:25 -0400 Received: by mail-vk0-f51.google.com with SMTP id c2so89934787vkg.1 for ; Thu, 23 Jun 2016 12:09:20 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <20160623143126.GA16664@redhat.com> <20160623170352.GA17372@redhat.com> From: Andy Lutomirski Date: Thu, 23 Jun 2016 12:08:59 -0700 Message-ID: Subject: Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Content-Type: text/plain; charset=UTF-8 Sender: linux-arch-owner@vger.kernel.org List-ID: To: Linus Torvalds Cc: Oleg Nesterov , Peter Zijlstra , Andy Lutomirski , the arch/x86 maintainers , Linux Kernel Mailing List , "linux-arch@vger.kernel.org" , Borislav Petkov , Nadav Amit , Kees Cook , Brian Gerst , "kernel-hardening@lists.openwall.com" , Josh Poimboeuf , Jann Horn , Heiko Carstens Message-ID: <20160623190859.ks-eNyHTl9S0unmjldyER5UL5bEYmfDcbYRyXq4_YU4@z> On Thu, Jun 23, 2016 at 11:46 AM, Linus Torvalds wrote: > On Thu, Jun 23, 2016 at 10:52 AM, Linus Torvalds > wrote: >> >> Ugh. Looking around at this, it turns out that a great example of this >> kind of legacy issue is the debug_mutex stuff. > > Interestingly, the *only* other user of ti->task for a full > allmodconfig build of x86-64 seems to be > > arch/x86/kernel/dumpstack.c > > with the print_context_stack() -> print_ftrace_graph_addr() -> task = > tinfo->task chain. > > And that doesn't really seem to want thread_info either. The callers > all have 'task', and have to generate thread_info from that anyway. > > So this attached patch (which includes the previous one) seems to > build. I didn't actually boot it, but there should be no users left > unless there is some asm code that has hardcoded offsets.. I think you'll break some architectures when you remove the initialization of ti->task. That either needs to be pushed down into arch code in unicore32, openrisc, microblaze, powerpx, xtensa, sparc, parisc, arm, mips, s390, and whatever I missed, or you should leave the field initialized and existing and wait for my patch to conditionally remove/embed thread_info to get rid of the initialization part. On the C side, there's: arm's contextidr_notifier (easily fixable) sh's irqctx->tinfo.task = curctx->task; (probably useless) and print_ftrace_graph_addr cris's ugdb_trap_user (probably easily fixable) sparc's arch_trigger_all_cpu_backtrace (possibly quite hard to fix) sparc's flush_thread (trivial) sparc's __save_stack_trace (not sure) unicore's __die (probably easy) metag's do_softirq_own_stack (not sure if it's useful) I found these with this coccinelle script: @@ struct thread_info *ti; @@ * ti->task $ spatch --sp-file titask.cocci --dir .