From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752877AbcFUROm (ORCPT ); Tue, 21 Jun 2016 13:14:42 -0400 Received: from mail-lb0-f179.google.com ([209.85.217.179]:36300 "EHLO mail-lb0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752700AbcFUROT (ORCPT ); Tue, 21 Jun 2016 13:14:19 -0400 MIME-Version: 1.0 In-Reply-To: References: From: Kees Cook Date: Tue, 21 Jun 2016 10:13:14 -0700 X-Google-Sender-Auth: 516qG28V74vIX0NSwtT_AKeRb5Y Message-ID: Subject: Re: [PATCH v3 06/13] fork: Add generic vmalloced stack support To: Andy Lutomirski Cc: Jann Horn , Andy Lutomirski , X86 ML , "linux-kernel@vger.kernel.org" , linux-arch , Borislav Petkov , Nadav Amit , Brian Gerst , "kernel-hardening@lists.openwall.com" , Linus Torvalds , 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 Tue, Jun 21, 2016 at 9:59 AM, Andy Lutomirski wrote: > On Tue, Jun 21, 2016 at 12:30 AM, Jann Horn wrote: >> On Tue, Jun 21, 2016 at 1:43 AM, Andy Lutomirski wrote: >>> If CONFIG_VMAP_STACK is selected, kernel stacks are allocated with >>> vmalloc_node. >> [...] >>> static struct thread_info *alloc_thread_info_node(struct task_struct *tsk, >>> int node) >>> { >>> +#ifdef CONFIG_VMAP_STACK >>> + struct thread_info *ti = __vmalloc_node_range( >>> + THREAD_SIZE, THREAD_SIZE, VMALLOC_START, VMALLOC_END, >>> + THREADINFO_GFP | __GFP_HIGHMEM, PAGE_KERNEL, >>> + 0, node, __builtin_return_address(0)); >>> + >> >> After spender gave some hints on IRC about the guard pages not working >> reliably, I decided to have a closer look at this. As far as I can >> tell, the idea is that __vmalloc_node_range() automatically adds guard >> pages unless the VM_NO_GUARD flag is specified. However, those guard >> pages are *behind* allocations, not in front of them, while a stack >> guard primarily needs to be in front of the allocation. This wouldn't >> matter if all allocations in the vmalloc area had guard pages behind >> them, but if someone first does some data allocation with VM_NO_GUARD >> and then a stack allocation directly behind that, there won't be a >> guard between the data allocation and the stack allocation. > > I'm tempted to explicitly disallow VM_NO_GUARD in the vmalloc range. > It has no in-tree users for non-fixed addresses right now. What about the lack of pre-range guard page? That seems like a critical feature for this. :) -Kees -- Kees Cook Chrome OS & Brillo Security From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kees Cook Subject: Re: [PATCH v3 06/13] fork: Add generic vmalloced stack support Date: Tue, 21 Jun 2016 10:13:14 -0700 Message-ID: References: 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: Sender: keescook@google.com In-Reply-To: To: Andy Lutomirski Cc: Jann Horn , Andy Lutomirski , X86 ML , "linux-kernel@vger.kernel.org" , linux-arch , Borislav Petkov , Nadav Amit , Brian Gerst , "kernel-hardening@lists.openwall.com" , Linus Torvalds , Josh Poimboeuf , Jann Horn , Heiko Carstens List-Id: linux-arch.vger.kernel.org On Tue, Jun 21, 2016 at 9:59 AM, Andy Lutomirski wrote: > On Tue, Jun 21, 2016 at 12:30 AM, Jann Horn wrote: >> On Tue, Jun 21, 2016 at 1:43 AM, Andy Lutomirski wrote: >>> If CONFIG_VMAP_STACK is selected, kernel stacks are allocated with >>> vmalloc_node. >> [...] >>> static struct thread_info *alloc_thread_info_node(struct task_struct *tsk, >>> int node) >>> { >>> +#ifdef CONFIG_VMAP_STACK >>> + struct thread_info *ti = __vmalloc_node_range( >>> + THREAD_SIZE, THREAD_SIZE, VMALLOC_START, VMALLOC_END, >>> + THREADINFO_GFP | __GFP_HIGHMEM, PAGE_KERNEL, >>> + 0, node, __builtin_return_address(0)); >>> + >> >> After spender gave some hints on IRC about the guard pages not working >> reliably, I decided to have a closer look at this. As far as I can >> tell, the idea is that __vmalloc_node_range() automatically adds guard >> pages unless the VM_NO_GUARD flag is specified. However, those guard >> pages are *behind* allocations, not in front of them, while a stack >> guard primarily needs to be in front of the allocation. This wouldn't >> matter if all allocations in the vmalloc area had guard pages behind >> them, but if someone first does some data allocation with VM_NO_GUARD >> and then a stack allocation directly behind that, there won't be a >> guard between the data allocation and the stack allocation. > > I'm tempted to explicitly disallow VM_NO_GUARD in the vmalloc range. > It has no in-tree users for non-fixed addresses right now. What about the lack of pre-range guard page? That seems like a critical feature for this. :) -Kees -- Kees Cook Chrome OS & Brillo Security From mboxrd@z Thu Jan 1 00:00:00 1970 Reply-To: kernel-hardening@lists.openwall.com MIME-Version: 1.0 Sender: keescook@google.com In-Reply-To: References: From: Kees Cook Date: Tue, 21 Jun 2016 10:13:14 -0700 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: [kernel-hardening] Re: [PATCH v3 06/13] fork: Add generic vmalloced stack support To: Andy Lutomirski Cc: Jann Horn , Andy Lutomirski , X86 ML , "linux-kernel@vger.kernel.org" , linux-arch , Borislav Petkov , Nadav Amit , Brian Gerst , "kernel-hardening@lists.openwall.com" , Linus Torvalds , Josh Poimboeuf , Jann Horn , Heiko Carstens List-ID: On Tue, Jun 21, 2016 at 9:59 AM, Andy Lutomirski wrote: > On Tue, Jun 21, 2016 at 12:30 AM, Jann Horn wrote: >> On Tue, Jun 21, 2016 at 1:43 AM, Andy Lutomirski wrote: >>> If CONFIG_VMAP_STACK is selected, kernel stacks are allocated with >>> vmalloc_node. >> [...] >>> static struct thread_info *alloc_thread_info_node(struct task_struct *tsk, >>> int node) >>> { >>> +#ifdef CONFIG_VMAP_STACK >>> + struct thread_info *ti = __vmalloc_node_range( >>> + THREAD_SIZE, THREAD_SIZE, VMALLOC_START, VMALLOC_END, >>> + THREADINFO_GFP | __GFP_HIGHMEM, PAGE_KERNEL, >>> + 0, node, __builtin_return_address(0)); >>> + >> >> After spender gave some hints on IRC about the guard pages not working >> reliably, I decided to have a closer look at this. As far as I can >> tell, the idea is that __vmalloc_node_range() automatically adds guard >> pages unless the VM_NO_GUARD flag is specified. However, those guard >> pages are *behind* allocations, not in front of them, while a stack >> guard primarily needs to be in front of the allocation. This wouldn't >> matter if all allocations in the vmalloc area had guard pages behind >> them, but if someone first does some data allocation with VM_NO_GUARD >> and then a stack allocation directly behind that, there won't be a >> guard between the data allocation and the stack allocation. > > I'm tempted to explicitly disallow VM_NO_GUARD in the vmalloc range. > It has no in-tree users for non-fixed addresses right now. What about the lack of pre-range guard page? That seems like a critical feature for this. :) -Kees -- Kees Cook Chrome OS & Brillo Security