From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751963AbcFWRoN (ORCPT ); Thu, 23 Jun 2016 13:44:13 -0400 Received: from mail-oi0-f66.google.com ([209.85.218.66]:36713 "EHLO mail-oi0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751690AbcFWRoK convert rfc822-to-8bit (ORCPT ); Thu, 23 Jun 2016 13:44:10 -0400 MIME-Version: 1.0 In-Reply-To: <20160623170352.GA17372@redhat.com> References: <20160623143126.GA16664@redhat.com> <20160623170352.GA17372@redhat.com> From: Linus Torvalds Date: Thu, 23 Jun 2016 10:44:08 -0700 X-Google-Sender-Auth: mA_6Mh2f5I45ofoJKVkAC4iJ5bQ Message-ID: Subject: Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) To: Oleg Nesterov Cc: Andy Lutomirski , 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 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jun 23, 2016 at 10:03 AM, Oleg Nesterov wrote: > > Let me quote my previous email ;) > > And we can't free/nullify it when the parent/debuger reaps a zombie, > say, mark_oom_victim() expects that get_task_struct() protects > thread_info as well. > > probably we can fix all such users though... TIF_MEMDIE is indeed a potential problem, but I don't think mark_oom_victim() is actually problematic. mark_oom_victim() is called with either "current", or with a victim that still has its mm and signal pointer (and the task is locked). So the lifetime is already guaranteed - or that code is already very very buggy, since it follows tsk->signal and tsk->mm So as far as I can tell, that's all fine. That said, by now it would actually in many ways be great if we could get rid of thread_info entirely. The historical reasons for thread_info have almost all been subsumed by the percpu area. The reason for thread_info originally was - we used to find the task_struct by just masking the stack pointer (long long ago). When the task struct grew too big, we kept just the critical pieces and some arch-specific stuff and , called it "thread_info", and moved the rest to an external allocation and added the pointer to it. - the really crticial stuff we didn't want to follow a pointer for, so things like preempt_count etc were in thread_info - but they were *so* critical that PeterZ (at my prodding) moved those things to percpu caches that get updated at schedule time instead so these days, thread_info has almost nothing really critical in it any more. There's the thread-local flags, yes, but they could stay or easily be moved to the task_struct or get similar per-cpu fixup as preempt_count did a couple of years ago. The only annoyance is the few remaining entry code assembly sequences, but I suspect they would actually become simpler with a per-cpu thing, and with Andy's cleanups they are pretty insignificant these days. There seems to be exactly two uses of ASM_THREAD_INFO(TI_flags,.. left. So I suspect that it would (a) already be possible to just free the stack and thread info at release time, because any rcu users will already be doing task_lock() and check mm etc. (b) it probably would be a nice cleanup to try to make it even more obviously safe by just shrinking thread_info more (or even getting rid of it entirely, but that may be too painful because there are other architectures that may depend on it more). I dunno. Looking at what remains of thread_info, it really doesn't seem very critical. The thread_info->tsk pointer, that was one of the most critical issues and the main raison d'être of the thread_info, has been replaced on x86 by just using the per-cpu "current_task". Yes,.there are probably more than a few "ti->task" users left for legacy reasons, harking back to when the thread-info was cheaper to access, but it shouldn't be a big deal. Linus From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Subject: Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) Date: Thu, 23 Jun 2016 10:44:08 -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 Content-Transfer-Encoding: quoted-printable Return-path: List-Post: List-Help: List-Unsubscribe: List-Subscribe: Sender: linus971@gmail.com In-Reply-To: <20160623170352.GA17372@redhat.com> To: Oleg Nesterov Cc: Andy Lutomirski , 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 10:03 AM, Oleg Nesterov wrote: > > Let me quote my previous email ;) > > And we can't free/nullify it when the parent/debuger reaps a zomb= ie, > say, mark_oom_victim() expects that get_task_struct() protects > thread_info as well. > > probably we can fix all such users though... TIF_MEMDIE is indeed a potential problem, but I don't think mark_oom_victim() is actually problematic. mark_oom_victim() is called with either "current", or with a victim that still has its mm and signal pointer (and the task is locked). So the lifetime is already guaranteed - or that code is already very very buggy, since it follows tsk->signal and tsk->mm So as far as I can tell, that's all fine. That said, by now it would actually in many ways be great if we could get rid of thread_info entirely. The historical reasons for thread_info have almost all been subsumed by the percpu area. The reason for thread_info originally was - we used to find the task_struct by just masking the stack pointer (long long ago). When the task struct grew too big, we kept just the critical pieces and some arch-specific stuff and , called it "thread_info", and moved the rest to an external allocation and added the pointer to it. - the really crticial stuff we didn't want to follow a pointer for, so things like preempt_count etc were in thread_info - but they were *so* critical that PeterZ (at my prodding) moved those things to percpu caches that get updated at schedule time instead so these days, thread_info has almost nothing really critical in it any more. There's the thread-local flags, yes, but they could stay or easily be moved to the task_struct or get similar per-cpu fixup as preempt_count did a couple of years ago. The only annoyance is the few remaining entry code assembly sequences, but I suspect they would actually become simpler with a per-cpu thing, and with Andy's cleanups they are pretty insignificant these days. There seems to be exactly two uses of ASM_THREAD_INFO(TI_flags,.. left. So I suspect that it would (a) already be possible to just free the stack and thread info at release time, because any rcu users will already be doing task_lock() and check mm etc. (b) it probably would be a nice cleanup to try to make it even more obviously safe by just shrinking thread_info more (or even getting rid of it entirely, but that may be too painful because there are other architectures that may depend on it more). I dunno. Looking at what remains of thread_info, it really doesn't seem very critical. The thread_info->tsk pointer, that was one of the most critical issues and the main raison d'=C3=AAtre of the thread_info, has been replaced on x86 by just using the per-cpu "current_task". Yes,.there are probably more than a few "ti->task" users left for legacy reasons, harking back to when the thread-info was cheaper to access, but it shouldn't be a big deal. Linus From mboxrd@z Thu Jan 1 00:00:00 1970 Reply-To: kernel-hardening@lists.openwall.com MIME-Version: 1.0 Sender: linus971@gmail.com In-Reply-To: <20160623170352.GA17372@redhat.com> References: <20160623143126.GA16664@redhat.com> <20160623170352.GA17372@redhat.com> From: Linus Torvalds Date: Thu, 23 Jun 2016 10:44:08 -0700 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [kernel-hardening] Re: [PATCH v3 00/13] Virtually mapped stacks with guard pages (x86, core) To: Oleg Nesterov Cc: Andy Lutomirski , 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 10:03 AM, Oleg Nesterov wrote: > > Let me quote my previous email ;) > > And we can't free/nullify it when the parent/debuger reaps a zomb= ie, > say, mark_oom_victim() expects that get_task_struct() protects > thread_info as well. > > probably we can fix all such users though... TIF_MEMDIE is indeed a potential problem, but I don't think mark_oom_victim() is actually problematic. mark_oom_victim() is called with either "current", or with a victim that still has its mm and signal pointer (and the task is locked). So the lifetime is already guaranteed - or that code is already very very buggy, since it follows tsk->signal and tsk->mm So as far as I can tell, that's all fine. That said, by now it would actually in many ways be great if we could get rid of thread_info entirely. The historical reasons for thread_info have almost all been subsumed by the percpu area. The reason for thread_info originally was - we used to find the task_struct by just masking the stack pointer (long long ago). When the task struct grew too big, we kept just the critical pieces and some arch-specific stuff and , called it "thread_info", and moved the rest to an external allocation and added the pointer to it. - the really crticial stuff we didn't want to follow a pointer for, so things like preempt_count etc were in thread_info - but they were *so* critical that PeterZ (at my prodding) moved those things to percpu caches that get updated at schedule time instead so these days, thread_info has almost nothing really critical in it any more. There's the thread-local flags, yes, but they could stay or easily be moved to the task_struct or get similar per-cpu fixup as preempt_count did a couple of years ago. The only annoyance is the few remaining entry code assembly sequences, but I suspect they would actually become simpler with a per-cpu thing, and with Andy's cleanups they are pretty insignificant these days. There seems to be exactly two uses of ASM_THREAD_INFO(TI_flags,.. left. So I suspect that it would (a) already be possible to just free the stack and thread info at release time, because any rcu users will already be doing task_lock() and check mm etc. (b) it probably would be a nice cleanup to try to make it even more obviously safe by just shrinking thread_info more (or even getting rid of it entirely, but that may be too painful because there are other architectures that may depend on it more). I dunno. Looking at what remains of thread_info, it really doesn't seem very critical. The thread_info->tsk pointer, that was one of the most critical issues and the main raison d'=C3=AAtre of the thread_info, has been replaced on x86 by just using the per-cpu "current_task". Yes,.there are probably more than a few "ti->task" users left for legacy reasons, harking back to when the thread-info was cheaper to access, but it shouldn't be a big deal. Linus