From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Lutomirski Subject: Re: [PATCH v4 08/29] dma-api: Teach the "DMA-from-stack" check about vmapped stacks Date: Wed, 6 Jul 2016 06:20:46 -0700 Message-ID: References: <20160630193723.GE3932@pd.tnic> 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: In-Reply-To: <20160630193723.GE3932@pd.tnic> To: Borislav Petkov Cc: Andy Lutomirski , X86 ML , "linux-kernel@vger.kernel.org" , linux-arch , Nadav Amit , Kees Cook , Brian Gerst , "kernel-hardening@lists.openwall.com" , Linus Torvalds , Josh Poimboeuf , Jann Horn , Heiko Carstens , Andrew Morton , Arnd Bergmann List-Id: linux-arch.vger.kernel.org On Thu, Jun 30, 2016 at 12:37 PM, Borislav Petkov wrote: > On Sun, Jun 26, 2016 at 02:55:30PM -0700, Andy Lutomirski wrote: >> If we're using CONFIG_VMAP_STACK and we manage to point an sg entry >> at the stack, then either the sg page will be in highmem or sg_virt >> will return the direct-map alias. In neither case will the existing >> check_for_stack() implementation realize that it's a stack page. >> >> Fix it by explicitly checking for stack pages. >> >> This has no effect by itself. It's broken out for ease of review. >> >> Cc: Andrew Morton >> Cc: Arnd Bergmann >> Signed-off-by: Andy Lutomirski >> --- >> lib/dma-debug.c | 39 +++++++++++++++++++++++++++++++++------ >> 1 file changed, 33 insertions(+), 6 deletions(-) >> >> diff --git a/lib/dma-debug.c b/lib/dma-debug.c >> index 51a76af25c66..5b2e63cba90e 100644 >> --- a/lib/dma-debug.c >> +++ b/lib/dma-debug.c >> @@ -22,6 +22,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -1162,11 +1163,35 @@ static void check_unmap(struct dma_debug_entry *= ref) >> put_hash_bucket(bucket, &flags); >> } >> >> -static void check_for_stack(struct device *dev, void *addr) >> +static void check_for_stack(struct device *dev, >> + struct page *page, size_t offset) >> { >> - if (object_is_on_stack(addr)) >> - err_printk(dev, NULL, "DMA-API: device driver maps memory = from " >> - "stack [addr=3D%p]\n", addr); >> + void *addr; >> + struct vm_struct *stack_vm_area =3D task_stack_vm_area(current); > > lib/dma-debug.c: In function =E2=80=98check_for_stack=E2=80=99: > lib/dma-debug.c:1170:36: error: implicit declaration of function =E2=80= =98task_stack_vm_area=E2=80=99 [-Werror=3Dimplicit-function-declaration] > struct vm_struct *stack_vm_area =3D task_stack_vm_area(current); > ^ > lib/dma-debug.c:1170:36: warning: initialization makes pointer from integ= er without a cast [-Wint-conversion] > cc1: some warnings being treated as errors > make[1]: *** [lib/dma-debug.o] Error 1 > make: *** [lib] Error 2 > make: *** Waiting for unfinished jobs.... > > Probably reorder pieces from patch 9 to earlier ones... I'll address this by reordering it later in the series. The temporary loss of functionality will be unobservable. --Andy From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk0-f44.google.com ([209.85.213.44]:32989 "EHLO mail-vk0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752644AbcGFNVH convert rfc822-to-8bit (ORCPT ); Wed, 6 Jul 2016 09:21:07 -0400 Received: by mail-vk0-f44.google.com with SMTP id b192so4564267vke.0 for ; Wed, 06 Jul 2016 06:21:06 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20160630193723.GE3932@pd.tnic> References: <20160630193723.GE3932@pd.tnic> From: Andy Lutomirski Date: Wed, 6 Jul 2016 06:20:46 -0700 Message-ID: Subject: Re: [PATCH v4 08/29] dma-api: Teach the "DMA-from-stack" check about vmapped stacks Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-arch-owner@vger.kernel.org List-ID: To: Borislav Petkov Cc: Andy Lutomirski , X86 ML , "linux-kernel@vger.kernel.org" , linux-arch , Nadav Amit , Kees Cook , Brian Gerst , "kernel-hardening@lists.openwall.com" , Linus Torvalds , Josh Poimboeuf , Jann Horn , Heiko Carstens , Andrew Morton , Arnd Bergmann Message-ID: <20160706132046.cryuqp8vuRs07npuNWLJcgVtGDR7g85vWVojSXFLNlY@z> On Thu, Jun 30, 2016 at 12:37 PM, Borislav Petkov wrote: > On Sun, Jun 26, 2016 at 02:55:30PM -0700, Andy Lutomirski wrote: >> If we're using CONFIG_VMAP_STACK and we manage to point an sg entry >> at the stack, then either the sg page will be in highmem or sg_virt >> will return the direct-map alias. In neither case will the existing >> check_for_stack() implementation realize that it's a stack page. >> >> Fix it by explicitly checking for stack pages. >> >> This has no effect by itself. It's broken out for ease of review. >> >> Cc: Andrew Morton >> Cc: Arnd Bergmann >> Signed-off-by: Andy Lutomirski >> --- >> lib/dma-debug.c | 39 +++++++++++++++++++++++++++++++++------ >> 1 file changed, 33 insertions(+), 6 deletions(-) >> >> diff --git a/lib/dma-debug.c b/lib/dma-debug.c >> index 51a76af25c66..5b2e63cba90e 100644 >> --- a/lib/dma-debug.c >> +++ b/lib/dma-debug.c >> @@ -22,6 +22,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -1162,11 +1163,35 @@ static void check_unmap(struct dma_debug_entry *ref) >> put_hash_bucket(bucket, &flags); >> } >> >> -static void check_for_stack(struct device *dev, void *addr) >> +static void check_for_stack(struct device *dev, >> + struct page *page, size_t offset) >> { >> - if (object_is_on_stack(addr)) >> - err_printk(dev, NULL, "DMA-API: device driver maps memory from " >> - "stack [addr=%p]\n", addr); >> + void *addr; >> + struct vm_struct *stack_vm_area = task_stack_vm_area(current); > > lib/dma-debug.c: In function ‘check_for_stack’: > lib/dma-debug.c:1170:36: error: implicit declaration of function ‘task_stack_vm_area’ [-Werror=implicit-function-declaration] > struct vm_struct *stack_vm_area = task_stack_vm_area(current); > ^ > lib/dma-debug.c:1170:36: warning: initialization makes pointer from integer without a cast [-Wint-conversion] > cc1: some warnings being treated as errors > make[1]: *** [lib/dma-debug.o] Error 1 > make: *** [lib] Error 2 > make: *** Waiting for unfinished jobs.... > > Probably reorder pieces from patch 9 to earlier ones... I'll address this by reordering it later in the series. The temporary loss of functionality will be unobservable. --Andy