From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751751AbcFWQlf (ORCPT ); Thu, 23 Jun 2016 12:41:35 -0400 Received: from mail-vk0-f45.google.com ([209.85.213.45]:36251 "EHLO mail-vk0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750919AbcFWQlc (ORCPT ); Thu, 23 Jun 2016 12:41:32 -0400 MIME-Version: 1.0 In-Reply-To: References: <20160623143126.GA16664@redhat.com> From: Andy Lutomirski Date: Thu, 23 Jun 2016 09:41:11 -0700 Message-ID: Subject: Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) To: Linus Torvalds Cc: Oleg Nesterov , 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 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 Thu, Jun 23, 2016 at 9:30 AM, Linus Torvalds wrote: > On Thu, Jun 23, 2016 at 7:31 AM, Oleg Nesterov wrote: >> >> I didn't see the patches yet, quite possibly I misunderstood... But no, >> I don't this we can do this (if we are not going to move ti->flags to >> task_struct at least). > > Argh. Yes, ti->flags is used by others. Everything else should be > thread-synchronous, but there's ti->flags. > > (And if we get scheduled, the thread-synchronous things will matter, of course): > >> Yes, but the problem is that a zombie thread can do its last schedule >> before it is reaped. > > Worse, the wait sequence will definitely look at it. > > But that does bring up another possibility: do it at wait() time, when > we do release_thread(). That's when we *used* to synchronously free > it, before we did the lockless RCU walks. > > At that point, it has been removed from all the thread lists. So the > only way to find it is through the RCU walks. Do any of *those* touch > ti->flags? I'm not seeing it, and it sounds fixable if any do. > > If we could release the thread stack in release_thread(), that would be good. > > Andy - I bet you can at least test it. That sounds a bit more fragile than I'm really comfortable with, although it'll at least oops reliably if we get it wrong. But I'm planning on moving ti->flags (and the rest of thread_info, either piecemeal or as a unit) into task_struct on architectures that opt in, which, as a practical matter, hopefully means everyone who opts in to virtual stacks. So I'm more inclined make all the changes in a different order: 1. Virtually mapped stacks (off by default but merged for testing, possibly with a warning that distros shouldn't enable it yet.) 2. thread_info cleanup (which I want to do *anyway* because it's critical to get the full hardening benefit) 3. Free stacks immediately and cache them (really easy). This has the benefit of being much less dependent on who access what field when and it should perform well with no churn. I'm hoping to have the thread_info stuff done in time for 4.8, too. --Andy 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 09:41:11 -0700 Message-ID: References: <20160623143126.GA16664@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: Received: from mail-vk0-f54.google.com ([209.85.213.54]:34846 "EHLO mail-vk0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751138AbcFWQlc (ORCPT ); Thu, 23 Jun 2016 12:41:32 -0400 Received: by mail-vk0-f54.google.com with SMTP id j2so114387373vkg.2 for ; Thu, 23 Jun 2016 09:41:31 -0700 (PDT) In-Reply-To: Sender: linux-arch-owner@vger.kernel.org List-ID: To: Linus Torvalds Cc: Oleg Nesterov , 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 On Thu, Jun 23, 2016 at 9:30 AM, Linus Torvalds wrote: > On Thu, Jun 23, 2016 at 7:31 AM, Oleg Nesterov wrote: >> >> I didn't see the patches yet, quite possibly I misunderstood... But no, >> I don't this we can do this (if we are not going to move ti->flags to >> task_struct at least). > > Argh. Yes, ti->flags is used by others. Everything else should be > thread-synchronous, but there's ti->flags. > > (And if we get scheduled, the thread-synchronous things will matter, of course): > >> Yes, but the problem is that a zombie thread can do its last schedule >> before it is reaped. > > Worse, the wait sequence will definitely look at it. > > But that does bring up another possibility: do it at wait() time, when > we do release_thread(). That's when we *used* to synchronously free > it, before we did the lockless RCU walks. > > At that point, it has been removed from all the thread lists. So the > only way to find it is through the RCU walks. Do any of *those* touch > ti->flags? I'm not seeing it, and it sounds fixable if any do. > > If we could release the thread stack in release_thread(), that would be good. > > Andy - I bet you can at least test it. That sounds a bit more fragile than I'm really comfortable with, although it'll at least oops reliably if we get it wrong. But I'm planning on moving ti->flags (and the rest of thread_info, either piecemeal or as a unit) into task_struct on architectures that opt in, which, as a practical matter, hopefully means everyone who opts in to virtual stacks. So I'm more inclined make all the changes in a different order: 1. Virtually mapped stacks (off by default but merged for testing, possibly with a warning that distros shouldn't enable it yet.) 2. thread_info cleanup (which I want to do *anyway* because it's critical to get the full hardening benefit) 3. Free stacks immediately and cache them (really easy). This has the benefit of being much less dependent on who access what field when and it should perform well with no churn. I'm hoping to have the thread_info stuff done in time for 4.8, too. --Andy From mboxrd@z Thu Jan 1 00:00:00 1970 Reply-To: kernel-hardening@lists.openwall.com MIME-Version: 1.0 In-Reply-To: References: <20160623143126.GA16664@redhat.com> From: Andy Lutomirski Date: Thu, 23 Jun 2016 09:41:11 -0700 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: [kernel-hardening] Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) To: Linus Torvalds Cc: Oleg Nesterov , 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: On Thu, Jun 23, 2016 at 9:30 AM, Linus Torvalds wrote: > On Thu, Jun 23, 2016 at 7:31 AM, Oleg Nesterov wrote: >> >> I didn't see the patches yet, quite possibly I misunderstood... But no, >> I don't this we can do this (if we are not going to move ti->flags to >> task_struct at least). > > Argh. Yes, ti->flags is used by others. Everything else should be > thread-synchronous, but there's ti->flags. > > (And if we get scheduled, the thread-synchronous things will matter, of course): > >> Yes, but the problem is that a zombie thread can do its last schedule >> before it is reaped. > > Worse, the wait sequence will definitely look at it. > > But that does bring up another possibility: do it at wait() time, when > we do release_thread(). That's when we *used* to synchronously free > it, before we did the lockless RCU walks. > > At that point, it has been removed from all the thread lists. So the > only way to find it is through the RCU walks. Do any of *those* touch > ti->flags? I'm not seeing it, and it sounds fixable if any do. > > If we could release the thread stack in release_thread(), that would be good. > > Andy - I bet you can at least test it. That sounds a bit more fragile than I'm really comfortable with, although it'll at least oops reliably if we get it wrong. But I'm planning on moving ti->flags (and the rest of thread_info, either piecemeal or as a unit) into task_struct on architectures that opt in, which, as a practical matter, hopefully means everyone who opts in to virtual stacks. So I'm more inclined make all the changes in a different order: 1. Virtually mapped stacks (off by default but merged for testing, possibly with a warning that distros shouldn't enable it yet.) 2. thread_info cleanup (which I want to do *anyway* because it's critical to get the full hardening benefit) 3. Free stacks immediately and cache them (really easy). This has the benefit of being much less dependent on who access what field when and it should perform well with no churn. I'm hoping to have the thread_info stuff done in time for 4.8, too. --Andy